Re: [PATCH] [RFC] Closures and lambda functions in PHP

Hi Christian, I reworked your patch a little bit. 1) Fixed ref/noref issues 2) Explicit $this passing $a = function foo() use ($this) {} 3) Some code reorganization to encapsulate closure implementation. Thanks. Dmitry. Christian Seiler wrote: > Hi Dmitry, > >> I'm fine if you'll improve my patch (It's mainly yours :) > > I updated my closures RFC: http://wiki.php.net/rfc/closures > > I have based my new version of the patch on yours (Dmitry), but I made > some changes to that: > > * Objects instead of resources are used, two new files > zend_closures.[ch] are added where the new Closure class > is defined. Currently, it contains a dummy __toString method > that in future may be extended to provide enhanced debugging info, > also further additional cool stuff could be added to such a > class later on. But I prefer to only add the basic closure > functionality at first - you can always extend it once it's there. > > * I have *not* added any __invoke() magic to normal objects. This is > mainly due to the simple reason that adding that would not help > a closure implementation at all. Closures need some engine internal > magic (use a dynamically created op_array instead of looking one up, > setting the correct class scope and setting the correct EG(this). And > as I said: I want to stick with the closure basics for now. > > That said, I do like the possibility of invoking objects directly, so > I suggest someone created an additional proposal for that? > > * I've added a patch for PHP HEAD (PHP 6.0). This is due to the fact > that Dmitry's variant of my patch has far less intersections with > the unicode functionality than my original patch, so it was quite > straight-forward to do so. > > * Lexical vars are now copied instead of referenced by default. Using > & in front of the var, the behaviour may be changed. I added that in > order to demonstrate that both was possible and that a simply change > of grammar suffices. In my eyes this is the main issue where a > discussion has to take place (i.e. copy or reference by default? > possibility to change default via syntax? which lexical syntax?) > before the proposal can be accepted. > > * I provided patches for both lexical $var and use ($var) syntaxes. > > * I provided a patch variant that only stores $this if $this is > explicitely used inside a closure (or a nested closure of that > closure). This works since it is possible to detect whether $this > is used at compile time. For this, I have added a this_used flag > to the op_array structure. > > * I added tests (Zend/tests/closures_*.phpt) that ensure the correct > behaviour of closures. > > [Note that I created my own local SVN repos for developing these patches > because I was fed up with CVS's inability to local diffs and locally > mark files as added to include them in the diffs. Just to explain the > format of the patch.] > > Anyway, feel free to discuss. > > In my eyes, the following questions should be answered: > > * Do you want closures in PHP? > > I have not seen a single negative reaction to my proposal, so I > assume the answer to that is yes. ;-) > > * Which syntax should be used for lexical variables? Should references > or copies be created by default? > > This is far trickier. > > First of all: There must *always* be the _possiblity_ to create > references, else you can't call it closures. > > Second: I prefer the 'lexical' keyword, but I could live with the > use solution [function () use ($...)]. > > Third: There are several arguments against default referencing: > > * (use syntax) As they look similar to parameters and normal > parameters aren't passed by ref, this could be quite > odd. > * Loop index WTFs (see proposal) > * Speed? (You always read that refs are slower in PHP than normal > variable copies. Is that actually true?) > * While it is possible to simply add an & in front of the variable > name to switch to refs in "no refs default" mode, there is no > obvious syntax to use copies in "refs default" mode other than > unsetting the variable in the parent scope immediately after > closure creation. > > Fourth: There are several arguments for default referencing: > > * (lexical syntax) global also creates a reference, why shouldn't > lexical? > * Other languages *appear* to exhibit a very similar behaviour to > PHP if PHP created references. This is due to the fact that > other languages have a different concept of scope as PHP > does. > > Although the list of against arguments appears to be longer, I do > prefer using references by default nevertheless. But that's just > my personal opinion. > > * Are you OK with the change that $this is only stored when needed? > > I don't see a problem. Dmitry seems to be very touchy (;-)) about > changing op_arrays but in this case it's only a flag so I don't > see a problem for opcode caches (in contrast to a HashTable where > the opcode cache must actually add code to duplicate that table). > > * Do you want closures in PHP 5.3? > > Since the majority of core developers appear to be against it, I > presume the answer is no. > > I will provide a revised patch that incorporates the results of the > following discussion for 5_3 and HEAD once consensus or at least a > majority regarding the remaining issues is reached. I will also rewrite > the proposal to reflect the discussion results and adjust the tests. > After that, I hope that someone will commit at least the HEAD version. > > Regards, > Christian