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

Hello all, current state of the patch is attached. marcus Saturday, January 3, 2009, 3:47:44 PM, you wrote: > Hello Johannes, > Friday, January 2, 2009, 7:16:32 PM, you wrote: >> Hi, >> On Wed, 2008-12-31 at 17:38 +0100, Marcus Boerger wrote: >>> So far it is. Yet I as much as you do not like the inconsistency. So I >>> spend a little bit on providing the following patch that should do >>> what >>> you were looking for. >> I thought bout this issue for one day or so, there are three things to >> consider here: >> - the language design question >> - implementation-related things >> - getting 5.3 out >> For the first question I have to say that this is a major change to >> language. Bringing this feature in makes it a mixture of a class-based >> OO model and JavaScript-like class-less OO model. As I tend to read way >> more code than I write I, personally, prefer the clearer class based >> approach. >> Nonetheless there seems to be large support for this, so let's lake a >> deeper look: >>> The disadvantage: Calling properties is case sensitive while calling >>> methods isn't. But since this has nothign to do with this patch and >>> the patch only increases consistency I am all for applying it. >> That's one of the symptoms of the underlying problem we're having: In >> our object implementation methods and properties are kept completely >> separated. Using properties now like methods will have side effects, one >> is described above. I found another one today: >> <?php >> class A { >> public $p; >> public function __construct() { >> $this->p = function() { echo 1; }; >> } >> } >> class B extends A { >> private function p() { >> echo 2; >> } >> } >> $a = new A(); >> $b = new B(); >> $a->p(); >> $b->p(); ?>>> >> What do you expect? Well, the first call, to $a->p(); works, the second >> one, to $b->p();, doesn't since where accessing the private method >> B::p(). >> $ sapi/cli/php test.php >> 1 >> Fatal error: Call to undefined method B::p() in test.php on line 16 >> Depending on your view on this that result can be correct or wrong, it >> might even be wrong in different ways, maybe overwriting $a->p with >> $b->p should be forbidden, maybe it should treat private elements like >> non-existent ones. >> The obvious thing is that it feels like the "is-a" relationship isn't >> enforced anymore. > First, for properties we already have that anyway. Remember you can add > properties to object instances at any time. So being able to add methods > at any time just reduces one more inconsistency between things. Also > since properties follow a mixture of class based and prototype based > object model we have to respect prototype semantics for the parts where > proptotype model is used. This means when you dynamically add a closure > then this is done in the manner of prototype semantics and needs to follow > it. Thus with a slightly different code the result is perfectly correct: > <?php > class A { > // public $p; > } > class B extends A { > private function p() { > echo 2; > } > } > $a = new A(); > $b = new B(); > $a->p = function() { echo 1; }; > $a->p(); > $b->p(); ?>> > So this raises the question what the status quo without the patch is: > class Test { > public $func; > function __construct() { > $this->func = function() { > echo __METHOD__ . "()\n"; > }; > } > private function func() { > echo __METHOD__ . "()\n"; > } > } > $o = new Test; > $f = $o->func; > $f(); // Test::{closure}() > $o->func(); // Error call to private method... > So apparently we already have the exact issue at hand and we take > precedence in the actual function which in my opinion is a good > choice. And this also is exactly the behavior your example with the > patch applies shows. > To summarize this from my point of view we only get a bit more of > consistency. But for people with a JavaScript background we get a > lot more. As we now support what they are used to and that actually > is a pretty big advantage. > However if that is the intention, then something we might want to look > into in order to make it easier for those people is to fix something > that propbably looks wrong to them. That is binding a closure to a > property outside of the scope of the object does not bind the object > to the this_ptr as it would in case the assignment is done inside the > object scope (a class method). But once again this is somethign to be > fixed whether or not we apply the patch. >> Additionally there are other areas with such conflicts: Reflection >> (ReflectionMethod doesn't work, ReflectionProperty has no hint it's >> executable, ...) and other meta-functionality are obvious, others might >> (and will) be quite hidden. > Reflection now has closure support. >> I guess the only solid way to implement that feature would be by merging >> the property and method tables into an "element" table so we reduce such >> conflicts -- while that's a major engine and language change. Every >> other approach will have side-effects. > That would mean a major change in language and we should not go that route. >> This leads us to the third consideration: 5.3 is around 1.5 years in >> development now with lots of new features. It was announced that we >> wanted to go to a beta status to get it out soon ("release early, >> release often" ...) >> I'd say it would be good to concentrate on making 5.3 stable and then >> see how new features are accepted. If users really demand such a >> functionality, when using closures for "real life" development, we can >> still add it, but that should be done with the time needed to identify >> side-effects and other consequences, not in a rush during holiday season >> after a feature freeze has been announced. > Stable also means stable features and semantics. And that includes trying > to get rid of inconsistencies. > marcus > Best regards, > Marcus Best regards, Marcus

Index: Zend/zend_object_handlers.c =================================================================== RCS file: /repository/ZendEngine2/zend_object_handlers.c,v retrieving revision 1.135.2.6.2.22.2.24 diff -u -p -d -r1.135.2.6.2.22.2.24 zend_object_handlers.c --- Zend/zend_object_handlers.c 3 Jan 2009 17:48:39 -0000 1.135.2.6.2.22.2.24 +++ Zend/zend_object_handlers.c 3 Jan 2009 17:51:24 -0000 @@ -795,6 +795,25 @@ 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) { + /* Calling closures via properties is still disabled */ + 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_property_002.phpt =================================================================== RCS file: Zend/tests/closure_property_002.phpt diff -N Zend/tests/closure_property_002.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_002.phpt 3 Jan 2009 17:51:24 -0000 @@ -0,0 +1,45 @@ +--TEST-- +Closure property 002: 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_property_003.phpt =================================================================== RCS file: Zend/tests/closure_property_003.phpt diff -N Zend/tests/closure_property_003.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_003.phpt 3 Jan 2009 17:51:24 -0000 @@ -0,0 +1,48 @@ +--TEST-- +Closure property 003: Calling property supporting __invoke +--INI-- +date.timezone=UTC +--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_property_004.phpt =================================================================== RCS file: Zend/tests/closure_property_004.phpt diff -N Zend/tests/closure_property_004.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_004.phpt 3 Jan 2009 17:51:24 -0000 @@ -0,0 +1,43 @@ +--TEST-- +Closure property 004: 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_property_004.php on line %d Index: Zend/tests/closure_property_005.phpt =================================================================== RCS file: Zend/tests/closure_property_005.phpt diff -N Zend/tests/closure_property_005.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_005.phpt 3 Jan 2009 17:51:24 -0000 @@ -0,0 +1,31 @@ +--TEST-- +Closure property 005: Calling closure property via variable +--FILE-- +<?php + +class Test { + public $func1; + function __construct() { + $this->func1 = function() { + echo __METHOD__ . "()\n"; + }; + } +} + +$o = new Test; +$n = 'func1'; +$o->$n(); + +$o->func2 = function() { + echo __METHOD__ . "()\n"; +}; + +$n = 'func2'; +$o->$n(); + +?> +===DONE=== +--EXPECTF-- +Test::{closure}() +{closure}() +===DONE=== Index: Zend/zend_object_handlers.c =================================================================== RCS file: /repository/ZendEngine2/zend_object_handlers.c,v retrieving revision 1.209 diff -u -p -d -r1.209 zend_object_handlers.c --- Zend/zend_object_handlers.c 3 Jan 2009 17:46:24 -0000 1.209 +++ Zend/zend_object_handlers.c 3 Jan 2009 17:51:08 -0000 @@ -805,6 +805,25 @@ static union _zend_function *zend_std_ge zobj = Z_OBJ_P(object); if (zend_u_hash_find(&zobj->ce->function_table, type, lc_method_name, lc_method_name_len+1, (void **)&fbc) == FAILURE) { + /* Calling closures via properties is still disabled */ + zval **callable, member; + zend_property_info *property_info; + + ZVAL_TEXTL(&member, method_name, method_len, 0); + property_info = zend_get_property_info(zobj->ce, &member, 1 TSRMLS_CC); + + if (property_info && zend_u_hash_quick_find(zobj->properties, Z_TYPE(member), 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; + efree(lc_method_name.v); + return fbc; + } + } efree(lc_method_name.v); if (zobj->ce->__call) { zend_internal_function *call_user_call = emalloc(sizeof(zend_internal_function)); Index: Zend/tests/closure_property_002.phpt =================================================================== RCS file: Zend/tests/closure_property_002.phpt diff -N Zend/tests/closure_property_002.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_002.phpt 3 Jan 2009 17:51:09 -0000 @@ -0,0 +1,45 @@ +--TEST-- +Closure property 002: 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) { + [u"func"]=> + object(Closure)#%d (1) { + ["this"]=> + object(Test)#%d (1) { + [u"func"]=> + object(Closure)#%d (1) { + ["this"]=> + *RECURSION* + } + } + } + } +} +Test::{closure}() +Test::{closure}() +===DONE=== Index: Zend/tests/closure_property_003.phpt =================================================================== RCS file: Zend/tests/closure_property_003.phpt diff -N Zend/tests/closure_property_003.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_003.phpt 3 Jan 2009 17:51:09 -0000 @@ -0,0 +1,48 @@ +--TEST-- +Closure property 003: Calling property supporting __invoke +--INI-- +date.timezone=UTC +--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) +unicode(%d) "2%d-%d-%dT%d:%d:%d%s" +bool(true) +unicode(%d) "2%d-%d-%dT%d:%d:%d%s" +===DONE=== Index: Zend/tests/closure_property_004.phpt =================================================================== RCS file: Zend/tests/closure_property_004.phpt diff -N Zend/tests/closure_property_004.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_004.phpt 3 Jan 2009 17:51:09 -0000 @@ -0,0 +1,43 @@ +--TEST-- +Closure property 004: 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_property_004.php on line %d Index: Zend/tests/closure_property_005.phpt =================================================================== RCS file: Zend/tests/closure_property_005.phpt diff -N Zend/tests/closure_property_005.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ Zend/tests/closure_property_005.phpt 3 Jan 2009 17:51:09 -0000 @@ -0,0 +1,31 @@ +--TEST-- +Closure property 005: Calling closure property via variable +--FILE-- +<?php + +class Test { + public $func1; + function __construct() { + $this->func1 = function() { + echo __METHOD__ . "()\n"; + }; + } +} + +$o = new Test; +$n = 'func1'; +$o->$n(); + +$o->func2 = function() { + echo __METHOD__ . "()\n"; +}; + +$n = 'func2'; +$o->$n(); + +?> +===DONE=== +--EXPECTF-- +Test::{closure}() +{closure}() +===DONE===