On Tue, Apr 7, 2020 at 2:45 PM Nikita Popov <nikita....@gmail.com> wrote:
> Hi internals, > > It's been a few years since I originally brought up the named parameters > RFC: https://wiki.php.net/rfc/named_params > > This topic has recently come up as part of > https://externals.io/message/109220 again, in particular with the > observation that the combination of constructor parameter promotion and > named parameters will give us, as a side-effect, an ergonomic object > initialization syntax that is well-integrated with the remainder of the > language. > > I've reimplemented basic named param support for master in > https://github.com/php/php-src/pull/5357, but before going any further > here, I want to restart discussion on the various open issues that the > named parameter concept in PHP has. I think there's two primary issues: > > ## LSP checks for parameter names > > Parameter names currently have no particular significance in PHP, only > their position in the signature is important. If named parameters are > introduced (in a way that does not require opt-in at the declaration-site), > then parameter names become significant. This means that changing a > parameter name during inheritance constitutes an LSP violation (for > non-constructor methods). There are a number of ways in which this issue > may be approached: > > 1. Silently ignore the problem. No diagnostic is issued when a parameter > name is changed during inheritance. An error exception will be thrown when > trying to use the (now) unknown parameter name. > > 2. Throw a notice if a parameter name is changed. While LSP violations are > normally fatal errors (in PHP 8), we could use a lower-severity diagnostic > for this case, that allows code to still run, but makes developers aware of > the problem. (It should be noted that automatic fixup of parameter names > using tooling should be fairly easy to implement.) > I think option 2 here makes the most sense. People haven't taken parameter names into account for now, so warning them in 8.x should help avoid subtle bugs when starting to use named parameters. and we could make it an LSP failure in 9. > > 3. Allow using parameter names from the parent method, even if they have > been renamed. This makes things "just work", but seems fairly magic, and > has edge cases like a signature foo($a, $b) being changed to foo($b, $a), > where it's not possible to implicitly support both at the same time. > 4. Make named-parameter opt-in in some fashion, so that parameter names > only need to be preserved for methods that have the opt-in marker. I'm not > a fan of this, as it greatly diminishes the practical usefulness of named > parameters. > > ## Internal functions > > There are two problems when it comes to named parameters and internal > functions. The first is that the internal parameter names do not always > match the parameter names in the documentation. This is nothing we can't > solve (in a hopefully mostly automated way), but is work that needs to be > done. > > The larger problem is that internal functions don't have a well-defined > concept of parameter default value. Parameter defaults are implicit in C > code, and sometimes based on argument count checks. When named parameters > are involved, one generally cannot assume that if a later (optional) > parameter is passed, all earlier (optional) parameters are passed as well. > I still can see us getting 95-99% of php-src migrated to use parameters form the documentation in a short amount of time before 8, as the more detailed and complex work on the stubs has shown. > While it is possible (but rather challenging) to make things work mostly > transparently with current ZPP, some functions do need to adjusted to > support named parameters, and may cause misbehavior/crashes if they are > not. As a rule of thumb, functions that make non-trivial use of > ZEND_NUM_ARGS() are affected. This is something we can address for > functions in bundled extensions, but may also affect 3rd-party extensions. > > I think that ideally, we'd deal with internal functions the same way we do > for userland functions, by relying on default values that are part of the > function declaration. As part of the PHP stub work, we now have information > on default values in stub files, but this information is currently not > available to the engine. > > Máté Kocsis has been working on getting this information exposed in > Reflection, see https://github.com/php/php-src/pull/5353. In the current > form, accessing these defaults is fairly inefficient, but maybe with some > caching, we could base named parameters on this information: > > If there are no "gaps" in the passed parameters, then named parameters will > always work. If an optional parameter is not specified, but a later > parameter is, then we will fetch the default value for that parameter, and > pass it instead. If the parameter is optional, but has no well-defined > default (UNKNOWN in stubs), then the call is not permitted (Error: "Cannot > skip parameter $foo with unknown default value", or similar.) > I think this is a good approach. Even when the function has more complex "default" logic than the stub can represent, the default value in the stub (and more importantly in the docs) is what counts for users, not what the function body does when the argument is not present. > > Any thoughts on these issues, or named parameters in general would be > welcome. > > Regards, > Nikita >
