-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Let closure created from magic method accept named parameters #11364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
--TEST-- | ||
Trampoline closure created from magic method accepts named arguments | ||
--FILE-- | ||
<?php | ||
|
||
class Test { | ||
public function __call($name, $args) { | ||
var_dump($name, $args); | ||
} | ||
public static function __callStatic($name, $args) { | ||
var_dump($name, $args); | ||
} | ||
} | ||
|
||
$test = new Test; | ||
|
||
echo "-- Non-static cases --\n"; | ||
$test->test(...)(1, 2); | ||
$test->test(...)(1, 2, a: 123, b: $test); | ||
$test->test(...)(a: 123, b: $test); | ||
$test->test(...)(); | ||
|
||
echo "-- Static cases --\n"; | ||
Test::testStatic(1, 2, a: 123); | ||
Test::testStatic(...)(1, 2); | ||
Test::testStatic(...)(1, 2, a: 123, b: $test); | ||
Test::testStatic(...)(a: 123, b: $test); | ||
Test::testStatic(...)(); | ||
|
||
?> | ||
--EXPECT-- | ||
-- Non-static cases -- | ||
string(4) "test" | ||
array(2) { | ||
[0]=> | ||
int(1) | ||
[1]=> | ||
int(2) | ||
} | ||
string(4) "test" | ||
array(4) { | ||
[0]=> | ||
int(1) | ||
[1]=> | ||
int(2) | ||
["a"]=> | ||
int(123) | ||
["b"]=> | ||
object(Test)#1 (0) { | ||
} | ||
} | ||
string(4) "test" | ||
array(2) { | ||
["a"]=> | ||
int(123) | ||
["b"]=> | ||
object(Test)#1 (0) { | ||
} | ||
} | ||
string(4) "test" | ||
array(0) { | ||
} | ||
-- Static cases -- | ||
string(10) "testStatic" | ||
array(3) { | ||
[0]=> | ||
int(1) | ||
[1]=> | ||
int(2) | ||
["a"]=> | ||
int(123) | ||
} | ||
string(10) "testStatic" | ||
array(2) { | ||
[0]=> | ||
int(1) | ||
[1]=> | ||
int(2) | ||
} | ||
string(10) "testStatic" | ||
array(4) { | ||
[0]=> | ||
int(1) | ||
[1]=> | ||
int(2) | ||
["a"]=> | ||
int(123) | ||
["b"]=> | ||
object(Test)#1 (0) { | ||
} | ||
} | ||
string(10) "testStatic" | ||
array(2) { | ||
["a"]=> | ||
int(123) | ||
["b"]=> | ||
object(Test)#1 (0) { | ||
} | ||
} | ||
string(10) "testStatic" | ||
array(0) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -294,7 +294,18 @@ static ZEND_NAMED_FUNCTION(zend_closure_call_magic) /* {{{ */ { | |
fci.params = params; | ||
fci.param_count = 2; | ||
ZVAL_STR(&fci.params[0], EX(func)->common.function_name); | ||
if (ZEND_NUM_ARGS()) { | ||
if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { | ||
zend_string *name; | ||
zval *named_param_zval; | ||
array_init_size(&fci.params[1], ZEND_NUM_ARGS() + zend_hash_num_elements(EX(extra_named_params))); | ||
/* Avoid conversion from packed to mixed later. */ | ||
zend_hash_real_init_mixed(Z_ARRVAL(fci.params[1])); | ||
zend_copy_parameters_array(ZEND_NUM_ARGS(), &fci.params[1]); | ||
ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(EX(extra_named_params), name, named_param_zval) { | ||
Z_TRY_ADDREF_P(named_param_zval); | ||
zend_hash_add_new(Z_ARRVAL(fci.params[1]), name, named_param_zval); | ||
} ZEND_HASH_FOREACH_END(); | ||
} else if (ZEND_NUM_ARGS()) { | ||
array_init_size(&fci.params[1], ZEND_NUM_ARGS()); | ||
zend_copy_parameters_array(ZEND_NUM_ARGS(), &fci.params[1]); | ||
} else { | ||
|
@@ -841,7 +852,7 @@ void zend_closure_from_frame(zval *return_value, zend_execute_data *call) { /* { | |
|
||
memset(&trampoline, 0, sizeof(zend_internal_function)); | ||
trampoline.type = ZEND_INTERNAL_FUNCTION; | ||
trampoline.fn_flags = mptr->common.fn_flags & ZEND_ACC_STATIC; | ||
trampoline.fn_flags = mptr->common.fn_flags & (ZEND_ACC_STATIC | ZEND_ACC_VARIADIC); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed something else, above there is a check for the But that is an aside. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably, feel free to open a PR and check if there's more such cases ;) |
||
trampoline.handler = zend_closure_call_magic; | ||
trampoline.function_name = mptr->common.function_name; | ||
trampoline.scope = mptr->common.scope; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering why we couldn't use the
*named_params
field of the FCI struct, before realising that we need to merge all the parameters into a single array to pass as an argument to the magic method.