Re: [RFC] Re: [PHP-DEV] __invoke() weirdness

Hello Hannes, as discussed online. At the moment we should not have any __get() calls during method resolution. The newly updated patch does that now. And I think we are now safe to submit. In the future we could think of adding __getClosure() which would be called to resolve a dynamic closure. But for the moment we do not need it badly and the patch with the increased consistency is good enough. marcus Thursday, January 1, 2009, 4:09:39 PM, you wrote: > Hello Hannes, > Wednesday, December 31, 2008, 8:33:43 PM, you wrote: >> On Wed, Dec 31, 2008 at 20:12, Marcus Boerger <helly@php.net> wrote: >>> Hello Lars, >>> >>> Wednesday, December 31, 2008, 6:59:08 PM, you wrote: >>> >>>> Hi Markus, >>> >>>> have you measured the performance impact in a class with - say - ten >>>> methods? And what to do with __get() and __call()? How are the >>>> prioritized in the method resolve order? >>> >>> Translated into user code we now have: >>> >>> public function __zend_call($name, $args) { >>> // Added property lookup >>> if (isset($this->$name)) { // may call __isset >>> $callable = $this->$name; // may call __get >> Uhmm. I hope I got this wrong as: > Well yes, there are no __isset() calls unless you call isset() of course. > I have updated the patch and added a test to demonstrate it better > (closure_036.phpt). I also added debug information to closures which makes > development much easier. The next step is to fix an issue in the engine and > then submit unless there is a bigger issue with this. >> class foo { >> function __isset() { >> return true; >> } >> function __get() { >> return "hello world"; >> } >> function __call() { >> } >> } >> $foo = new foo; >> $foo->foobar(); >> will first execute __isset(), then __get() and then __call()? That is >> a major backwards compatibility break, and increases the inconsistency >> and decreases readability 10times >> -Hannes > Best regards, > Marcus Best regards, Marcus

Index: Zend/zend_closures.c =================================================================== RCS file: /repository/ZendEngine2/zend_closures.c,v retrieving revision 1.3.2.17 diff -u -p -d -r1.3.2.17 zend_closures.c --- Zend/zend_closures.c 31 Dec 2008 11:15:31 -0000 1.3.2.17 +++ Zend/zend_closures.c 1 Jan 2009 16:02:14 -0000 @@ -14,6 +14,7 @@ +----------------------------------------------------------------------+ | Authors: Christian Seiler <chris_se@gmx.net> | | Dmitry Stogov <dmitry@zend.com> | + | Marcus Boerger <helly@php.net> | +----------------------------------------------------------------------+ */ @@ -22,6 +23,7 @@ #include "zend.h" #include "zend_API.h" #include "zend_closures.h" +#include "zend_exceptions.h" #include "zend_interfaces.h" #include "zend_objects.h" #include "zend_objects_API.h" @@ -222,6 +224,59 @@ int zend_closure_get_closure(zval *obj, } /* }}} */ +ZEND_API HashTable *zend_closure_get_debug_info(zval *object, int *is_temp TSRMLS_DC) /* {{{ */ +{ + zend_closure *closure = (zend_closure *)zend_object_store_get_object(object TSRMLS_CC); + HashTable *rv; + zval *val; + struct _zend_arg_info *arg_info = closure->func.common.arg_info; + + *is_temp = 1; + ALLOC_HASHTABLE(rv); + zend_hash_init(rv, 1, NULL, ZVAL_PTR_DTOR, 0); + val = closure->this_ptr; + if (!val) { + ALLOC_INIT_ZVAL(val); + } else { + Z_ADDREF_P(val); + } + zend_symtable_update(rv, "this", sizeof("this"), (void *) &val, sizeof(zval *), NULL); + if (closure->func.type == ZEND_USER_FUNCTION && closure->func.op_array.static_variables) { + HashTable *static_variables = closure->func.op_array.static_variables; + MAKE_STD_ZVAL(val); + array_init(val); + zend_hash_copy(Z_ARRVAL_P(val), static_variables, (copy_ctor_func_t)zval_add_ref, NULL, sizeof(zval*)); + zend_symtable_update(rv, "static", sizeof("static"), (void *) &val, sizeof(zval *), NULL); + } + + if (arg_info) { + MAKE_STD_ZVAL(val); + array_init(val); + zend_uint i, required = closure->func.common.required_num_args; + for (i = 0; i < closure->func.common.num_args; i++) { + char *name, *info; + int name_len, info_len; + if (arg_info->name) { + name_len = zend_spprintf(&name, 0, "%s$%s", + arg_info->pass_by_reference ? "&" : "", + arg_info->name); + } else { + name_len = zend_spprintf(&name, 0, "%s$param%d", + arg_info->pass_by_reference ? "&" : "", + i + 1); + } + info_len = zend_spprintf(&info, 0, "%s", + i >= required ? "<optional>" : "<required>"); + add_assoc_stringl_ex(val, name, name_len + 1, info, info_len, 0); + efree(name); + arg_info++; + } + zend_symtable_update(rv, "parameter", sizeof("parameter"), (void *) &val, sizeof(zval *), NULL); + } + return rv; +} +/* }}} */ + void zend_register_closure_ce(TSRMLS_D) /* {{{ */ { zend_class_entry ce; @@ -243,6 +298,7 @@ void zend_register_closure_ce(TSRMLS_D) closure_handlers.unset_property = zend_closure_unset_property; closure_handlers.compare_objects = zend_closure_compare_objects; closure_handlers.clone_obj = NULL; + closure_handlers.get_debug_info = zend_closure_get_debug_info; closure_handlers.get_closure = zend_closure_get_closure; } /* }}} */ Index: Zend/zend_object_handlers.c =================================================================== RCS file: /repository/ZendEngine2/zend_object_handlers.c,v retrieving revision 1.135.2.6.2.22.2.23 diff -u -p -d -r1.135.2.6.2.22.2.23 zend_object_handlers.c --- Zend/zend_object_handlers.c 31 Dec 2008 11:15:32 -0000 1.135.2.6.2.22.2.23 +++ Zend/zend_object_handlers.c 1 Jan 2009 16:02:14 -0000 @@ -791,6 +791,24 @@ static union _zend_function *zend_std_ge zobj = Z_OBJ_P(object); if (zend_hash_find(&zobj->ce->function_table, lc_method_name, method_len+1, (void **)&fbc) == FAILURE) { + zval **callable, member; + zend_property_info *property_info; + + ZVAL_STRINGL(&member, method_name, method_len, 0); + property_info = zend_get_property_info(zobj->ce, &member, 1 TSRMLS_CC); + + if (property_info && zend_hash_quick_find(zobj->properties, property_info->name, property_info->name_length+1, property_info->h, (void **) &callable) == SUCCESS) { + zval *callable_obj; + zend_class_entry *ce_ptr; + + if (Z_TYPE_PP(callable) == IS_OBJECT + && Z_OBJ_HANDLER_PP(callable, get_closure) + && Z_OBJ_HANDLER_PP(callable, get_closure)(*callable, &ce_ptr, &fbc, &callable_obj TSRMLS_CC) == SUCCESS) { + *object_ptr = callable_obj; + free_alloca(lc_method_name, use_heap); + return fbc; + } + } free_alloca(lc_method_name, use_heap); if (zobj->ce->__call) { zend_internal_function *call_user_call = emalloc(sizeof(zend_internal_function)); Index: Zend/tests/closure_020.phpt =================================================================== RCS file: /repository/ZendEngine2/tests/closure_020.phpt,v retrieving revision 1.1.2.1 diff -u -p -d -r1.1.2.1 closure_020.phpt --- Zend/tests/closure_020.phpt 14 Jul 2008 13:36:40 -0000 1.1.2.1 +++ Zend/tests/closure_020.phpt 1 Jan 2009 16:02:15 -0000 @@ -27,7 +27,40 @@ object(foo)#%d (%d) { ["test":"foo":private]=> int(3) ["a"]=> - object(Closure)#%d (0) { + object(Closure)#%d (2) { + ["this"]=> + object(foo)#%d (2) { + ["test":"foo":private]=> + int(3) + ["a"]=> + object(Closure)#%d (2) { + ["this"]=> + *RECURSION* + ["static"]=> + array(1) { + ["a"]=> + *RECURSION* + } + } + } + ["static"]=> + array(1) { + ["a"]=> + &object(foo)#%d (2) { + ["test":"foo":private]=> + int(3) + ["a"]=> + object(Closure)#%d (2) { + ["this"]=> + *RECURSION* + ["static"]=> + array(1) { + ["a"]=> + *RECURSION* + } + } + } + } } } bool(true) Index: Zend/tests/closure_026.phpt =================================================================== RCS file: /repository/ZendEngine2/tests/closure_026.phpt,v retrieving revision 1.1.2.2 diff -u -p -d -r1.1.2.2 closure_026.phpt --- Zend/tests/closure_026.phpt 28 Jul 2008 14:10:00 -0000 1.1.2.2 +++ Zend/tests/closure_026.phpt 1 Jan 2009 16:02:15 -0000 @@ -32,7 +32,18 @@ object(foo)#%d (1) { ["a"]=> array(1) { [0]=> - object(Closure)#%d (0) { + object(Closure)#%d (1) { + ["this"]=> + object(foo)#%d (1) { + ["a"]=> + array(1) { + [0]=> + object(Closure)#%d (1) { + ["this"]=> + *RECURSION* + } + } + } } } } @@ -41,7 +52,21 @@ int(1) string(1) "a" array(1) { [0]=> - object(Closure)#%d (0) { + object(Closure)#%d (1) { + ["this"]=> + object(foo)#%d (1) { + ["a"]=> + array(1) { + [0]=> + object(Closure)#%d (1) { + ["this"]=> + object(foo)#%d (1) { + ["a"]=> + *RECURSION* + } + } + } + } } } int(1) Index: Zend/tests/closure_032.phpt =================================================================== RCS file: /repository/ZendEngine2/tests/closure_032.phpt,v retrieving revision 1.1.2.2 diff -u -p -d -r1.1.2.2 closure_032.phpt --- Zend/tests/closure_032.phpt 3 Nov 2008 19:28:32 -0000 1.1.2.2 +++ Zend/tests/closure_032.phpt 1 Jan 2009 16:02:15 -0000 @@ -53,6 +53,12 @@ Array ( [0] => Closure Object ( + [this] => + [parameter] => Array + ( + [$param] => <required> + ) + ) ) Index: Zend/tests/closure_033.phpt =================================================================== RCS file: Zend/tests/closure_033.phpt diff -N Zend/tests/closure_033.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_033.phpt 1 Jan 2009 16:02:15 -0000 @@ -0,0 +1,122 @@ +--TEST-- +Closure 033: var_dump() of a Closure +--FILE-- +<?php + +$outer = 25; + +class Test { + public $func1; + public $var = 42; + function __construct() { + global $outer; + $this->func1 = function($param, $other = "default") use ($outer) { + }; + } +} + +$o = new Test; +var_dump($o->func1); + +$o->func2 = function($param, $other = "default") use ($outer) { +}; + +var_dump($o->func2); + +$func3 = function($param, $other = "default") use ($outer) { +}; + +var_dump($func3); + +?> +===DONE=== +--EXPECTF-- +object(Closure)#%d (3) { + ["this"]=> + object(Test)#%d (2) { + ["func1"]=> + object(Closure)#%d (3) { + ["this"]=> + object(Test)#%d (2) { + ["func1"]=> + object(Closure)#%d (3) { + ["this"]=> + *RECURSION* + ["static"]=> + array(1) { + ["outer"]=> + int(25) + } + ["parameter"]=> + array(2) { + ["$param"]=> + string(10) "<required>" + ["$other"]=> + string(10) "<optional>" + } + } + ["var"]=> + int(42) + } + ["static"]=> + array(1) { + ["outer"]=> + int(25) + } + ["parameter"]=> + array(2) { + ["$param"]=> + string(10) "<required>" + ["$other"]=> + string(10) "<optional>" + } + } + ["var"]=> + int(42) + } + ["static"]=> + array(1) { + ["outer"]=> + int(25) + } + ["parameter"]=> + array(2) { + ["$param"]=> + string(10) "<required>" + ["$other"]=> + string(10) "<optional>" + } +} +object(Closure)#%d (3) { + ["this"]=> + NULL + ["static"]=> + array(1) { + ["outer"]=> + &int(25) + } + ["parameter"]=> + array(2) { + ["$param"]=> + string(10) "<required>" + ["$other"]=> + string(10) "<optional>" + } +} +object(Closure)#%d (3) { + ["this"]=> + NULL + ["static"]=> + array(1) { + ["outer"]=> + int(25) + } + ["parameter"]=> + array(2) { + ["$param"]=> + string(10) "<required>" + ["$other"]=> + string(10) "<optional>" + } +} +===DONE=== Index: Zend/tests/closure_034.phpt =================================================================== RCS file: Zend/tests/closure_034.phpt diff -N Zend/tests/closure_034.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_034.phpt 1 Jan 2009 16:02:15 -0000 @@ -0,0 +1,45 @@ +--TEST-- +Closure 034: Calling property Closure +--FILE-- +<?php + +class Test { + public $func; + function __construct() { + $this->func = function() { + echo __METHOD__ . "()\n"; + }; + } +} + +$o = new Test; +ReflectionProperty::export($o, 'func'); +var_dump($o->func); +$f = $o->func; +$f(); +$o->func(); + +?> +===DONE=== +--EXPECTF-- +Property [ <default> public $func ] + +object(Closure)#%d (1) { + ["this"]=> + object(Test)#%d (1) { + ["func"]=> + object(Closure)#%d (1) { + ["this"]=> + object(Test)#%d (1) { + ["func"]=> + object(Closure)#%d (1) { + ["this"]=> + *RECURSION* + } + } + } + } +} +Test::{closure}() +Test::{closure}() +===DONE=== Index: Zend/tests/closure_035.phpt =================================================================== RCS file: Zend/tests/closure_035.phpt diff -N Zend/tests/closure_035.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_035.phpt 1 Jan 2009 16:02:15 -0000 @@ -0,0 +1,46 @@ +--TEST-- +Closure 035: Calling property supporting __invoke +--FILE-- +<?php + +class Curry +{ + protected $callable; + protected $args; + + public static function create($callable) + { + $curry = new self($callable, array_slice(func_get_args(), 1)); + return $curry; + } + + protected function __construct($callable, $args) + { + $this->callable = $callable; + $this->args = $args; + } + + public function __invoke() + { + return call_user_func_array($this->callable, array_merge($this->args, func_get_args())); + } +} + +$d = new DateTime(); +$getAtom = Curry::create(array($d, 'format'), DATE_ATOM); +var_dump(is_Callable($getAtom)); +var_dump($getAtom()); + +$d = new DateTime(); +$d->getAtom = Curry::create(array($d, 'format'), DATE_ATOM); +var_dump(is_Callable($d->getAtom)); +var_dump($d->getAtom()); + +?> +===DONE=== +--EXPECTF-- +bool(true) +string(%d) "2%d-%d-%dT%d:%d:%d%s" +bool(true) +string(%d) "2%d-%d-%dT%d:%d:%d%s" +===DONE=== Index: Zend/tests/closure_036.phpt =================================================================== RCS file: Zend/tests/closure_036.phpt diff -N Zend/tests/closure_036.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_036.phpt 1 Jan 2009 16:02:15 -0000 @@ -0,0 +1,43 @@ +--TEST-- +Closure 036: Calling dynamic property Closure (not supported) +--FILE-- +<?php + +class Test { + public $methods = array(); + function __isset($name) { + echo __METHOD__ . "($name)\n"; + return isset($this->methods[$name]); + } + function __get($name) { + echo __METHOD__ . "($name)\n"; + return $this->methods[$name]; + } + function __set($name, $data) { + echo __METHOD__ . "($name)\n"; + $this->methods[$name] = $data; + } + function __toString() { + return __CLASS__; + } +} + +$o = new Test; +var_dump(isset($o->func)); +$o->func = function() { + echo __METHOD__ . "(" . var_export($this, true) . ")\n"; +}; +var_dump(isset($o->func)); +$o->func(); +var_dump($o->func); + +?> +===DONE=== +--EXPECTF-- +Test::__isset(func) +bool(false) +Test::__set(func) +Test::__isset(func) +bool(true) + +Fatal error: Call to undefined method Test::func() in %sclosure_036.php on line %d