On 06/22/2015 07:54 PM, Stanislav Malyshev wrote: > Hi! > > So I tried to remove the checks for ZEND_ACC_HAS_TYPE_HINTS: > https://github.com/php/php-src/pull/1354 > > Turns out there is a lot of tests that assume function with wrong > arguments throws Error, but ZPP and type checks work differently - ZPP > first checks argument number (and doesn't throw if number is wrong but > instead creates warning), while type checks (that shouldn't really > happen on internals but do) check types first and not check argument number. > > So I wonder which way is the best to proceed here. Looks like this comes > from 5.6 where internal function types were verified in executor: > https://github.com/php/php-src/blob/PHP-5.6/Zend/zend_vm_def.h#L1974 > > I'm not sure what to do with it - on one side, I think ZPP should handle > it for internal functions, otherwise we're doing the same work twice. On > the other side, that would be pretty substantial, even if maybe correct, > BC break. Should we keep the ZEND_ACC_HAS_TYPE_HINTS type checks and > accept the fact that this way we're checking everything twice, or should > be clean it up?
Is it a BC break against real code though? Fixing tests isn't a big deal. What kind of real code would break by turning calls with the wrong number of args from an error to a warning? I suppose it is a bit late in the game to fix this, but it really does seem very broken to the point where I wonder if we shouldn't just disable reflection on internal functions entirely since the arginfo is so woefully incomplete. Plus the double-checking is super messy. -Rasmus
signature.asc
Description: OpenPGP digital signature
