Fix ReflectionFunction::isDeprecated() for materialized __call()#17914
Conversation
| call.function_name = mptr->common.function_name; | ||
| call.scope = mptr->common.scope; | ||
| call.doc_comment = NULL; | ||
| call.attributes = mptr->common.attributes; |
There was a problem hiding this comment.
I don't quite understand, why I do not need to increase the refcount here: If I do, I get a leak.
There was a problem hiding this comment.
call isn't a trampoline, and the function call is freed in zend_closure_free_storage. The attributes are ignored by zend_closure_free_storage. So you're counting on that the attributes of the original function outlive the closure instance.
There was a problem hiding this comment.
Now I actually wonder why we refcount the attributes for trampolines in the first place. They always come from a real function, and real functions should outlive trampolines I think. Maybe I'm missing something...
EDIT: I'm running the test suite now with refcounting removed.
|
The deprecated attribute is only since 8.4, but if the patch is going to expose all of the attributes from |
I can do this, but that would require cherry-picking f37b165 and 2542357 (incl. the tests that don't directly apply to 8.3, resulting in messy merges). Therefore I'd rather not do this, because for |
|
@TimWolla I agree, let's keep this in 8.4. |
* PHP-8.4: Fix `ReflectionFunction::isDeprecated()` for materialized `__call()` (#17914)
3 participants
Fixes #17913