Re: [PHP-DEV] Resurrecting named parameters
On Tue, Apr 7, 2020 at 2:45 PM Nikita Popov 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:
Re: [PHP-DEV] Resurrecting named parameters
On Thu, Apr 9, 2020 at 7:21 AM Stanislav Malyshev
wrote:
> Hi!
>
> > 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 we should have a notice for now, and eventually when people are
> used to it upgrade it to an error.
>
> > 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.
>
> IIRC I did some work on that when making the skipped parameter RFC, but
> I think we have no choice but to make all internal functions we control
> have proper default implementation. Older modules may be lagging though,
> so another option would be to add some kind of flag to internal
> functions that would block named parameters if not set, and make
> extension writers fix their functions and then set this flag explicitly.
> Of course we'll set this flag for all functions in core, after fixing
> those of them that need fixing. Maybe not a flag but using different
> macro/API function, etc. - the idea is that some explicit change would
> be needed. Yes, that means additional work...
Yeah, I'm aware of the work you did in this area, and used this as part of
my previous named parameters implementation. However, I'm not sure that
this is the right way forward anymore, for a couple of reasons:
1. Safety: 3rd party extension functions that don't get updated may end up
crashing if it has ZEND_NUM_ARGS() based assumptions. Not just stop working
or misbehave, but segfault. I think we should try to avoid that.
2. Implementation: This approach still requires ZPP support, in that we
need to skip over arguments that have not been passed and leave them at
their default values. This is pretty easy to do for classic ZPP, but
somewhat problematic for FastZPP. This will require an extra UNDEF check
for all arguments, which (I think) is not much of a concern for
performance, but is a concern for code size, which is a limiting factor of
this API.
3. Behavior: Even if there are no safety issues because no arg num
assumptions are made, we might still be left with behavior that does not
make a lot of sense. For example, the array_keys() function does not do arg
count checks, but writing something like
array_keys($array, strict => true);
would still not make any sense, because the "strict" parameter is only
meaningful if the "search_value" parameter is specified. We specify this
function as follows in stubs:
function array_keys(array $arg, $search_value = UNKNOWN, bool $strict =
false): array {}
If we simply allow the call and fall back to ZPP defaults, that means the
call works, and will have the same behavior as array_keys($array), with the
"strict" parameter simply going ignored. On the other hand, if we handle
internal functions like userland functions, and replace missing arguments
with specified default values, then this case would yield an exception:
array_keys(): Argument #2 ($search_value) must be passed explicitly,
because the default value is not known
I think that this gives us a more sensible behavior. We have
(unfortunately) quite a lot of internal functions that are "magic" in some
way, and where certain parameters do not have well-defined defaults and
cannot reasonably be skipped when named parameters are used. I think that
this approach gives us a pretty clean handling of such cases.
Of course, the big disadvantage is that it does require default value
information in stubs. We have this information for nearly all bundled
extensions at this point, but not for 3rd party extensions. The ZPP based
approach does make most code "just work", even if it will some of the code
"just crash" instead...
Regards,
Nikita
Re: [PHP-DEV] Resurrecting named parameters
On Tue, Apr 7, 2020 at 2:45 PM Nikita Popov wrote: > > ## 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. I prefer this approach. It might seem inelegant at first but I believe none of the other options will do us any real favors, especially stronger LSP enforcement on the language level. And opt-in seems like a non-starter for reasons Rowan Tommins lay out (though he's in favor :-) Dev side tools will be fully able to "deal" with the downfall. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Resurrecting named parameters
Hi! > 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 we should have a notice for now, and eventually when people are used to it upgrade it to an error. > 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. IIRC I did some work on that when making the skipped parameter RFC, but I think we have no choice but to make all internal functions we control have proper default implementation. Older modules may be lagging though, so another option would be to add some kind of flag to internal functions that would block named parameters if not set, and make extension writers fix their functions and then set this flag explicitly. Of course we'll set this flag for all functions in core, after fixing those of them that need fixing. Maybe not a flag but using different macro/API function, etc. - the idea is that some explicit change would be needed. Yes, that means additional work... More complicated but milder option would be to only check the flag if there's a "gap" in parameters - but this may have higher learning curve as extension writers wouldn't even know their extension needs work until someone skips a parameter. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Resurrecting named parameters
Le mardi 7 avril 2020, 14:44:38 CEST Nikita Popov a écrit : > 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. > > 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. This is a more general problem with internal functions which bugs me a lot. They can behave in a different way when not providing an attribute and when providing the default value. We should aim to make them behave exactly as a userland function with the same stub, no? -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Resurrecting named parameters
On 07/04/2020 13:44, Nikita Popov wrote:
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.
I generally prefer the idea of named parameters requiring some sort of
opt-in at the definition site.
I feel like this would solve a whole bunch of problems:
- LSP violations could be stricter errors, because they would show up
once the parent class opted in, not as soon as you upgraded your PHP
version.
- Similarly, library authors would have a chance to tidy up their
parameter naming, knowing that it was going to be a part of the
compatibility contract for future versions. Without an opt-in, they
might find themselves unable to fix poor or inconsistent names without
declaring a BC break.
- Internal functions could also be "opted in" gradually, giving core
devs and extension authors time to sort out any that do odd things with
ZPP or ZEND_NUM_ARGS.
- Variadic functions could be automatically "opted out" without needing
a particular special case (they could just give a generic "function
doesn't support named params" error).
The biggest downside I can see is that there's no obvious way of library
code opting in to named parameters when run under PHP 8 (or whenever we
add them) but also compiling successfully under earlier versions. That
would certainly slow the spread of the feature rather drastically, which
would be a shame. I wonder if there's a clever syntax we could adopt
that would avoid this.
In some ways, it would be nice to go one step further, and separate the
names presented to callers from the names used inside the function body.
Since parameters become ordinary mutable variables for the rest of the
function, it can be tempting to think of them as part of the
implementation, not part of the signature. This can lead to them
naturally changing both over time, and in inheritance hierarchies.
For instance, an interface might define a parameter as "$id", but a
concrete class want to label it as "$requestedFoobarId" to make the
implementation clearer. It would be nice to be able to (optionally)
define both. Straw man example syntax, using ":$foo" as opt-in, and
"foo: $bar" as aliasing:
function get( int id: $requestedFoobarId, boolean :$deep )
That would essentially be sugar for:
<>
function get( int $id, boolean $deep ) {
$requestedFoobarId = $id;
unset($id);
// ...
}
In the same way, normal positional parameters could be (and in some
languages are) seen as sugar for this:
function get( int $1, boolean $2 ) {
$requestedFoobarId = $1;
$deep = $2;
// ...
}
It would also give another option for the LSP problem - if the parent
class or interface opts into named parameters, the child class gets
implicit aliases if it doesn't specify any:
interface Foo { function thing( :$id ); } // named parameter "id"
class Bar implements Foo { function thing( id: $barId ) {} } // explicit
alias matches interface, $bar->thing(id: 42) will set $barId=42
class Baz implements Foo { function thing( $bazId ) {} } // implicitly
aliased, $baz->thing(id: 42) will set $bazId=42 based on the name in the
interface
class Wrong implements Foo { function thing( :$wrongName ) {} } //
Error: tries to accept named parameter "wrongName", but interface
requires it to accept named parameter "id" instead
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Resurrecting named parameters
On Tue, Apr 7, 2020 at 6:45 AM Nikita Popov wrote: > > ## 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). I would prefer to emit a low severity diagnostic if the parameter names do not match during inheritance, specifically noting that named parameter calls may not work as expected on such methods, rather than emitting a diagnostic when named parameters get used. If we decide to do it at named parameter usage as proposed, I would want a stronger error than simply a diagnostic: the user is directly hitting a case where we have very reasonable doubt that the code is doing what was intended. > > ## Internal functions > > 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 know that if we can make parameter defaults in Reflection that some people would consider that reason enough to vote yes on this proposal. It's always annoying with Internal and User functions have differences. I am okay with a heavier cost to named parameters in general. It's a new feature, and generally a _convenience_ one. - I am a bit worried about variadics having named parameters in them. I remember encountering code that assumes that `...$a` is a packed array. Is that assumption always correct today? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
