Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:34239
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.net designates 213.165.64.20 as permitted sender)
Message-ID: <476FCCB8.2060604@gmx.net>
Date: Mon, 24 Dec 2007 16:14:00 +0100
User-Agent: Thunderbird 2.0.0.9 (X11/20071031)
MIME-Version: 1.0
To: php-dev List <internals@lists.php.net>
References: <98b8086f0712150818n40056cedyf0aae7a5a08a27b7@mail.gmail.com> <476582E6.7020808@zend.com> <D1011E25-76DB-4936-AF3B-F98F19DC2139@procata.com> <200712172130.08216.larry@garfieldtech.com> <4FADC266-873E-4FD2-BEC8-28EA9D833297@procata.com> <476D2854.5070803@gmx.net> <503D0384-1014-4CD1-BCAC-8F0950D5FC68@bitxtender.com> <476E6532.5080402@gmx.net> <20071224040401.GA36003@csh.rit.edu>
In-Reply-To: <20071224040401.GA36003@csh.rit.edu>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] PATCH: Implementing closures in PHP
From: chris_se@gmx.net (Christian Seiler)

Hi!

Thanks for reading through!

> 1.  There appear to be some spurious whitespace insertions in this
>     version of the patch.

Oh, that's probably my editor, I'll fix that.

> 2.  The terms "lamba" and "anonymous function" are being used
>     interchangeably.  If we're going to introduce new terminology, it
>     would be good to pick one name and use it consistently.  I don't
>     have a preference for which one is ultimately chosen.

Well, create_function uses an already-existing EG(lambda_count) and
names the function __lambda_$counter so I thought I'd use
CG(compiled_lambda_count) and name them __compiled_lambda_... But since
anonymous functions aren't REAL lambdas, I named them anonymous
elsewhere. But you're right, introducing duplicate terminology is a bad
idea, I'll change everything to lambda for consistency, even though it's
technically not 100% correct.

>     The term "lexical" could also be considered a competing term as
>     its used in part of the patch.

'lexical' is only used for the variables that are passed into the
closure, not for the closure itself.

> 3.  The "is_anonymous" flags could be zend_bool values instead of bare
>     integers, although that breaks the precedent started by some
>     related flags (such as "is_method").

You're right, zend_bool is a better idea. Since PHP 5.3 is going to
break binary compability anyway, would it do any harm changing the
types of the existing flags, too?

> 4.  This part of the zend_vm_def.h diff looks wrong (a stray "f"):
> 
>         -/*
>         +f/*

WTF? I thought I had already fixed that. Hmm, obviously I hadn't...

> Looks great overall!

Thanks!

Merry Christmas,
Christian