From: Stanislav Malyshev Date: Wed, 7 Sep 2011 06:46:27 +0000 (+0000) Subject: Commit Gustavo's closure rebinding patch as desided by vote X-Git-Tag: php-5.5.0alpha1~1254 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a447acdcc6f12ea3a5dcd22416cb033e62995935;p=php Commit Gustavo's closure rebinding patch as desided by vote --- diff --git a/Zend/tests/closure_036.phpt b/Zend/tests/closure_036.phpt index 0f8ccb1eda..77d7ce6906 100755 --- a/Zend/tests/closure_036.phpt +++ b/Zend/tests/closure_036.phpt @@ -1,5 +1,5 @@ --TEST-- -Closure 036: Rebinding closures +Closure 036: Rebinding closures, keep calling scope --FILE-- getIncrementor(); $cb = $ca->bindTo($b); -$cb2 = Closure::bind($b, $ca); +$cb2 = Closure::bind($ca, $b); var_dump($ca()); var_dump($cb()); diff --git a/Zend/tests/closure_038.phpt b/Zend/tests/closure_038.phpt new file mode 100644 index 0000000000..fef0734786 --- /dev/null +++ b/Zend/tests/closure_038.phpt @@ -0,0 +1,58 @@ +--TEST-- +Closure 038: Rebinding closures, change scope, different runtime type +--FILE-- +x = $v; + } + + public function getIncrementor() { + return function() { return ++$this->x; }; + } +} +class B extends A { + private $x; + public function __construct($v) { + parent::__construct($v); + $this->x = $v*2; + } +} + +$a = new A(0); +$b = new B(10); + +$ca = $a->getIncrementor(); +var_dump($ca()); + +echo "Testing with scope given as object", "\n"; + +$cb = $ca->bindTo($b, $b); +$cb2 = Closure::bind($ca, $b, $b); +var_dump($cb()); +var_dump($cb2()); + +echo "Testing with scope as string", "\n"; + +$cb = $ca->bindTo($b, 'B'); +$cb2 = Closure::bind($ca, $b, 'B'); +var_dump($cb()); +var_dump($cb2()); + +$cb = $ca->bindTo($b, NULL); +var_dump($cb()); + +?> +--EXPECTF-- +int(1) +Testing with scope given as object +int(21) +int(22) +Testing with scope as string +int(23) +int(24) + +Fatal error: Cannot access private property B::$x in %s on line %d diff --git a/Zend/tests/closure_039.phpt b/Zend/tests/closure_039.phpt new file mode 100644 index 0000000000..c6a727c0fa --- /dev/null +++ b/Zend/tests/closure_039.phpt @@ -0,0 +1,58 @@ +--TEST-- +Closure 039: Rebinding closures, change scope, same runtime type +--FILE-- +x = $v; + } + + public function getIncrementor() { + return function() { return ++$this->x; }; + } +} +class B extends A { + private $x; + public function __construct($v) { + parent::__construct($v); + $this->x = $v*2; + } +} + +$a = new B(-5); +$b = new B(10); + +$ca = $a->getIncrementor(); +var_dump($ca()); + +echo "Testing with scope given as object", "\n"; + +$cb = $ca->bindTo($b, $b); +$cb2 = Closure::bind($ca, $b, $b); +var_dump($cb()); +var_dump($cb2()); + +echo "Testing with scope as string", "\n"; + +$cb = $ca->bindTo($b, 'B'); +$cb2 = Closure::bind($ca, $b, 'B'); +var_dump($cb()); +var_dump($cb2()); + +$cb = $ca->bindTo($b, NULL); +var_dump($cb()); + +?> +--EXPECTF-- +int(-4) +Testing with scope given as object +int(21) +int(22) +Testing with scope as string +int(23) +int(24) + +Fatal error: Cannot access private property B::$x in %s on line %d diff --git a/Zend/tests/closure_040.phpt b/Zend/tests/closure_040.phpt new file mode 100644 index 0000000000..a1b6cd604a --- /dev/null +++ b/Zend/tests/closure_040.phpt @@ -0,0 +1,45 @@ +--TEST-- +Closure 040: Rebinding closures, bad arguments +--FILE-- +x = $v; + } + + public function getIncrementor() { + return function() { return ++$this->x; }; + } + public function getStaticIncrementor() { + return static function() { return ++static::$xs; }; + } +} + +$a = new A(20); + +$ca = $a->getIncrementor(); +$cas = $a->getStaticIncrementor(); + +$ca->bindTo($a, array()); +$ca->bindTo(array(), 'A'); +$ca->bindTo($a, array(), ""); +$ca->bindTo(); +$cas->bindTo($a, 'A'); + +?> +--EXPECTF-- +Notice: Array to string conversion in %s on line %d + +Warning: Class 'Array' not found in %s on line %d + +Warning: Closure::bindTo() expects parameter 1 to be object, array given in %s on line 25 + +Warning: Closure::bindTo() expects at most 2 parameters, 3 given in %s on line %d + +Warning: Closure::bindTo() expects at least 1 parameter, 0 given in %s on line %d + +Warning: Cannot bind an instance to a static closure in %s on line %d diff --git a/Zend/tests/closure_041.phpt b/Zend/tests/closure_041.phpt new file mode 100644 index 0000000000..79cf9f3380 --- /dev/null +++ b/Zend/tests/closure_041.phpt @@ -0,0 +1,106 @@ +--TEST-- +Closure 041: Rebinding: preservation of previous scope when not given as arg unless impossible +--FILE-- +getStaticClosure(); +$nonstaticScoped = $a->getClosure(); + +echo "Before binding", "\n"; +$staticUnscoped(); echo "\n"; +$nonstaticUnscoped(); echo "\n"; +$staticScoped(); echo "\n"; +$nonstaticScoped(); echo "\n"; + +echo "After binding, no instance", "\n"; +$d = $staticUnscoped->bindTo(null); $d(); echo "\n"; +$d = $nonstaticUnscoped->bindTo(null); $d(); echo "\n"; +$d = $staticScoped->bindTo(null); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(null); $d(); echo "\n"; +//$d should have been turned to static +$d->bindTo($d); + +echo "After binding, with same-class instance for the bound ones", "\n"; +$d = $staticUnscoped->bindTo(new A); $d(); echo "\n"; +$d = $nonstaticUnscoped->bindTo(new A); $d(); echo " (should be scoped to dummy class)\n"; +$d = $staticScoped->bindTo(new A); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(new A); $d(); echo "\n"; + +echo "After binding, with different instance for the bound ones", "\n"; +$d = $nonstaticUnscoped->bindTo(new B); $d(); echo " (should be scoped to dummy class)\n"; +$d = $nonstaticScoped->bindTo(new B); $d(); echo "\n"; + +echo "Done.\n"; + +--EXPECTF-- +Before binding +scoped to A: bool(false) +bound: no +scoped to A: bool(false) +bound: no +scoped to A: bool(true) +bound: no +scoped to A: bool(true) +bound: A +After binding, no instance +scoped to A: bool(false) +bound: no +scoped to A: bool(false) +bound: no +scoped to A: bool(true) +bound: no +scoped to A: bool(true) +bound: no + +Warning: Cannot bind an instance to a static closure in %s on line %d +After binding, with same-class instance for the bound ones + +Warning: Cannot bind an instance to a static closure in %s on line %d +scoped to A: bool(false) +bound: no +scoped to A: bool(false) +bound: A (should be scoped to dummy class) + +Warning: Cannot bind an instance to a static closure in %s on line %d +scoped to A: bool(true) +bound: no +scoped to A: bool(true) +bound: A +After binding, with different instance for the bound ones +scoped to A: bool(false) +bound: B (should be scoped to dummy class) +scoped to A: bool(true) +bound: B +Done. \ No newline at end of file diff --git a/Zend/tests/closure_042.phpt b/Zend/tests/closure_042.phpt new file mode 100644 index 0000000000..8969765bcd --- /dev/null +++ b/Zend/tests/closure_042.phpt @@ -0,0 +1,29 @@ +--TEST-- +Closure 042: Binding an instance to a non-scoped non-static closures gives it a dummy scope +--SKIPIF-- + +--FILE-- +bindTo(new stdClass); +$d(); +$rm = new ReflectionFunction($d); +var_dump($rm->getClosureScopeClass()->name); //dummy sope is Closure + +//should have the same effect +$d = $c->bindTo(new stdClass, NULL); +$d(); +$rm = new ReflectionFunction($d); +var_dump($rm->getClosureScopeClass()->name); //dummy sope is Closure + +echo "Done.\n"; + +--EXPECTF-- +object(stdClass)#%d (0) { +} +string(7) "Closure" +object(stdClass)#%d (0) { +} +string(7) "Closure" +Done. diff --git a/Zend/tests/closure_043.phpt b/Zend/tests/closure_043.phpt new file mode 100644 index 0000000000..98c88fda39 --- /dev/null +++ b/Zend/tests/closure_043.phpt @@ -0,0 +1,86 @@ +--TEST-- +Closure 043: Scope/bounding combination invariants; static closures +--FILE-- +bindTo(null, null); $d(); echo "\n"; +$d = $staticScoped->bindTo(null, null); $d(); echo "\n"; + +echo "After binding, null scope, with instance", "\n"; +$d = $staticUnscoped->bindTo(new A, null); $d(); echo "\n"; +$d = $staticScoped->bindTo(new A, null); $d(); echo "\n"; + +echo "After binding, with scope, no instance", "\n"; +$d = $staticUnscoped->bindTo(null, 'A'); $d(); echo "\n"; +$d = $staticScoped->bindTo(null, 'A'); $d(); echo "\n"; + +echo "After binding, with scope, with instance", "\n"; +$d = $staticUnscoped->bindTo(new A, 'A'); $d(); echo "\n"; +$d = $staticScoped->bindTo(new A, 'A'); $d(); echo "\n"; + +echo "Done.\n"; + +--EXPECTF-- +Before binding +bool(false) +bool(false) + +bool(true) +bool(false) + +After binding, null scope, no instance +bool(false) +bool(false) + +bool(false) +bool(false) + +After binding, null scope, with instance + +Warning: Cannot bind an instance to a static closure in %s on line %d +bool(false) +bool(false) + + +Warning: Cannot bind an instance to a static closure in %s on line %d +bool(false) +bool(false) + +After binding, with scope, no instance +bool(true) +bool(false) + +bool(true) +bool(false) + +After binding, with scope, with instance + +Warning: Cannot bind an instance to a static closure in %s on line %d +bool(true) +bool(false) + + +Warning: Cannot bind an instance to a static closure in %s on line %d +bool(true) +bool(false) + +Done. diff --git a/Zend/tests/closure_044.phpt b/Zend/tests/closure_044.phpt new file mode 100644 index 0000000000..d2644c0401 --- /dev/null +++ b/Zend/tests/closure_044.phpt @@ -0,0 +1,78 @@ +--TEST-- +Closure 044: Scope/bounding combination invariants; non static closures +--FILE-- +getClosure(); + +echo "Before binding", "\n"; +$nonstaticUnscoped(); echo "\n"; +$nonstaticScoped(); echo "\n"; + +echo "After binding, null scope, no instance", "\n"; +$d = $nonstaticUnscoped->bindTo(null, null); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(null, null); $d(); echo "\n"; + +echo "After binding, null scope, with instance", "\n"; +$d = $nonstaticUnscoped->bindTo(new A, null); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(new A, null); $d(); echo "\n"; + +echo "After binding, with scope, no instance", "\n"; +$d = $nonstaticUnscoped->bindTo(null, 'A'); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(null, 'A'); $d(); echo "\n"; + +echo "After binding, with scope, with instance", "\n"; +$d = $nonstaticUnscoped->bindTo(new A, 'A'); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(new A, 'A'); $d(); echo "\n"; + +echo "Done.\n"; + +--EXPECTF-- +Before binding +bool(false) +bool(false) + +bool(true) +bool(true) + +After binding, null scope, no instance +bool(false) +bool(false) + +bool(false) +bool(false) + +After binding, null scope, with instance +bool(false) +bool(true) + +bool(false) +bool(true) + +After binding, with scope, no instance +bool(true) +bool(false) + +bool(true) +bool(false) + +After binding, with scope, with instance +bool(true) +bool(true) + +bool(true) +bool(true) + +Done. diff --git a/Zend/tests/closure_045.phpt b/Zend/tests/closure_045.phpt new file mode 100644 index 0000000000..4115691761 --- /dev/null +++ b/Zend/tests/closure_045.phpt @@ -0,0 +1,19 @@ +--TEST-- +Closure 045: Closures created in static methods are static, even without the keyword +--FILE-- +bindTo(new A); + +echo "Done.\n"; + +--EXPECTF-- +Warning: Cannot bind an instance to a static closure in %s on line %d +Done. diff --git a/Zend/tests/closure_046.phpt b/Zend/tests/closure_046.phpt new file mode 100644 index 0000000000..30a9d3e1b1 --- /dev/null +++ b/Zend/tests/closure_046.phpt @@ -0,0 +1,70 @@ +--TEST-- +Closure 046: Rebinding: preservation of previous scope when "static" given as scope arg (same as closure #041) +--FILE-- +getClosure(); + +echo "Before binding", "\n"; +$nonstaticUnscoped(); echo "\n"; +$nonstaticScoped(); echo "\n"; + +echo "After binding, no instance", "\n"; +$d = $nonstaticUnscoped->bindTo(null, "static"); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(null, "static"); $d(); echo "\n"; +//$d should have been turned to static +$d->bindTo($d); + +echo "After binding, with same-class instance for the bound one", "\n"; +$d = $nonstaticUnscoped->bindTo(new A, "static"); $d(); echo "\n"; +$d = $nonstaticScoped->bindTo(new A, "static"); $d(); echo "\n"; + +echo "After binding, with different instance for the bound one", "\n"; +$d = $nonstaticScoped->bindTo(new B, "static"); $d(); echo "\n"; + +echo "Done.\n"; + +--EXPECTF-- +Before binding +bool(false) +bool(false) + +bool(true) +bool(true) + +After binding, no instance +bool(false) +bool(false) + +bool(true) +bool(false) + + +Warning: Cannot bind an instance to a static closure in %s on line %d +After binding, with same-class instance for the bound one +bool(false) +bool(true) + +bool(true) +bool(true) + +After binding, with different instance for the bound one +bool(true) +bool(true) + +Done. diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 3ea3eba783..46f9c67036 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -76,39 +76,67 @@ ZEND_METHOD(Closure, __invoke) /* {{{ */ } /* }}} */ -/* {{{ proto Closure Closure::bindTo(object $to) - Bind a closure to another object */ -ZEND_METHOD(Closure, bindTo) /* {{{ */ +/* {{{ proto Closure Closure::bind(Closure $old, object $to [, mixed $scope = "static" ] ) + Create a closure from another one and bind to another object and scope */ +ZEND_METHOD(Closure, bind) /* {{{ */ { - zval *newthis; - zend_closure *closure = (zend_closure *)zend_object_store_get_object(getThis() TSRMLS_CC); + zval *newthis, *zclosure, *scope_arg = NULL; + zend_closure *closure; + zend_class_entry *ce, **ce_p; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "o!", &newthis) == FAILURE) { + if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Oo!|z", &zclosure, zend_ce_closure, &newthis, &scope_arg) == FAILURE) { RETURN_NULL(); } - zend_create_closure(return_value, &closure->func, newthis?Z_OBJCE_P(newthis):NULL, newthis TSRMLS_CC); -} -/* }}} */ - -/* {{{ proto Closure Closure::bind(object $to, Closure $old) - Create a closure to with binding to another object */ -ZEND_METHOD(Closure, bind) /* {{{ */ -{ - zval *newthis, *zclosure; - zend_closure *closure; + closure = (zend_closure *)zend_object_store_get_object(zclosure TSRMLS_CC); - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "o!O", &newthis, &zclosure, zend_ce_closure) == FAILURE) { - RETURN_NULL(); + if ((newthis != NULL) && (closure->func.common.fn_flags & ZEND_ACC_STATIC)) { + zend_error(E_WARNING, "Cannot bind an instance to a static closure"); } - closure = (zend_closure *)zend_object_store_get_object(zclosure TSRMLS_CC); + if (scope_arg != NULL) { /* scope argument was given */ + if (IS_ZEND_STD_OBJECT(*scope_arg)) { + ce = Z_OBJCE_P(scope_arg); + } else if (Z_TYPE_P(scope_arg) == IS_NULL) { + ce = NULL; + } else { + char *class_name; + int class_name_len; + zval tmp_zval; + INIT_ZVAL(tmp_zval); + + if (Z_TYPE_P(scope_arg) == IS_STRING) { + class_name = Z_STRVAL_P(scope_arg); + class_name_len = Z_STRLEN_P(scope_arg); + } else { + tmp_zval = *scope_arg; + zval_copy_ctor(&tmp_zval); + convert_to_string(&tmp_zval); + class_name = Z_STRVAL(tmp_zval); + class_name_len = Z_STRLEN(tmp_zval); + } + + if ((class_name_len == sizeof("static") - 1) && + (memcmp("static", class_name, sizeof("static") - 1) == 0)) { + ce = closure->func.common.scope; + } + else if (zend_lookup_class_ex(class_name, class_name_len, NULL, 1, &ce_p TSRMLS_CC) == FAILURE) { + zend_error(E_WARNING, "Class '%s' not found", class_name); + zval_dtor(&tmp_zval); + RETURN_NULL(); + } else { + ce = *ce_p; + } + zval_dtor(&tmp_zval); + } + } else { /* scope argument not given; do not change the scope by default */ + ce = closure->func.common.scope; + } - zend_create_closure(return_value, &closure->func, newthis?Z_OBJCE_P(newthis):NULL, newthis TSRMLS_CC); + zend_create_closure(return_value, &closure->func, ce, newthis TSRMLS_CC); } /* }}} */ - static zend_function *zend_closure_get_constructor(zval *object TSRMLS_DC) /* {{{ */ { zend_error(E_RECOVERABLE_ERROR, "Instantiation of 'Closure' is not allowed"); @@ -356,19 +384,21 @@ ZEND_METHOD(Closure, __construct) } /* }}} */ -ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 0) +ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1) ZEND_ARG_INFO(0, newthis) + ZEND_ARG_INFO(0, newscope) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 0) - ZEND_ARG_INFO(0, newthis) +ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2) ZEND_ARG_INFO(0, closure) + ZEND_ARG_INFO(0, newthis) + ZEND_ARG_INFO(0, newscope) ZEND_END_ARG_INFO() static const zend_function_entry closure_functions[] = { ZEND_ME(Closure, __construct, NULL, ZEND_ACC_PRIVATE) - ZEND_ME(Closure, bindTo, arginfo_closure_bindto, ZEND_ACC_PUBLIC) ZEND_ME(Closure, bind, arginfo_closure_bind, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) + ZEND_MALIAS(Closure, bindTo, bind, arginfo_closure_bindto, ZEND_ACC_PUBLIC) {NULL, NULL, NULL} }; @@ -409,6 +439,12 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure->func = *func; closure->func.common.prototype = NULL; + if ((scope == NULL) && (this_ptr != NULL)) { + /* use dummy scope if we're binding an object without specifying a scope */ + /* maybe it would be better to create one for this purpose */ + scope = zend_ce_closure; + } + if (closure->func.type == ZEND_USER_FUNCTION) { if (closure->func.op_array.static_variables) { HashTable *static_variables = closure->func.op_array.static_variables; @@ -439,6 +475,9 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent } } + /* Invariants: + * If the closure is unscoped, it has no bound object. + * The the closure is scoped, it's either static or it's bound */ closure->func.common.scope = scope; if (scope) { closure->func.common.fn_flags |= ZEND_ACC_PUBLIC; diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 1dd8d4a8a2..628f492a21 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -1670,6 +1670,27 @@ ZEND_METHOD(reflection_function, getClosureThis) } /* }}} */ +/* {{{ proto public ReflectionClass ReflectionFunction::getClosureScopeClass() + Returns the scope associated to the closure */ +ZEND_METHOD(reflection_function, getClosureScopeClass) +{ + reflection_object *intern; + zend_function *fptr; + const zend_function *closure_func; + + if (zend_parse_parameters_none() == FAILURE) { + return; + } + GET_REFLECTION_OBJECT_PTR(fptr); + if (intern->obj) { + closure_func = zend_get_closure_method_def(intern->obj TSRMLS_CC); + if (closure_func && closure_func->common.scope) { + zend_reflection_class_factory(closure_func->common.scope, return_value TSRMLS_CC); + } + } +} +/* }}} */ + /* {{{ proto public mixed ReflectionFunction::getClosure() Returns a dynamically created closure for the function */ ZEND_METHOD(reflection_function, getClosure) @@ -5574,6 +5595,7 @@ static const zend_function_entry reflection_function_abstract_functions[] = { ZEND_ME(reflection_function, isInternal, arginfo_reflection__void, 0) ZEND_ME(reflection_function, isUserDefined, arginfo_reflection__void, 0) ZEND_ME(reflection_function, getClosureThis, arginfo_reflection__void, 0) + ZEND_ME(reflection_function, getClosureScopeClass, arginfo_reflection__void, 0) ZEND_ME(reflection_function, getDocComment, arginfo_reflection__void, 0) ZEND_ME(reflection_function, getEndLine, arginfo_reflection__void, 0) ZEND_ME(reflection_function, getExtension, arginfo_reflection__void, 0) diff --git a/ext/reflection/tests/ReflectionFunction_getClosureScopeClass.phpt b/ext/reflection/tests/ReflectionFunction_getClosureScopeClass.phpt new file mode 100644 index 0000000000..f725dfd088 --- /dev/null +++ b/ext/reflection/tests/ReflectionFunction_getClosureScopeClass.phpt @@ -0,0 +1,31 @@ +--TEST-- +Reflection::getClosureScopeClass() +--SKIPIF-- + +--FILE-- +getClosureScopeClass()); + +Class A { + public static function getClosure() { + return function($param) { return "this is a closure"; }; + } +} + +$closure = A::getClosure(); +$rf = new ReflectionFunction($closure); +var_dump($rf->getClosureScopeClass()); +echo "Done!\n"; +--EXPECTF-- +NULL +object(ReflectionClass)#%d (1) { + ["name"]=> + string(1) "A" +} +Done!