reflection: try to avoid manual string comparison

[DanielEScherzer]

In is_closure_invoke() compare with the well known zend_string ZEND_STR_MAGIC_INVOKE rather than the underlying literal value.

[TimWolla]

There is another one in

php-src/ext/reflection/php_reflection.c

Lines 3247 to 3248 in 459fc9d

	if (ce == zend_ce_closure && orig_obj && (method_name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1)
	&& memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0

and also some in Zend/zend_builtin_functions.c, can you fix those as well?

[DanielEScherzer]

There is another one in

php-src/ext/reflection/php_reflection.c

Lines 3247 to 3248 in 459fc9d

	if (ce == zend_ce_closure && orig_obj && (method_name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1)
	&& memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0

and also some in Zend/zend_builtin_functions.c, can you fix those as well?

So I tried to fix the other case you found in reflection, but that one is really complicated, since the source being compared is also a char array, and its not clear how to get a zend_string in the case when the char array was extracted from the broader string of SomeClass::someMethod.

I addressed the builtin functions, but I'll note that using zend_string_equals_ci probably doesn't help much (and, it might even be worse since two extra ZSTR_LEN calls and an extra ZSTR_VAL call are used, while zend_string_equals_literal_ci uses the literal values and sizeof which can probably be evaluated by the compiler rather than waiting until runtime)

[TimWolla]

So I tried to fix the other case you found in reflection, but that one is really complicated,

I think that one could be:

zend_string_equals_cstr(ZSTR_KNOWN(ZEND_STR_MAGIC_INVOKE), lcname, method_name_len)

That would at least hide away the length comparison and the memcmp in a function, making it a little less verbose.

[TimWolla]

it might even be worse since two extra ZSTR_LEN calls and an extra ZSTR_VAL call are used

FWIW: Those are not calls. Those are just macros hiding a field access on the struct.

[TimWolla]

There's another use of ZEND_INVOKE_FUNC_NAME in:

php-src/Zend/zend_closures.c

Line 513 in 0006522

if (zend_string_equals_literal_ci(method, ZEND_INVOKE_FUNC_NAME)) {

Overall this PR LGTM though.

I suggest to retitle it to something like “Compare strings with ZEND_STR_MAGIC_INVOKE rather than ZEND_INVOKE_FUNC_NAME” (or whatever), since you are not just touching reflection here.

Feel free to merge when you're happy without another review pass.

[Girgias]

@DanielEScherzer is this PR still relevant now that #21402 has been merged?