Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118179
To: Larry Garfield <larry@garfieldtech.com>, internals@lists.php.net
Cc: php internals <internals@lists.php.net>, Marco Pivetta <ocramius@gmail.com>
Date: Mon, 04 Jul 2022 19:15:12 +0200
Message-ID: <6128711.lOV4Wx5bFT@arnaud-t490>
In-Reply-To: <CADyq6sJ0mvA9i4qEzeJ2K6JRakQNaMZz-cFAh3YV_iT=v3s+iA@mail.gmail.com>
References: <e506a378-2060-4cc1-98a6-3c9c7c0bd903@www.fastmail.com> <CADyq6sJ0mvA9i4qEzeJ2K6JRakQNaMZz-cFAh3YV_iT=v3s+iA@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [PHP-DEV] [RFC][Vote] Auto-capture closures
From: arnaud.lb@gmail.com (Arnaud Le Blanc)

Hi,

On lundi 4 juillet 2022 12:04:59 CEST Marco Pivetta wrote:
> I ended up voting NO due to the fact that there is no mention of `$this`
> behavior changes ( https://externals.io/message/117888#117889 ) : I also
> disagree with NikiC's stance on this being the same construct as before (
> https://externals.io/message/117888#117897 ), so I really wanted to get rid
> of this constant memleak risk that we have in closures. I should have made
> my stance stronger, instead of letting the discussion thread die, but time
> is what it is.

Forgot to add this in Future Scope, sorry.

The RFC does NOT change the behavior of `$this`, so as to keep it small. This 
is something I personally want, but it can be done after this RFC, and the 
change could be applied to all function kinds.

> Unclear (to me) after looking at
> https://gist.github.com/arnaud-lb/d9adfbf786ce9e37c7157b0f9c8d8e13, is
> whether the optimizations are applied at compile time. It looks like the
> changes are inside the `Zend/Optimizer/`, and `zend_compile.c`, so perhaps
> the benchmarks are probably just worded in a confusing way.

Sorry for the confusion. I confirm that the optimizations are applied at 
compile time. The benchmarks measure the runtime effect of these 
optimizations.

> Important to note how `Zend/zend_compile.c` now depends on `Optimizer/`,
> which is a potential design issue.

The Optimizer was moved to `Zend/Optimizer` earlier so that in the long term 
it could be used by the compiler pipeline directly (rather than as part of 
caching).

There are clear benefits in reusing the Optimizer here. Duplicating or 
reimplementing live variable analysis would increase maintenance cost, whereas 
by reusing the Optimizer's implementation we take profit of a fast and well 
tested implementation.

Greets,