Hi Etienne, In general the patch looks fine.
The only thing I don't like is explicit declaration of Closure::__invode method. It won't have proper arg_info.
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. -- PascalAfter 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? )
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
