Fix #72305: ReflectionParameter::isCallable not implemented properly #2302
Conversation
ZEND_ARG_CALLABLE_INFO() is available as of PHP 7.0.0, so we finally use it.
|
@weltling okay for 7.0 ? |
|
Anyhow, let's wait if Travis would be fine with these changes. :-) |
|
@cmb69 yeah, that will happen whatever, just getting in early to get the right people involved ;) |
|
I'm -1 on any changes adding arginfo argument types until such a time as we do not validate them for internal functions. Otherwise this results in duplicate validation, which especially for callables is very expensive. Additionally, it should be noted that adding types changes the error behavior outside of strict mode. |
|
@nikic @dstogov commented on a closed PR that it would be okay to add types in master ... is the reason I considered it okay ... But these are good points you raise ... Guess it probably is too late for 7.0/7.1, but wanted to start a conversation ... mission accomplished ... let's see what others say ;) |
Indeed, and considering the BC impact confirmed by Travis, I don't think we can add the arginfo types in a revision; even a minor version appears to be too much of an issue. |
|
It's almost impressive to make that many tests fail at once 🤣 It was pretty obviously going to happen ... still, that there is disconnect between what the manual says, and what is actually the case is a thing worth trying to fix ... even if it will take another few years to actually do it ... Maybe it's another thing we slab for PHP 8 ? |
|
Maybe add an arginfo option for unenforced type hints (only for reflection and inheritance)? We can't add type hints as is and changing them all to unenforced may have side-effects on 3rd party extensions. |
|
More good points ... Without having thought about it too deeply, that seems viable, @cmb69 want to have a go at that ? |
Sorry, I'm not familiar enough with reflection and the engine to do that with reasonable effort. I assume that this PR isn't of any value, so I'm closing. |
|
Thanks for the effort @cmb69 ... we did advance the conversation, that's valuable ;) |
|
@carusogabriel This is still blocked on https://wiki.php.net/rfc/consistent_type_errors. |
ZEND_ARG_CALLABLE_INFO() is available as of PHP 7.0.0, so we finally
use it.