Re: [PHP-DEV] Handling of null arguments to internal functions

This is only part of a thread. view whole thread
June 6, 2019 11:48 (Arvids Godjuks)
чт, 6 июн. 2019 г. в 10:55, Nikita Popov>:

> Hi internals, > > The 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 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. > > 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? > > Regards, > Nikita >
Hello Nikita, as a user-land dev and in general looking at where I would like for PHP to end up long-term, I think the first one is a better solution, though quite a bit more painful to do and longer-term to implement due to the amount of work involved. My reasoning is JIT - as things go forward, we probably are going to start to see some things get implemented in PHP instead of the C core (*caught* PDO) and that might lead to some strange situations if the argument handling is going to be inconsistent between these two worlds. But consistency, in general, would be a nice change of pace so you don't have to keep in mind that there are slight differences in behaviour depending on what you call - a built-in function or a userland one. My 0.02$, Thanks. -- Arvīds Godjuks +371 26 851 664 Skype: psihius Telegram: @psihius
June 6, 2019 12:46 (Rowan Collins)
On Thu, 6 Jun 2019 at 12:49, Arvids Godjuks>

> consistency, in general, would be a nice change of pace so you don't have > to keep in mind that there are slight differences in behaviour depending on > what you call - a built-in function or a userland one. >
This is my view as well. Another thing that inconsistency causes problems with is polyfills - if you want to wrap, emulate, or otherwise reimplement an internal function, it can be fiddly to emulate the subtleties of ZPP. (This gets worse with objects, which can do all sorts of wacky things internally that have no user-space equivalent, but that's a topic for another day.) Would it be possible to use a combination of automation (analysis of the C code, or fuzz testing of the functions) and collaboration (a great big list people can work through a section of and report back) to categorise the functions in core? Something like: a) not affected, because handling is consistent with userland anyway b) should explicitly accept nulls c) should explicitly reject nulls We do however have to make a tricky judgement on functions in category c, of how much code is going to break if we make them stricter. Regards, -- Rowan Collins [IMSoP]