Re: [PATCH] Closures and reflection improvements

Hello Christian, I updated your patch for 5.3 due to recent changes. It works pretty fine for me. Care to test again, especially with your new tests? Note that test ext/reflection/tests/closures_001.phpt does not work for me and I have no clue why. Well, besides that obviously the closure class no longer has an entry for __closure, so it is more a question of how to know when to add this. Also, it appears you do not have a cvs account, or did I overlook something? Modified patch attached, including the updated reflection tests. Hello Johannes, I addressed the thing you spotted. best regards marcus Monday, August 4, 2008, 12:59:13 PM, you wrote: > Christian, > On Mon, 2008-08-04 at 11:33 +0400, Dmitry Stogov wrote: >> >> >> >> http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-5.3.patch >> >> >> >> >> >> >> >> http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-6.patch >> >> >> > > without applying and testing the patch: > - zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC, > (apply_func_args_t) _addmethod, 3, &ce, return_value, filter); > + zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC, > (apply_func_args_t) _addmethod, 3, &ce, return_value, filter, intern->obj); > that looks a bit strange, probably you want 4 instead of 3 now? > johannes Best regards, Marcus

Index: Zend/zend_closures.c =================================================================== RCS file: /repository/ZendEngine2/zend_closures.c,v retrieving revision 1.3.2.10 diff -u -p -d -r1.3.2.10 zend_closures.c --- Zend/zend_closures.c 7 Aug 2008 13:35:51 -0000 1.3.2.10 +++ Zend/zend_closures.c 8 Aug 2008 21:33:11 -0000 @@ -26,7 +26,6 @@ #include "zend_objects_API.h" #include "zend_globals.h" -#define ZEND_INVOKE_FUNC_NAME "__invoke" #define ZEND_CLOSURE_PRINT_NAME "Closure object" #define ZEND_CLOSURE_PROPERTY_ERROR() \ @@ -38,7 +37,8 @@ typedef struct _zend_closure { zval *this_ptr; } zend_closure; -static zend_class_entry *zend_ce_closure; +/* non-static since it needs to be referenced */ +zend_class_entry *zend_ce_closure; static zend_object_handlers closure_handlers; ZEND_METHOD(Closure, __invoke) /* {{{ */ @@ -99,6 +99,24 @@ static int zend_closure_compare_objects( } /* }}} */ +ZEND_API zend_function *zend_get_closure_invoke_method(zval *obj TSRMLS_DC) /* {{{ */ +{ + zend_closure *closure = (zend_closure *)zend_object_store_get_object(obj TSRMLS_CC); + zend_function *invoke = (zend_function*)emalloc(sizeof(zend_function)); + + invoke = (zend_function*)emalloc(sizeof(zend_function)); + invoke->common = closure->func.common; + invoke->type = ZEND_INTERNAL_FUNCTION; + invoke->internal_function.handler = ZEND_MN(Closure___invoke); + invoke->internal_function.module = 0; + invoke->internal_function.scope = zend_ce_closure; + invoke->internal_function.function_name = estrndup(ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1); + invoke->internal_function.fn_flags = ZEND_ACC_PUBLIC | ZEND_ACC_CALL_VIA_HANDLER; + return invoke; + +} +/* }}} */ + static zend_function *zend_closure_get_method(zval **object_ptr, char *method_name, int method_len TSRMLS_DC) /* {{{ */ { char *lc_name; @@ -109,18 +127,8 @@ static zend_function *zend_closure_get_m if ((method_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && memcmp(lc_name, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 ) { - zend_closure *closure = (zend_closure *)zend_object_store_get_object(*object_ptr TSRMLS_CC); - zend_function *invoke = (zend_function*)emalloc(sizeof(zend_function)); - - invoke->common = closure->func.common; - invoke->type = ZEND_INTERNAL_FUNCTION; - invoke->internal_function.fn_flags = ZEND_ACC_CALL_VIA_HANDLER; - invoke->internal_function.handler = ZEND_MN(Closure___invoke); - invoke->internal_function.module = 0; - invoke->internal_function.scope = zend_ce_closure; - invoke->internal_function.function_name = estrndup(ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1); free_alloca(lc_name, use_heap); - return invoke; + return zend_get_closure_invoke_method(*object_ptr TSRMLS_CC); } free_alloca(lc_name, use_heap); return NULL; Index: Zend/zend_closures.h =================================================================== RCS file: /repository/ZendEngine2/zend_closures.h,v retrieving revision 1.1.2.2 diff -u -p -d -r1.1.2.2 zend_closures.h --- Zend/zend_closures.h 14 Jul 2008 09:48:59 -0000 1.1.2.2 +++ Zend/zend_closures.h 8 Aug 2008 21:33:11 -0000 @@ -24,10 +24,15 @@ BEGIN_EXTERN_C() +#define ZEND_INVOKE_FUNC_NAME "__invoke" + void zend_register_closure_ce(TSRMLS_D); +extern zend_class_entry *zend_ce_closure; + ZEND_API void zend_create_closure(zval *res, zend_function *op_array, zend_class_entry *scope, zval *this_ptr TSRMLS_DC); ZEND_API int zend_get_closure(zval *obj, zend_class_entry **ce_ptr, zend_function **fptr_ptr, zval **zobj_ptr, zval ***zobj_ptr_ptr TSRMLS_DC); +ZEND_API zend_function *zend_get_closure_invoke_method(zval *obj TSRMLS_DC); END_EXTERN_C() Index: ext/reflection/php_reflection.c =================================================================== RCS file: /repository/php-src/ext/reflection/php_reflection.c,v retrieving revision 1.164.2.33.2.45.2.27 diff -u -p -d -r1.164.2.33.2.45.2.27 php_reflection.c --- ext/reflection/php_reflection.c 8 Aug 2008 10:52:48 -0000 1.164.2.33.2.45.2.27 +++ ext/reflection/php_reflection.c 8 Aug 2008 21:33:12 -0000 @@ -536,6 +536,14 @@ static void _class_string(string *str, z zend_hash_get_current_key_ex(&ce->function_table, &key, &key_len, &num_index, 0, &pos) != HASH_KEY_IS_STRING || zend_binary_strcasecmp(key, key_len-1, mptr->common.function_name, len) == 0) { + zend_function *closure_mptr; + /* see if this is a closure */ + if (ce == zend_ce_closure && obj && (len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && + memcmp(mptr->common.function_name, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && + (closure_mptr = zend_get_closure_invoke_method(obj TSRMLS_CC)) != NULL + ) { + mptr = closure_mptr; + } string_printf(&dyn, "\n"); _function_string(&dyn, mptr, ce, sub_indent.string TSRMLS_CC); count++; @@ -1890,15 +1898,34 @@ ZEND_METHOD(reflection_parameter, __cons convert_to_string_ex(method); lcname_len = Z_STRLEN_PP(method); lcname = zend_str_tolower_dup(Z_STRVAL_PP(method), lcname_len); - if (zend_hash_find(&ce->function_table, lcname, lcname_len + 1, (void **) &fptr) == FAILURE) { + if (ce == zend_ce_closure && Z_TYPE_PP(classref) == IS_OBJECT && + (lcname_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && + memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && + (fptr = zend_get_closure_invoke_method(*classref TSRMLS_CC)) != NULL + ) { + /* do nothing, fptr is already set */ + } else if (zend_hash_find(&ce->function_table, lcname, lcname_len + 1, (void **) &fptr) == FAILURE) { efree(lcname); zend_throw_exception_ex(reflection_exception_ptr, 0 TSRMLS_CC, - "Method %s::%s() does not exist", Z_STRVAL_PP(classref), Z_TYPE_PP(method), Z_STRVAL_PP(method)); + "Method %s::%s() does not exist", ce->name, Z_STRVAL_PP(method)); return; } efree(lcname); } break; + + case IS_OBJECT: { + ce = Z_OBJCE_P(reference); + + if (ce == zend_ce_closure && (fptr = zend_get_closure_invoke_method(reference TSRMLS_CC)) != NULL) { + /* do nothing, fptr is already set */ + } else if (zend_hash_find(&ce->function_table, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME), (void **)&fptr) == FAILURE) { + zend_throw_exception_ex(reflection_exception_ptr, 0 TSRMLS_CC, + "Method %s::%s() does not exist", ce->name, ZEND_INVOKE_FUNC_NAME); + return; + } + } + break; default: _DO_THROW("The parameter class is expected to be either a string or an array(class, method)"); @@ -2207,7 +2234,7 @@ ZEND_METHOD(reflection_method, export) ZEND_METHOD(reflection_method, __construct) { zval *name, *classname; - zval *object; + zval *object, *orig_obj; reflection_object *intern; char *lcname; zend_class_entry **pce; @@ -2217,7 +2244,11 @@ ZEND_METHOD(reflection_method, __constru int name_len, tmp_len; zval ztmp; - if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "zs", &classname, &name_str, &name_len) == FAILURE) { + if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "o", &classname) == SUCCESS) { + name_str = ZEND_INVOKE_FUNC_NAME; + name_len = sizeof(ZEND_INVOKE_FUNC_NAME)-1; + orig_obj = classname; + } else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "zs", &classname, &name_str, &name_len) == FAILURE) { if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &name_str, &name_len) == FAILURE) { return; } @@ -2230,6 +2261,11 @@ ZEND_METHOD(reflection_method, __constru ZVAL_STRINGL(classname, name_str, tmp_len, 1); name_len = name_len - (tmp_len + 2); name_str = tmp + 2; + orig_obj = NULL; + } else if (Z_TYPE_P(classname) == IS_OBJECT) { + orig_obj = classname; + } else { + orig_obj = NULL; } object = getThis(); @@ -2275,7 +2311,12 @@ ZEND_METHOD(reflection_method, __constru lcname = zend_str_tolower_dup(name_str, name_len); - if (zend_hash_find(&ce->function_table, lcname, name_len + 1, (void **) &mptr) == FAILURE) { + if (ce == zend_ce_closure && orig_obj && (name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && + memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && + (mptr = zend_get_closure_invoke_method(orig_obj TSRMLS_CC)) != NULL + ) { + /* do nothing, mptr already set */ + } else if (zend_hash_find(&ce->function_table, lcname, name_len + 1, (void **) &mptr) == FAILURE) { efree(lcname); zend_throw_exception_ex(reflection_exception_ptr, 0 TSRMLS_CC, "Method %s::%s() does not exist", ce->name, name_str); @@ -3139,7 +3180,13 @@ ZEND_METHOD(reflection_class, getMethod) GET_REFLECTION_OBJECT_PTR(ce); lc_name = zend_str_tolower_dup(name, name_len); - if (zend_hash_find(&ce->function_table, lc_name, name_len + 1, (void**) &mptr) == SUCCESS) { + if (ce == zend_ce_closure && intern->obj && (name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && + memcmp(lc_name, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && + (mptr = zend_get_closure_invoke_method(intern->obj TSRMLS_CC)) != NULL + ) { + reflection_method_factory(ce, mptr, return_value TSRMLS_CC); + efree(lc_name); + } else if (zend_hash_find(&ce->function_table, lc_name, name_len + 1, (void**) &mptr) == SUCCESS) { reflection_method_factory(ce, mptr, return_value TSRMLS_CC); efree(lc_name); } else { @@ -3158,9 +3205,18 @@ static int _addmethod(zend_function *mpt zend_class_entry *ce = *va_arg(args, zend_class_entry**); zval *retval = va_arg(args, zval*); long filter = va_arg(args, long); + zval *obj = va_arg(args, zval *); + uint len = strlen(mptr->common.function_name); + zend_function *closure_mptr; if (mptr->common.fn_flags & filter) { ALLOC_ZVAL(method); + if (ce == zend_ce_closure && obj && (len == sizeof(ZEND_INVOKE_FUNC_NAME)-1) && + memcmp(mptr->common.function_name, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && + (closure_mptr = zend_get_closure_invoke_method(obj TSRMLS_CC)) != NULL + ) { + mptr = closure_mptr; + } reflection_method_factory(ce, mptr, method TSRMLS_CC); add_next_index_zval(retval, method); } @@ -3190,7 +3246,7 @@ ZEND_METHOD(reflection_class, getMethods GET_REFLECTION_OBJECT_PTR(ce); array_init(return_value); - zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC, (apply_func_args_t) _addmethod, 3, &ce, return_value, filter); + zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC, (apply_func_args_t) _addmethod, 4, &ce, return_value, filter, intern->obj); } /* }}} */ Index: ext/reflection/tests/closures_001.phpt =================================================================== RCS file: ext/reflection/tests/closures_001.phpt diff -N ext/reflection/tests/closures_001.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ ext/reflection/tests/closures_001.phpt 8 Aug 2008 21:33:12 -0000 @@ -0,0 +1,70 @@ +--TEST-- +Reflection on closures +--FILE-- +<?php + +$closure = function ($a, $b = 0) { }; + +$ro = new ReflectionObject ($closure); +$rm = $ro->getMethod ('__invoke'); +var_dump ($rm->getNumberOfParameters()); +var_dump ($rm->getNumberOfRequiredParameters()); +$rms = $ro->getMethods(); +foreach ($rms as $rm) { + if ($rm->getName () == '__invoke') { + var_dump ($rm->getNumberOfParameters()); + var_dump ($rm->getNumberOfRequiredParameters()); + } +} + +echo "---\n"; + +$rm = new ReflectionMethod ($closure); +var_dump ($rm->getName ()); +var_dump ($rm->getNumberOfParameters()); +var_dump ($rm->getNumberOfRequiredParameters()); + +echo "---\n"; + +$rp = new ReflectionParameter (array ($closure, '__invoke'), 0); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter (array ($closure, '__invoke'), 1); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter (array ($closure, '__invoke'), 'a'); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter (array ($closure, '__invoke'), 'b'); +var_dump ($rp->isOptional ()); + +echo "---\n"; + +$rp = new ReflectionParameter ($closure, 0); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter ($closure, 1); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter ($closure, 'a'); +var_dump ($rp->isOptional ()); +$rp = new ReflectionParameter ($closure, 'b'); +var_dump ($rp->isOptional ()); + +?> +===DONE=== +--EXPECTF-- +int(2) +int(1) +int(2) +int(1) +--- +string(8) "__invoke" +int(2) +int(1) +--- +bool(false) +bool(true) +bool(false) +bool(true) +--- +bool(false) +bool(true) +bool(false) +bool(true) +===DONE=== Index: ext/reflection/tests/closures_002.phpt =================================================================== RCS file: ext/reflection/tests/closures_002.phpt diff -N ext/reflection/tests/closures_002.phpt --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ ext/reflection/tests/closures_002.phpt 8 Aug 2008 21:33:12 -0000 @@ -0,0 +1,25 @@ +--TEST-- +Reflection on invokable objects +--FILE-- +<?php + +$rm = new ReflectionMethod (new C); +var_dump ($rm->getName ()); +var_dump ($rm->getNumberOfParameters()); +var_dump ($rm->getNumberOfRequiredParameters()); + +$rp = new ReflectionParameter (new C, 0); +var_dump ($rp->isOptional ()); + +class C { + function __invoke ($a) { } +} + +?> +===DONE=== +--EXPECTF-- +string(8) "__invoke" +int(1) +int(1) +bool(false) +===DONE===