Hello, On Mon, Aug 4, 2008 at 4:18 PM, Dmitry Stogov <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php