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

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

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 15:05:59 -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 15:05:59 -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) { + if (Z_OBJ_HT_PP(object_ptr)->read_property) { + zval *callable, property, *callable_obj; + zend_class_entry *ce_ptr; + + INIT_PZVAL(&property); + ZVAL_STRINGL(&property, method_name, method_len, 0); + callable = Z_OBJ_HANDLER_PP(object_ptr,read_property)(*object_ptr, &property, BP_VAR_IS TSRMLS_CC); + + if (Z_TYPE_P(callable) == IS_OBJECT + && Z_OBJ_HANDLER_P(callable, get_closure) + && Z_OBJ_HANDLER_P(callable, get_closure)(callable, &ce_ptr, &fbc, &callable_obj TSRMLS_CC) == SUCCESS) { + if (callable_obj) { + *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 15:05:59 -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 15:05:59 -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 15:05:59 -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 15:05:59 -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 15:05:59 -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 15:05:59 -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 15:05:59 -0000 @@ -0,0 +1,49 @@ +--TEST-- +Closure 036: Calling dynamic property Closure +--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__ . "($this)\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) +Test::__get(func) +{closure}(Test) +Test::__get(func) +object(Closure)#%d (1) { + ["this"]=> + NULL +} +===DONE===