On Thu, 6 Jun 2019 at 10:54, Nikita Popov <nikita....@gmail.com> wrote:
> Hi internals, > > The https://wiki.php.net/rfc/consistent_type_errors RFC resolved one of > the > big differences in argument handling between internal and userland > functions. This change allowed us to add return type information (available > through reflection) to internal functions, something that Gabriel Caruso > has been working on. > > During the discussion of that RFC, Andrea raised another important > difference: User functions do not accept null for a typed argument, unless > it is explicitly nullable (through ?Type or a null default value). This is > independent of whether the type is scalar (int and array both reject null) > and also independent of strict_types. > > For internal functions on the other hand, in weak typing mode, null will be > accepted to scalar arguments and coerced to the respective type. For > example strlen(null) becomes strlen('') becomes 0, without emitting any > diagnostics. > > More precisely, this is a difference between two different argument > handling approaches, arginfo (used by userland, forbids null) and zpp (used > by internal, allows null). As we would like to also use arginfo for > internal functions (as it is available through reflection, while zpp is > not), we need to resolve this discrepancy in some way. > > The "correct" thing to do would be to forbid passing null to non-nullable > arguments for internal functions / zpp as well, making behavior fully > consistent. I've created https://github.com/php/php-src/pull/4227 to > prototype this and see how the test impact looks like. > > However, I think there will be a lot of fallout from this change. When > running this patch against the Symfony test suite, I get many thousands of > deprecation warnings (though many with the same root cause). In some cases, > it's a matter of us not marking arguments as nullable that likely should > be, e.g. I was able to remove a slew of deprecation warnings by making the > $pattern argument in NumberFormatter/IntlDateFormatter nullable. > > This is what I originally wanted to propose here, but after seeing the > impact this has (relative to, say, changing == semantics) it might not be a > good idea. > I personnally think it's a good idea. However with the possible impact we could maybe work on this on a longer timescale, as in soft deprecation in PHP 7.4 by informing users that some of the internal functions where a null value isn't sensible (e.g. strlen) will emit deprecation notices for the whole PHP 8 lifecycle and be removed in PHP 9 (whenever that is). But this would take a lot of effort to audit every internal function and debate if a null argument makes "sense". Another way would be to mark every internal function scalar argument explicitly nullable and implement the current behavior into the function, this is even a bigger task and probably not the wisest too, but it would resolve the discrepancy without any BC breaks. > If we don't want to do that, then we should start accepting null for > scalars for internal functions in arginfo as well. It will resolve the > arginfo/zpp discrepancy, but will leave the divide between > internal/userland functions. > > Thoughts? > I'm not really keen on the idea that internal functions can behave differently than userland ones, even though this is currently the case. > Regards, > Nikita > Best regards George P. Banyard
