Fix ReflectionFunction::isDeprecated() for materialized __call()

[TimWolla]

Fixes #17913

[TimWolla]

I don't quite understand, why I do not need to increase the refcount here: If I do, I get a leak.

[nielsdos]

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.

[nielsdos]

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.

[nielsdos]

That seems to work fine.

[DanielEScherzer]

The deprecated attribute is only since 8.4, but if the patch is going to expose all of the attributes from __call and __callStatic to closures, shouldn't this be targeting 8.3?

[nielsdos]

This should be fine according to the discussion in #17880
As for the target branch: this is likely safe for 8.3

[TimWolla]

As for the target branch: this is likely safe for 8.3

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 isDeprecated() it's an actual bugfix (the warning behavior is inconsistent with Reflection), but for other attributes it's more of a feature request.

[nielsdos]

@TimWolla I agree, let's keep this in 8.4.