Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:39635
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.44.30 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version
         :content-type:content-transfer-encoding:content-disposition
         :references:x-google-sender-auth;
        b=nMV1drHnf/r9+afxLxJnzBNMC072IsFcsEVEyb+93XYf6Rdm/XkJQYiLYUvufTwJaZ
         lhM9N3IDZOLz9eglI3REhHAoOInZmxO8MqIvqfYUb4frGCSU+FxfMTBjMD3YURyJavBb
         laL1FkiXkRqgu+sac2VI5PX7NIfXhN2rTKZkA=
Message-ID: <e666c3890808040945u1e49c256s1c95b5655daa32e0@mail.gmail.com>
Date: Mon, 4 Aug 2008 18:45:05 +0200
Sender: ekneuss@gmail.com
To: "Dmitry Stogov" <dmitry@zend.com>
Cc: "Christian Seiler" <chris_se@gmx.net>, 
	"PHP Internals List" <internals@lists.php.net>
In-Reply-To: <48970FBC.4060401@zend.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <e666c3890808021036p58de549as422bec8471b7a992@mail.gmail.com>
	 <e666c3890808031503v5a966b75y4d81c750b43b64b7@mail.gmail.com>
	 <48970FBC.4060401@zend.com>
Subject: Re: [PHP-DEV] Re: __invoke concerns
From: colder@php.net ("Etienne Kneuss")

Hello,

On Mon, Aug 4, 2008 at 4:18 PM, Dmitry Stogov <dmitry@zend.com> wrote:
> Hi Christan,
>
> Could you please look into this patch.
> I'm not sure if explicit declaration of Closure::__invoke() is good idea. As
> it cannot provide proper argument information.
>
> May be the patch which you propose already solves this problem for
> reflection. (I didn't have time to look into it).
>

After discussing it with Dmitry, I removed the reflection part from
the patch, giving full reflection info on Closures seems like a better
idea, even if it requires patching Reflection itself.

Regards

> Thanks. Dmitry.
>
> Etienne Kneuss wrote:
>>
>> Hello,
>>
>> On Sat, Aug 2, 2008 at 7:36 PM, Etienne Kneuss <colder@php.net> wrote:
>>>
>>> Hi,
>>>
>>> this is probably not the best time to raise concerns about __invoke
>>> (closures) now that alpha1 is already realeased, but I believe it's
>>> worth it.
>>>
>>> 1) I don't believe that having it thrown as another of those magic
>>> method is a good idea. Rather, I'd like to have it represented by an
>>> interface: Invokable. That way, type hints/checks can be done in user
>>> land in a sane matter:
>>>
>>> function foo(Invokable $obj) {
>>>   $obj();
>>> }
>>>
>>> if ($foo instanceof Invokable) $foo();
>>>
>>> etc..
>>>
>>> 2) Do we really want __invoke's argument to be mapped to caller
>>> arguments. Providing an array of arguments, ala __call(Static) sounds
>>> more consistent.
>>> class A { public function __invoke($arg) {var_dump($arg); }} $a = new
>>> A; $a(1,2); // int(1), currently. IMHO it should be array(1,2)
>>>
>>>
>>> 3) Do we really want to allow both static and non-static versions of
>>> __invoke ?
>>> class A { public static function __invoke($args) { .. }} $a = new A;
>>> $a(); being a static call to __invoke doesn't make much sense to me.
>>>
>>>
>>> I hope these issues can be discussed and maybe addressed before a
>>> final 5.3 release. I'm willing to do patches for all three concerns if
>>> I sense a positive feeling towards this.
>>>
>>> Regards,
>>>
>>> --
>>> Etienne Kneuss
>>> http://www.colder.ch
>>>
>>> Men never do evil so completely and cheerfully as
>>> when they do it from a religious conviction.
>>> -- Pascal
>>>
>>
>> After looking deeper into it, I noticed that this interface brings a
>> lot of problems:
>>
>> 1) With the interface, the prototype is fixed.
>> Which means that the following are no longer possible:
>>
>> Class A {
>> public function &__invoke(&$a, $b, $c) { return $a; }
>> }
>> $a = new A; $e =& $a($b,$c, $d);
>>
>> 2) __invoke($args) seems more consistent, and would give a consistent
>> prototype, but
>> 2.1) references are no longer possible (1)
>> 2.2) __invoke of actual closures/lambdas needs to map parameters for
>> $a = function($b,$c){}; to work properly. I don't believe $cl =
>> function($args){..} is something we want
>>
>> So, with those counter-concerns in mind, this Invokable interface is
>> no longer implementable, IMO.
>>
>> However, I'd still like to make closures more flexible and
>> internals-friendly by implementing zend_get_closure as a handler.
>> The only (tiny) issue here is that we export a bit more
>> closure-material in the engine, and it's no longer so much
>> self-contained in zend_closures.
>>
>> Patches:
>> http://patches.colder.ch/Zend/zend_get_closure_handler_53.patch?markup
>> http://patches.colder.ch/Zend/zend_get_closure_handler_HEAD.patch?markup
>>
>> ps: this patches also expose the __invoke method for reflection. (Or
>> is there an actual reason behind not exposing it? )
>
>



-- 
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal