[RFC] Default expression

Matthew Weier O'Phinney

2 years ago

On Sat, Aug 24, 2024, 11:50 AM Bilge <bilge@scriptfusion.com> wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>

This is a feature I've wanted for a very long time! The RFC is very
straight forward, and the appendix does a great job of enumerating the
possible expressions.

Nice work all around!

Bilge

2 years ago

On 24/08/2024 18:01, Matthew Weier O'Phinney wrote:

>
> This is a feature I've wanted for a very long time! The RFC is very
> straight forward, and the appendix does a great job of enumerating the
> possible expressions.
>
> Nice work all around!

Thanks, Matt! Glad you like this one (and the last one!)

Hopefully we can land it this time 🙂

Cheers,
Bilge

Rob Landers

2 years ago

On Sat, Aug 24, 2024, at 18:49, Bilge wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've
> been cooking for a little while. Considering I don't know C or PHP
> internals, one might think implementing this feature would be
> prohibitively difficult, but considering the amount of help and guidance
> I received from Ilija, Bob and others, it would be truer to say it would
> have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge
>

This is pretty awesome! I see this as some syntax sugar, to be honest:

> as soon as two or more nullable arguments are involved

I'm not sure what you mean here. I use this method all the time :) much to the chagrin of some of my coworkers.

function stuff($foo = 'bar', $baz = 'world');

stuff(...[ ...($foo ? ['foo' => $foo] : []), ...($baz ? ['baz' => $baz] : [])]);

Having this would be a lot less verbose.

— Rob

Bilge

2 years ago

On 24/08/2024 22:14, Rob Landers wrote:

> On Sat, Aug 24, 2024, at 18:49, Bilge wrote:
>
>> as soon as two or more nullable arguments are involved
>
> I'm not sure what you mean here. I use this method all the time :)
> much to the chagrin of some of my coworkers.
>
> function stuff($foo = 'bar', $baz = 'world');
>
> stuff(...[ ...($foo ? ['foo' => $foo] : []), ...($baz ? ['baz' =>
> $baz] : [])]);
>

You're right; splat with keys does work. This is something my RFC
currently glosses over and should probably be rectified!

Cheers,
Bilge

Mike Schinkel

2 years ago

>
> On Aug 24, 2024 at 5:16 PM,  <Rob Landers (mailto:rob@bottled.codes)>  wrote:
>
>
>
> I'm not sure what you mean here. I use this method all the time :) much to the chagrin of some of my coworkers.
>
>
>
> function stuff($foo = 'bar', $baz = 'world');
>
>
>
> stuff(...[ ...($foo ? ['foo' =>  $foo] : []), ...($baz ? ['baz' =>  $baz] : [])]);
>
>

And you are one who complains about gotos!    😲

-Mike

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 04:41, Mike Schinkel wrote:

>
>
>> On Aug 24, 2024 at 5:16 PM, <Rob Landers <mailto:rob@bottled.codes>> wrote:
>> I'm not sure what you mean here. I use this method all the time :) much to the chagrin of some of my coworkers.
>>
>> function stuff($foo = 'bar', $baz = 'world');
>>
>> stuff(...[ ...($foo ? ['foo' => $foo] : []), ...($baz ? ['baz' => $baz] : [])]);
>
> And you are one who complains about gotos!  😲
>
> -Mike

Haha, there is a difference between production/professional code and internal tools. Internal tools are a place to experiment and have a little fun, IMHO.

— Rob

Ben Ramsey

2 years ago

> On Aug 24, 2024, at 11:49, Bilge <bilge@scriptfusion.com> wrote:
>
> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've been cooking for a little while. Considering I don't know C or PHP internals, one might think implementing this feature would be prohibitively difficult, but considering the amount of help and guidance I received from Ilija, Bob and others, it would be truer to say it would have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge

Great RFC, Bilge! I was already on-board after the introduction, but if I had any doubts, the examples in the appendix sold me.

Cheers,
Ben

Bilge

2 years ago

On 25/08/2024 04:06, Ben Ramsey wrote:

> Great RFC, Bilge! I was already on-board after the introduction, but if I had any doubts, the examples in the appendix sold me.
>
> Cheers,
> Ben

Thanks, Ben. That means a lot to me :)

Cheers,
Bilge

Unnamed Person

2 years ago

(resending as I accidentally originally send a private reply instead of
sending the below to the list)

On 24-8-2024 18:49, Bilge wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've
> been cooking for a little while. Considering I don't know C or PHP
> internals, one might think implementing this feature would be
> prohibitively difficult, but considering the amount of help and
> guidance I received from Ilija, Bob and others, it would be truer to
> say it would have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge
>

Hi Bilge,

I like the idea, but see some potential for issues with ambiguity, which
I don't see mentioned in the RFC as "solved".

Example 1:
```php
function foo($paramA, $default = false) {}
foo( default: default ); // <= Will this be handled correctly ?
```

Example 2:
```php
callme(
match($a) {
10 => $a * 10,
20 => $a * 20,
default => $a * default, // <= Based on a test in the PR this
should work. Could you confirm ?
}
);
```

Example 3:
```php
switch($a) {
case 'foo':
return callMe($a, default); // I presume this shouldn't be a
problem, but might still be good to have a test for this ?
default:
return callMe(10, default); // I presume this shouldn't be a
problem, but might still be good to have a test for this ?
}
```

On that note, might it be an idea to introduce a separate token for the
`default` keyword when used as a default expression in a function call
to reduce ambiguity ?

Smile,
Juliette

Bilge

2 years ago

On 25/08/2024 10:49, Juliette Reinders Folmer wrote:

> (resending as I accidentally originally send a private reply instead
> of sending the below to the list)
>
> On 24-8-2024 18:49, Bilge wrote:
>> Hi gang,
>>
>> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
>> think some of you might enjoy this one. Hit me with any feedback.
>>
>> This one already comes complete with working implementation that I've
>> been cooking for a little while. Considering I don't know C or PHP
>> internals, one might think implementing this feature would be
>> prohibitively difficult, but considering the amount of help and
>> guidance I received from Ilija, Bob and others, it would be truer to
>> say it would have been more difficult to fail! Huge thanks to them.
>>
>> Cheers,
>> Bilge
>>
>
> Hi Bilge,

Hi :)

> I like the idea, but see some potential for issues with ambiguity,
> which I don't see mentioned in the RFC as "solved".
>
> Example 1:
> ```php
> function foo($paramA, $default = false) {}
> foo( default: default ); // <= Will this be handled correctly ?
> ```

No, but not because of my RFC, but because $paramA is a required
parameter that was not specified. Assuming that was just a typo, the
following works as expected:

function foo($paramA = 1, $default = false) {
    var_dump($default);
}
foo(default: default); // bool(false)

> Example 2:
> ```php
> callme(
>     match($a) {
>         10 => $a * 10,
>         20 => $a * 20,
>         default => $a * default, // <= Based on a test in the PR this
> should work. Could you confirm ?
>     }
> );
> ```

Yes.

> Example 3:
> ```php
> switch($a) {
>     case 'foo':
>         return callMe($a, default); // I presume this shouldn't be a
> problem, but might still be good to have a test for this ?
>     default:
>         return callMe(10, default); // I presume this shouldn't be a
> problem, but might still be good to have a test for this ?
> }
> ```

Yes.

> On that note, might it be an idea to introduce a separate token for
> the `default` keyword when used as a default expression in a function
> call to reduce ambiguity ?

Considering the Bison grammar compiles, I believe there can be no
ambiguity. I specifically picked `default` because I think it is the
most intuitive keyword to use for this, and it's conveniently already a
reserved word.

Cheers,
Bilge

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 12:01, Bilge wrote:

> On 25/08/2024 10:49, Juliette Reinders Folmer wrote:
> > (resending as I accidentally originally send a private reply instead
> > of sending the below to the list)
> >
> > On 24-8-2024 18:49, Bilge wrote:
> >> Hi gang,
> >>
> >> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> >> think some of you might enjoy this one. Hit me with any feedback.
> >>
> >> This one already comes complete with working implementation that I've
> >> been cooking for a little while. Considering I don't know C or PHP
> >> internals, one might think implementing this feature would be
> >> prohibitively difficult, but considering the amount of help and
> >> guidance I received from Ilija, Bob and others, it would be truer to
> >> say it would have been more difficult to fail! Huge thanks to them.
> >>
> >> Cheers,
> >> Bilge
> >>
> >
> > Hi Bilge,
> Hi :)
> > I like the idea, but see some potential for issues with ambiguity,
> > which I don't see mentioned in the RFC as "solved".
> >
> > Example 1:
> > ```php
> > function foo($paramA, $default = false) {}
> > foo( default: default ); // <= Will this be handled correctly ?
> > ```
>
> No, but not because of my RFC, but because $paramA is a required
> parameter that was not specified. Assuming that was just a typo, the
> following works as expected:
>
> function foo($paramA = 1, $default = false) {
>     var_dump($default);
> }
> foo(default: default); // bool(false)
>
> > Example 2:
> > ```php
> > callme(
> >     match($a) {
> >         10 => $a * 10,
> >         20 => $a * 20,
> >         default => $a * default, // <= Based on a test in the PR this
> > should work. Could you confirm ?
> >     }
> > );
> > ```
> Yes.
> > Example 3:
> > ```php
> > switch($a) {
> >     case 'foo':
> >         return callMe($a, default); // I presume this shouldn't be a
> > problem, but might still be good to have a test for this ?
> >     default:
> >         return callMe(10, default); // I presume this shouldn't be a
> > problem, but might still be good to have a test for this ?
> > }
> > ```
> Yes.
> > On that note, might it be an idea to introduce a separate token for
> > the `default` keyword when used as a default expression in a function
> > call to reduce ambiguity ?
> Considering the Bison grammar compiles, I believe there can be no
> ambiguity. I specifically picked `default` because I think it is the
> most intuitive keyword to use for this, and it's conveniently already a
> reserved word.

Other tools parse the tokens directly (for example, I have a tool to take php classes and convert them to graphql specifications. It parses the tokens emitted the tokenization extension), and having "default" tokens in unexpected places presents an ambiguity and BC break for those tools. By having a token (DEFAULT_PARAM_VALUE) or something to disambiguate might be better. I had assumed it was a separate token when I first read it, so this is a good point.

— Rob

Larry Garfield

2 years ago

On Sat, Aug 24, 2024, at 11:49 AM, Bilge wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've
> been cooking for a little while. Considering I don't know C or PHP
> internals, one might think implementing this feature would be
> prohibitively difficult, but considering the amount of help and guidance
> I received from Ilija, Bob and others, it would be truer to say it would
> have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge

I am still not fully sold on this, but I like it a lot better than the previous attempt at a default keyword.  It's good that you mention named arguments, as those do replace like 95% of the use cases for "put default here" in potential function calls, and the ones it doesn't, you call out explicitly as the justification for this RFC.

The approach here seems reasonable overall.  The mental model I have from the RFC is "yoink the default value out of the function, drop it into this expression embedded in the function call, and let the chips fall where they may."  Is that about accurate?

My main holdup is the need.  I... can't recall ever having a situation where this is something I needed.  Some of the examples show valid use cases (eg, the "default plus this binary flag" example), but again, I've never actually run into that myself in practice.

My other concern is the list of supported expression types.  I understand how the implementation would naturally make all of those syntactically valid, but it seems many of them, if not most, are semantically nonsensical.  Eg, `default > 1` would take a presumably numeric default value and output a boolean, which should really never be type compatible with the function being called.  (A param type of int|bool is a code smell at best, and a fatal waiting to happen at worst.)  In practice, I think a majority of those expressions would be logically nonsensical, so I wonder if it would be better to only allow a few reasonable ones and block the others, to keep people from thinking nonsensical code would do something useful.

--Larry Garfield

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 15:35, Larry Garfield wrote:

> On Sat, Aug 24, 2024, at 11:49 AM, Bilge wrote:
> > Hi gang,
> >
> > New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> > think some of you might enjoy this one. Hit me with any feedback.
> >
> > This one already comes complete with working implementation that I've
> > been cooking for a little while. Considering I don't know C or PHP
> > internals, one might think implementing this feature would be
> > prohibitively difficult, but considering the amount of help and guidance
> > I received from Ilija, Bob and others, it would be truer to say it would
> > have been more difficult to fail! Huge thanks to them.
> >
> > Cheers,
> > Bilge
>
> I am still not fully sold on this, but I like it a lot better than the previous attempt at a default keyword.  It's good that you mention named arguments, as those do replace like 95% of the use cases for "put default here" in potential function calls, and the ones it doesn't, you call out explicitly as the justification for this RFC.
>
> The approach here seems reasonable overall.  The mental model I have from the RFC is "yoink the default value out of the function, drop it into this expression embedded in the function call, and let the chips fall where they may."  Is that about accurate?
>
> My main holdup is the need.  I... can't recall ever having a situation where this is something I needed.  Some of the examples show valid use cases (eg, the "default plus this binary flag" example), but again, I've never actually run into that myself in practice.

Potentially the most useful place would be in attributes. Take crell\serde (:p) for instance:

#[SequenceField(implodeOn: default . ' ', joinOn: ' ' . default . ' ')]

Where you may just want it to be a little more readable, but aren't interested in the default implosion. In attributes, it has to be a static expression and I think this passes that test? At least that is one place I would find most useful.

Then there are things like the example I gave before, where you need to call some library code as library code and pass through the intentions. It also gets us one step closer to something like these shenanigans:

function configureSerializer(Serde $serializer = new SerdeCommon(formatters: default as $formatters));

Where we can call configureSerializer(formatters: new JsonStreamFormatter()).

Some pretty interesting stuff.

>
> My other concern is the list of supported expression types.  I understand how the implementation would naturally make all of those syntactically valid, but it seems many of them, if not most, are semantically nonsensical.  Eg, `default > 1` would take a presumably numeric default value and output a boolean, which should really never be type compatible with the function being called.  (A param type of int|bool is a code smell at best, and a fatal waiting to happen at worst.)  In practice, I think a majority of those expressions would be logically nonsensical, so I wonder if it would be better to only allow a few reasonable ones and block the others, to keep people from thinking nonsensical code would do something useful.

I'm reasonably certain you can write nonsensical PHP without this feature. I don't think we should be the nanny of developers.

— Rob

Bilge

2 years ago

On 25/08/2024 14:35, Larry Garfield wrote:

> The approach here seems reasonable overall.  The mental model I have from the RFC is "yoink the default value out of the function, drop it into this expression embedded in the function call, and let the chips fall where they may."  Is that about accurate?

Yes, as it happens. That is the approach we took, because the
alternative would have been changing how values are sent to functions,
which would have required a lot more changes to the engine with no clear
benefit. Internally it literally calls the reflection API, but a
low-level call, that elides the class instantiation and unnecessary
hoops of the public interface that would just slow it down.

> My main holdup is the need.  I... can't recall ever having a situation where this is something I needed.  Some of the examples show valid use cases (eg, the "default plus this binary flag" example), but again, I've never actually run into that myself in practice.

That's fine. Not everyone will have such a need, and of those that do,
I'm willing to bet it will be rare or uncommon at best. But for those
times it is needed, the frequency by which it is needed in no way
diminishes its usefulness.I rarely use `goto` but that doesn't mean we
shouldn't have the feature.

> My other concern is the list of supported expression types.  I understand how the implementation would naturally make all of those syntactically valid, but it seems many of them, if not most, are semantically nonsensical.  Eg, `default > 1` would take a presumably numeric default value and output a boolean, which should really never be type compatible with the function being called.  (A param type of int|bool is a code smell at best, and a fatal waiting to happen at worst.)  In practice, I think a majority of those expressions would be logically nonsensical, so I wonder if it would be better to only allow a few reasonable ones and block the others, to keep people from thinking nonsensical code would do something useful.

Since you're not the only one raising this, I will address it, but just
to say there is no good reason, in my mind, to ever prohibit the
expressiveness. To quote Rob

>I'm reasonably certain you can write nonsensical PHP without this

feature. I don't think we should be the nanny of developers.

I fully agree with that sentiment. It seems to be biting me that I went
to the trouble of listing out every permutation of what /expression/
means where perhaps this criticism would not have been levied at all had
I chosen not to do so. Why does that matter? Because PHP already allows
you to do many more ridiculous things, they're just not routinely
presented to you so they're not part of your mind map. The end-user
documentation will also not mention the nonsense cases, so the average
developer will not think of them. You can write, `include(1 + 1);`,
because `include()` accepts an expression. You will get: "Failed opening
'2' for inclusion". Should we restrict that? No, because that's just how
expressions work in any context where they're allowed. Special-casing
the T_DEFAULT grammar would not only bloat the grammar rules but also
increase the chance that new expression grammars introduced in future,
which could conveniently interoperate with `default`, would be
unintentionally excluded by omission.

Cheers,
Bilge

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 16:29, Bilge wrote:

> You can write, `include(1 + 1);`, because `include()` accepts an
> expression. You will get: "Failed opening '2' for inclusion". Should
> we restrict that? No, because that's just how expressions work in any
> context where they're allowed.

I think a better comparison might be the "new in initializers" and
"fetch property in const expressions" RFCs, which both forbid uses which
would naturally be allowed by the grammar. The rationale in those cases
was laid out in
https://wiki.php.net/rfc/new_in_initializers#unsupported_positions and
https://wiki.php.net/rfc/fetch_property_in_const_expressions#supporting_all_objects

To pull out a point that might be overlooked at the bottom of my longer
response earlier:

> As the RFC points out, library authors already worry about the

maintenance burden of named argument support, will they now also need to
question whether someone is relying on "default + 1" having some
specific effect?

By saying "default can be used in any expression, as complex as the
caller can imagine", we're implicitly saying "if you add a default to
your function signature, that is no information a user can pull *out* as
part of your API".

Regards,

--
Rowan Tommins
[IMSoP]

Ilija Tovilo

2 years ago

Hi Rowan

On Sun, Aug 25, 2024 at 6:06 PM Rowan Tommins [IMSoP]
<imsop.php@rwec.co.uk> wrote:

>
> On 25/08/2024 16:29, Bilge wrote:
> > You can write, `include(1 + 1);`, because `include()` accepts an
> > expression. You will get: "Failed opening '2' for inclusion". Should
> > we restrict that? No, because that's just how expressions work in any
> > context where they're allowed.
>
>
> I think a better comparison might be the "new in initializers" and
> "fetch property in const expressions" RFCs, which both forbid uses which
> would naturally be allowed by the grammar. The rationale in those cases
> was laid out in
> https://wiki.php.net/rfc/new_in_initializers#unsupported_positions and
> https://wiki.php.net/rfc/fetch_property_in_const_expressions#supporting_all_objects

I don't agree with that. Constant expressions in PHP already only
support a subset of operations that expressions do. However, default
is proposed to be a true expression, i.e. one that compiles to
opcodes. Looking at the `expr` nonterminal [1] I can't see any
productions that are restricted in the context they can be used in,
even though plenty of them are nonsensical (e.g. exit(1) + 2).

Furthermore, new in initializers was disallowed in some contexts not
because it would be nonsensical, but because it posed technical
difficulties.

I also believe some of the rules you've laid out would be hard to enforce.

> 1) The expression should be reasonably guaranteed to produce the same type as the actual default.

Even the simple cases of ??, ?: can easily break this rule.

Furthermore, context restriction is easily circumvented. E.g.

foo((int) default); // This is not allowed
foo((int) match (true) { default => default }); // Let me just do that

I'm not sure context restriction is worthwhile, if 1. we can't do it
properly anyway and 2. there are no technical reasons to do so.

Ilija

[1] https://github.com/php/php-src/blob/3f4028d3d9d63e1dae012a9c350141493b30825f/Zend/zend_language_parser.y#L1198

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 17:36, Ilija Tovilo wrote:

> I don't agree with that. Constant expressions in PHP already only
> support a subset of operations that expressions do. However, default
> is proposed to be a true expression, i.e. one that compiles to
> opcodes.

This is circular: obviously, changing the proposal requires making
changes to what is proposed.

I'm arguing that allowing default as a token that's usable in arbitrary
expressions is unnecessary and problematic, and that we should instead
define the specific use cases, and build the feature around those.

> I also believe some of the rules you've laid out would be hard to enforce.

The rules were intended to guide the design of the feature, not be
things that someone needed to enforce in code somewhere. If you start
with the aim of implementing:

- Use in place of an argument
- Use with bitwise | and &
- Use on the RHS of ?: and ??

Then maybe you end up with a completely different implementation from
what's currently been written.

For instance, rather than adding "default" to the "expr" rule in the
grammar, and then restricting it at compile-time, maybe we add a new
grammar rule "expr_with_default", usable only in expressions and with a
very limited set of productions. Maybe that means we can't support
match() expressions, because it would bloat the grammar too much, but
"match($foo) { 'blah'=> 'bleugh', default => default }" is pretty ugly
anyway.

Or maybe, the expressions are allowed, but they're compiled down with
"default" as a special pseudo-type that has limited legal operations, so
that the result of "(int)default" is undefined, no matter how you try to
obfuscate it.


Just because it's easy to implement a feature a particular way, doesn't
mean that's necessarily the right way.

--
Rowan Tommins
[IMSoP]

Bilge

2 years ago

On 25/08/2024 18:12, Rowan Tommins [IMSoP] wrote:

>
> For instance, rather than adding "default" to the "expr" rule in the
> grammar, and then restricting it at compile-time, maybe we add a new
> grammar rule "expr_with_default", usable only in expressions and with
> a very limited set of productions.
>

Like the original commit
<https://github.com/php/php-src/pull/15437/commits/fd7ac5f83b8282227235095843cb73e9d66b0717#diff-3e6742a9069b5717cf961c9d6b2aefbd1c730869d8a58123b1b5f3bc3e9082fcR1329>?
Yeah, we did that.

>
> Just because it's easy to implement a feature a particular way,
> doesn't mean that's necessarily the right way.
>

With respect, you do not know what you're talking about here. The
original approach was to start manually whitelisting each expression
grammar I thought made sense. THAT was the easiest way because both
myself an Ilija failed in our first attempts to expand the grammar to
support default as a general expression, and not for lack of trying. It
took a Bison grammar expert to drop a patch that certainly wowed me,
because hitherto I wasn't even certain it was possible, mainly because
of the conflicts with `match` (but also `switch` to some extent). Aside,
with respect to match, there is still an unresolved case and the RFC
needs updating with the semantics we want to enforce there. So we
pursued default as an expression not because it was easy, but despite
the fact that it was hard, because it was precisely what we wanted to do.

I apologise for coming on strong, but I put a lot of effort into this,
so I take exception to the implication that anyone involved took the
easy way out to arrive at this (our best) solution.

Kind regards,
Bilge

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 18:30, Bilge wrote:

> I apologise for coming on strong, but I put a lot of effort into this,
> so I take exception to the implication that anyone involved took the
> easy way out to arrive at this (our best) solution.

I apologise for the inadvertent offence.

It was based solely on this comment from Ilija:

> I also believe some of the rules you've laid out would be hard to

enforce.

I took that to mean that supporting generic expressions was
straight-forward, but supporting a limited set would be complex in some
way. Apparently I was wrong in that interpretation; in which case, I've
no idea what that sentence was referring to.

--
Rowan Tommins
[IMSoP]

Bilge

2 years ago

On 25/08/2024 18:46, Rowan Tommins [IMSoP] wrote:

> On 25/08/2024 18:30, Bilge wrote:
>> I apologise for coming on strong, but I put a lot of effort into
>> this, so I take exception to the implication that anyone involved
>> took the easy way out to arrive at this (our best) solution.
>
>
> I apologise for the inadvertent offence.

That's OK, I can tell you're passionate about PHP and you're interested
in having a constructive discussion about this RFC, so we have that in
common 🙂

Cheers,
Bilge

Bilge

2 years ago

On 25/08/2024 17:05, Rowan Tommins [IMSoP] wrote:

> On 25/08/2024 16:29, Bilge wrote:
>> You can write, `include(1 + 1);`, because `include()` accepts an
>> expression. You will get: "Failed opening '2' for inclusion". Should
>> we restrict that? No, because that's just how expressions work in any
>> context where they're allowed.
>
>
> I think a better comparison might be the "new in initializers" and
> "fetch property in const expressions" RFCs, which both forbid uses
> which would naturally be allowed by the grammar. The rationale in
> those cases was laid out in
> https://wiki.php.net/rfc/new_in_initializers#unsupported_positions and
> https://wiki.php.net/rfc/fetch_property_in_const_expressions#supporting_all_objects
>

They do not seem like better comparisons because, in both cases, support
for those respective features was limited due to technical obstructions.
My implementation already permits `default` as a general expression
(thanks to Bob's Bison patch), Q.E.D. there is no technical constraint
precluding support for default as a general expression grammar. What you
are proposing is an artificial limitation on the language, which is an
entirely different proposition (and not a healthy one, in my view).

Allow me to address the point made in your previous email which ended up
hinting that `default + 1` should also be prohibited because it hasn't
been explicitly justified.

Notwithstanding I don't have the energy to justify every single
permutation of expressions, I'll humour the arithmetic operators
criticism with an example just to demonstrate that one can justify just
about anything with sufficient enthusiasm and creativity.

Suppose we have a Suspension class that suspends the current process for
a specified delay in milliseconds, but our subclass wants to present an
interface that deals with whole seconds (including fractional seconds
using floats).

class Suspension {
    /**
     * @param int $delay Specifies the delay in milliseconds.
     */
    public function suspend(int $delay = 1_000) {
        var_dump($delay);
    }
}

class MySuspension extends Suspension {
    /**
     * @param float|int|null $delay Specifies the delay in seconds.
     */
    public function suspend(float|int|null $delay = null) {
        parent::suspend((int)(($delay ?? 0) * 1000) ?: default);
    }
}

new MySuspension()->suspend(2.2345); // int(2234)

Not only have I demonstrated the need to use multiplication or division
to change the scale, but also the need to cast.

Cheers,
Bilge

John Coggeshall

2 years ago

> public function suspend(float|int|null $delay = null) {
> parent::suspend((int)(($delay ?? 0) * 1000) ?: default);
> }
> }
>
> new MySuspension()->suspend(2.2345); // int(2234)
> Not only have I demonstrated the need to use multiplication or division
> to change the scale, but also the need to cast.
>

I appreciate what you're saying here.
I've been struggling a little bit to really nail my language here on what I think should and shouldn't be allowed. Essentially I'm trying to say (and I think others are too) is this:
The engine should not allow the use of default in an expression that doesn't ultimately evaluate to default *.
In the above example the left - hand of the ?: operator doesn't use default , so it's evaluation is whatever it's evaluation is. The right-hand of the ?: operator DOES use default , and thus it must evaluate ultimately to default or that would be an error. Another example:
parent::foo((default >= 10) ? default : 10)
Would be permitted because the left-hand uses default , but the evaluation if the conditional where default was used for the true case true is default . Likewise the right-hand is just 10 and irrelevant
This would not be permitted
parent::foo((default >= 10) ? (default + 1) : 10)
Because now the ultimate evaluation of default is default + 1 -- not default
*The exception to the rule I've described above would be IFF the expression default is in only uses specific allowed operators like a subset of the bitwise operators.
I very much appreciate that what is being described here is a significant effort to achieve, I'm not even sure it's reasonably possible.. but I just can't get behind the idea that (default)->foobar() is a valid expression in this context or a good idea for the language. The use of this proposed default keyword must have guardrails IMO. I think my definition above is a pretty reasonable attempt at capturing where I think the line is here and hopefully that helps guide this discussion.
John

Rowan Tommins [IMSoP]

2 years ago

On 25 August 2024 21:00:03 BST, Bilge <bilge@scriptfusion.com> wrote:

>class Suspension {
>    /**
>     * @param int $delay Specifies the delay in milliseconds.
>     */
>    public function suspend(int $delay = 1_000) {
>        var_dump($delay);
>    }
>}
>
>class MySuspension extends Suspension {
>    /**
>     * @param float|int|null $delay Specifies the delay in seconds.
>     */
>    public function suspend(float|int|null $delay = null) {
>        parent::suspend((int)(($delay ?? 0) * 1000) ?: default);
>    }
>}
>
>new MySuspension()->suspend(2.2345); // int(2234)
>
>Not only have I demonstrated the need to use multiplication or division to change the scale, but also the need to cast.

Possibly something got lost as you redrafted the example, because as you've written it, neither the multiplication nor the cast are applied to the value looked up by "default". The parameter reduces to "(expression) ?: default", which I've already agreed is useful.

I was thinking about why "bitwise or" feels so different from other operators here, and I realised it's because it's idempotent (I hope I'm using that term correctly): if the specified bits are already set, it will have no effect.

Consequently, we know that ($x | SOME_FLAG) & SOME_FLAG === SOME_FLAG without knowing the value of $x. That in turn means that regardless of how the default value changes in future, we know what "default | JSON_PRETTY_PRINT" will do.

It's as though each bit flag is a separate parameter, and you're saying "pass true to $prettyPrint, but let the implementation decide sensible defaults for all other flags".

The majority of operators don't have that property, so they require some additional assumptions about the default, which might not hold in future. For instance, if you use "default + 1", you are implicitly assuming that the default value is not the maximum allowed value.

Rowan Tommins
[IMSoP]

Bilge

2 years ago

On 25/08/2024 22:09, Rowan Tommins [IMSoP] wrote:

>
> On 25 August 2024 21:00:03 BST, Bilge<bilge@scriptfusion.com>  wrote:
>> class Suspension {
>>      /**
>>       * @param int $delay Specifies the delay in milliseconds.
>>       */
>>      public function suspend(int $delay = 1_000) {
>>          var_dump($delay);
>>      }
>> }
>>
>> class MySuspension extends Suspension {
>>      /**
>>       * @param float|int|null $delay Specifies the delay in seconds.
>>       */
>>      public function suspend(float|int|null $delay = null) {
>>          parent::suspend((int)(($delay ?? 0) * 1000) ?: default);
>>      }
>> }
>>
>> new MySuspension()->suspend(2.2345); // int(2234)
>>
>> Not only have I demonstrated the need to use multiplication or division to change the scale, but also the need to cast.
> Possibly something got lost as you redrafted the example, because as you've written it, neither the multiplication nor the cast are applied to the value looked up by "default". The parameter reduces to "(expression) ?: default", which I've already agreed is useful.

Great! I'm glad we're finally getting to this, because I think this is
what you, and everyone advocating for a restricted grammar, is actually
missing. You think you've caught me in some kind of "gotcha" moment, but
fair warning, I'm about to play my Uno Reverse card.

What you're saying is that, somehow, even though the default must be
constrained to a restricted subset of permissible grammars, it is still
acceptable to have unrestricted expressions in the other operands. So,
in this example, somehow `expr ?: default` is OK.This is simply
impossible and would cause catastrophic shift/reduce conflicts in the
grammar. If `default` is to live in a restricted subset of allowed
expression grammars, then it can only recurse with those same
restrictions, meaning /both/ operands of any operators are so
restricted. Ergo I do not need to demonstrate the usefulness of applying
other operators /directly/ to `default`, merely including them
/somewhere/ in the expression is sufficient to demonstrate they are
useful because at that point we're back to recursing the general
expression grammar (free of any restrictions), unless and until you're
willing to concede those particular operators I've just demonstrated the
useful application for should be entered into the arbitrarily-selected
restricted subset of grammars allowed to apply to `default`.

If you believe I am incorrect about this, I encourage you to submit a
(working) Bison patch to demonstrate how a restricted expression grammar
subset can still recurse with the unrestricted superset, then we can
start having this discussion more seriously.

Cheers, Bilge

Rowan Tommins [IMSoP]

2 years ago

On 25 August 2024 22:51:45 BST, Bilge <bilge@scriptfusion.com> wrote:

>Great! I'm glad we're finally getting to this, because I think this is what you, and everyone advocating for a restricted grammar, is actually missing. You think you've caught me in some kind of "gotcha" moment, but fair warning, I'm about to play my Uno Reverse card.

You could have got to it much quicker by just saying it earlier, particularly when explaining how the current implementation is *not* the easy path.

I was not in the slightest thinking I'd caught any kind of "gotcha", I was repeating something I'd already said multiple times, that the *behaviour* I feel is justified is having "default" usable in the RHS of a ternary or coalesce.

I'm not an expert on parsers, and never claimed to be, so it's not particularly surprising to me that I've overlooked a reason why "expr ?: default" can't be included without also including "default ?: expr", and will just have to take your word for it.

It doesn't, unfortunately, persuade me that the behaviour proposed is sensible.

Rowan Tommins
[IMSoP]

Bilge

2 years ago

On 25/08/2024 23:31, Rowan Tommins [IMSoP] wrote:

> It doesn't, unfortunately, persuade me that the behaviour proposed is sensible.

It should. But since it has apparently failed in that regard, I suggest
you take me up on my challenge to implement the grammar you want with a
patch and you will quickly convince yourself one way or the other. The
truth doesn't exist in my head or yours, nor on this mailing list. The
truth always lies in the code, which is why RFC authors are strongly
encouraged to pursue patches where there is doubt, and similarly, I
think counter-proposals on the mailing list should follow suit,
otherwise we can find ourselves arguing over nothing.

Kind regards, Bilge

Rowan Tommins [IMSoP]

2 years ago

On 25 August 2024 23:42:20 BST, Bilge <bilge@scriptfusion.com> wrote:

>On 25/08/2024 23:31, Rowan Tommins [IMSoP] wrote:
>> It doesn't, unfortunately, persuade me that the behaviour proposed is sensible.
>
>It should. But since it has apparently failed in that regard, I suggest you take me up on my challenge to implement the grammar you want with a patch and you will quickly convince yourself one way or the other.

I think I have been perfectly consistent in saying that I am discussing the proposed language behaviour, not anything about how it could or should be implemented.

If it's a case of "unfortunately, doing the right thing is impossible, so we're proposing this compromise", then that's a reasonable position, but not how this has been presented. I also think it is perfectly reasonable to conclude that the compromise gives away too much.

In particular, I think allowing assignments and method calls to "read out" a value which was previously a private implementation detail accessible only through the Reflection API, is a significant language change with a net negative impact. If that's the required tradeoff to allow "(some expression) ?: default", then my position is we should do without it.

Regards,
Rowan Tommins
[IMSoP]

John Coggeshall

2 years ago

On Aug 25 2024, at 6:42 pm, Bilge <bilge@scriptfusion.com> wrote:

> On 25/08/2024 23:31, Rowan Tommins [IMSoP] wrote:
> > It doesn't, unfortunately, persuade me that the behaviour proposed is sensible.
>
> It should. But since it has apparently failed in that regard, I suggest you take me up on my challenge to implement the grammar you want with a patch and you will quickly convince yourself one way or the other. The truth doesn't exist in my head or yours, nor on this mailing list. The truth always lies in the code, which is why RFC authors are strongly encouraged to pursue patches where there is doubt, and similarly, I think counter-proposals on the mailing list should follow suit, otherwise we can find ourselves arguing over nothing.

That's not really how that works -- I mean it's not really up to anyone else to write a PR to implement their version of your RFC just because they disagree with (portions of) the concept.

Bob Weinand

2 years ago

On 26.8.2024 00:31:57, Rowan Tommins [IMSoP] wrote:

> I'm not an expert on parsers, and never claimed to be, so it's not particularly surprising to me that I've overlooked a reason why "expr ?: default" can't be included without also including "default ?: expr", and will just have to take your word for it.
>
> It doesn't, unfortunately, persuade me that the behaviour proposed is sensible.
>
> Rowan Tommins
> [IMSoP]

Hey Rowan,

just to state this:

It is almost never sensible to arbitrarily restrict grammars.

In the sense of "allow this expression just in a context of this given
list of expressions". Sure, in some cases the permitted grammar doesn't
make sense (like, why would we allow arithmetic operators on the
left-hand side of a coalesce operation "($a + $b) ?? $c"), but that's on
the user to write a minimal bit of sensible code.

I hope you can understand that; thanks,

Bob

John Coggeshall

2 years ago

On Aug 25 2024, at 5:51 pm, Bilge <bilge@scriptfusion.com> wrote:

> If you believe I am incorrect about this, I encourage you to submit a (working) Bison patch to demonstrate how a restricted expression grammar subset can still recurse with the unrestricted superset, then we can start having this discussion more seriously.

I don't think the restrictions being championed by Rowan (to which I concur) wouldn't be solved in the parser at compile time anyway -- Enforcement would have to happen in the VM at runtime during execution.

Christoph Becker

2 years ago

On 25.08.2024 at 23:51, Bilge wrote:

> If you believe I am incorrect about this, I encourage you to submit a
> (working) Bison patch to demonstrate how a restricted expression grammar
> subset can still recurse with the unrestricted superset, then we can
> start having this discussion more seriously.

It seems to me that the restriction does not have be enforced by the
parser, but *could* be enforced during compilation of the AST.  If that
*should* be done, is a different question.

Christoph

Bilge

2 years ago

On 26/08/2024 09:58, Christoph M. Becker wrote:

> On 25.08.2024 at 23:51, Bilge wrote:
>
>> If you believe I am incorrect about this, I encourage you to submit a
>> (working) Bison patch to demonstrate how a restricted expression grammar
>> subset can still recurse with the unrestricted superset, then we can
>> start having this discussion more seriously.
> It seems to me that the restriction does not have be enforced by the
> parser, but *could* be enforced during compilation of the AST.  If that
> *should* be done, is a different question.
>
> Christoph
>

Thanks Christoph.

You're absolutely right, I would be interested to see any viable patch
that effectively implements a set of restrictions on how `default` may
be used. Requesting it be done at the parser level was not meant as a
gotcha, that's just how I (with my lack of experience) would have
approached it, but certainly trapping cases in the compiler is equally,
if not more valid and/or practical.

Cheers,
Bilge

Rowan Tommins [IMSoP]

2 years ago

On Mon, 26 Aug 2024, at 10:14, Bilge wrote:

> You're absolutely right, I would be interested to see any viable patch
> that effectively implements a set of restrictions on how `default` may
> be used. Requesting it be done at the parser level was not meant as a
> gotcha, that's just how I (with my lack of experience) would have
> approached it, but certainly trapping cases in the compiler is equally,
> if not more valid and/or practical.

Another approach that occurred to me was in the executor: rather than evaluating to the default value immediately, "default" could resolve to a special value, essentially wrapping the reflection parameter info. Then when the function is actually called, it would be "unboxed" and the actual value fetched, but use in any other context would be a type error.

That would allow arbitrarily complex expressions to resolve to "default", but not perform any operations on it - a bit like propagating sqrt(-1) through an engineering formula where you know it will be cancelled out eventually.

I don't know if this is practical - I'm not sure how that special value would be represented - but I thought I'd mention it in case it sparks further ideas.

Larry Garfield

2 years ago

On Sun, Aug 25, 2024, at 10:29 AM, Bilge wrote:

> On 25/08/2024 14:35, Larry Garfield wrote:
>> The approach here seems reasonable overall.  The mental model I have from the RFC is "yoink the default value out of the function, drop it into this expression embedded in the function call, and let the chips fall where they may."  Is that about accurate? Yes, as it happens. That is the approach we took, because the alternative would have been changing how values are sent to functions, which would have required a lot more changes to the engine with no clear benefit. Internally it literally calls the reflection API, but a low-level call, that elides the class instantiation and unnecessary hoops of the public interface that would just slow it down. My main holdup is the need.  I... can't recall ever having a situation where this is something I needed.  Some of the examples show valid use cases (eg, the "default plus this binary flag" example), but again, I've never actually run into that myself in practice. That's fine. Not everyone will have such a need, and of those that do, I'm willing to bet it will be rare or uncommon at best. But for those times it is needed, the frequency by which it is needed in no way diminishes its usefulness. I rarely use `goto` but that doesn't mean we shouldn't have the feature. My other concern is the list of supported expression types.  I understand how the implementation would naturally make all of those syntactically valid, but it seems many of them, if not most, are semantically nonsensical.  Eg, `default > 1` would take a presumably numeric default value and output a boolean, which should really never be type compatible with the function being called.  (A param type of int|bool is a code smell at best, and a fatal waiting to happen at worst.)  In practice, I think a majority of those expressions would be logically nonsensical, so I wonder if it would be better to only allow a few reasonable ones and block the others, to keep people from thinking nonsensical code would do something useful.

> Since you're not the only one raising this, I will address it, but just
> to say there is no good reason, in my mind, to ever prohibit the
> expressiveness. To quote Rob
>
>>I'm reasonably certain you can write nonsensical PHP without this feature. I don't think we should be the nanny of developers.

See, I approach it from an entirely different philosophical perspective:

To the extent possible, the language and compiler should prevent you from doing stupid things, or at least make doing stupid things harder.

This is the design philosophy behind, well, most good user interfaces.  It's why it's good that US and EU power outlets are different, because they run different voltages, and blindly plugging one into the other can cause damage or death.

This is the design philosophy behind all type systems: Make illogical or dangerous or "we know it can't work" code paths a compile error, or even impossible to express at all.

This is the design philosophy behind password_hash() and friends: The easy behavior is, 99% of the time, the right one, so doing the "right thing" is easy.  Doing something dumb (like explicitly setting password_hash() to use md5 or something) may be possible, but it requires extra work to be dumb.

Good design makes the easy path the safe path.

Now, certainly, the definition of "stupid things" is subjective and squishy, and reasonable people can disagree on where that threshold is.  That's what a robust discussion is for, to figure out what qualifies as a "stupid thing" in this case.

Rob has shown some possible, hypothetical uses for some of the seemingly silly possible combinations, which may or may not carry weight with people.  But there are others that are still unjustified, so for now, I would still put "default != 5" into the "stupid things" category, for example.

As you've noted, this is already applicable only in some edge cases to begin with, so enabling edge cases of edge cases that only maybe make sense if you squint is very likely in the "stupid things" territory.

> I fully agree with that sentiment. It seems to be biting me that I went
> to the trouble of listing out every permutation of what *expression*
> means where perhaps this criticism would not have been levied at all
> had I chosen not to do so.

From one RFC author to another, it's better to make that list explicitly and let us collectively think through the logic of it than to be light on details and not realize what will break until later.  We've had RFCs that did that, and it caused problems.  The discussion can absolutely be frustrating (boy do I know), but the language is better for it.  So I'm glad you did call it out so we could have this discussion.

--Larry Garfield

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 14:35, Larry Garfield wrote:

> My other concern is the list of supported expression types.  I
> understand how the implementation would naturally make all of those
> syntactically valid, but it seems many of them, if not most, are
> semantically nonsensical.

I tend to agree with Larry and John that the list of operators should be
restricted - we can always allow more in future, but restricting later
is much harder.

A few rules that seem logical to me:

1) The expression should be reasonably guaranteed to produce the same
type as the actual default.

- No casts
- No comparison operators, because they produce booleans from
non-boolean input
- No "<=>". Technically, it has an integer result, but it's rare to use
it as one, rather than a kind of three-value boolean
- No "instanceof"
- No "empty"

2) The expression should not have side effects (outside of exotic
operator overloads).

- No "include", "require", etc
- No "throw"
- No "print"
- Borderline, but I would also say no "clone"

3) The expression should be passing additional information into the
function, not pulling information out of it. The syntax shouldn't be a
way to write obfuscated reflection, or invert data flow from callee to
caller.

- No assignments.
- No ternaries with "default" on the left-hand side - "$foo ? $bar :
default" is acting on local knowledge, but "default ? $foo : $bar" is
acting on information the caller shouldn't know
- Same for "?:" and "??"
- No "match" with "default" as the condition or branch, for the same
reason. "match($foo) { $bar => default }" is fine, match(default) { ...
}" or "match($foo) { default => ... }" are not.

Note that these can be seen as aspects of the same rule: the aim of the
expression should be to transform the default value into another value
of the same type, not to pull it out and perform arbitrary operations
based on it.


I believe that leaves us with:

- Arithmetic operators: binary + - * / % **, unary + -
- Bitwise operators: & | ^ << >>  ~
- Boolean operators: && || and or xor !
- Conditions with default on the RHS: $foo ? $bar : default, $foo ?:
default, $foo ?? default, match($foo) { $bar => default }
- Parentheses: (((default)))


Even then, I look at that list and see more problems than use cases. As
the RFC points out, library authors already worry about the maintenance
burden of named argument support, will they now also need to question
whether someone is relying on "default + 1" having some specific effect?

Maybe we should instead require justification for each addition:

- Bitwise | is nicely demonstrated in the RFC
- Bitwise & could probably be justified on similar grounds
- "$foo ? $bar : default" is discussed in the RFC
- The other "conditions with default on the RHS" in my shortlist above
fit the same basic use case

Beyond that, I'm struggling to think of meaningful uses: "whatever the
function sets as its default, do the opposite"; "whatever number the
function sets as default, raise it to the power of 3"; etc. Again, they
can easily be added in later versions, if a use case is pointed out.


Regards,

--
Rowan Tommins
[IMSoP]

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 17:31, Rowan Tommins [IMSoP] wrote:

> On 25/08/2024 14:35, Larry Garfield wrote:
>> My other concern is the list of supported expression types.  I
>> understand how the implementation would naturally make all of those
>> syntactically valid, but it seems many of them, if not most, are
>> semantically nonsensical.
>
>
> I tend to agree with Larry and John that the list of operators should be restricted - we can always allow more in future, but restricting later is much harder.
>
> A few rules that seem logical to me:
>
> 1) The expression should be reasonably guaranteed to produce the same type as the actual default.
>
>
> - No casts
> - No comparison operators, because they produce booleans from non-boolean input
> - No "<=>". Technically, it has an integer result, but it's rare to use it as one, rather than a kind of three-value boolean
> - No "instanceof"
> - No "empty"
>
> 2) The expression should not have side effects (outside of exotic operator overloads).
>
>
> - No "include", "require", etc
> - No "throw"
> - No "print"
> - Borderline, but I would also say no "clone"
>
> 3) The expression should be passing additional information into the function, not pulling information out of it. The syntax shouldn't be a way to write obfuscated reflection, or invert data flow from callee to caller.
>
>
> - No assignments.
> - No ternaries with "default" on the left-hand side - "$foo ? $bar : default" is acting on local knowledge, but "default ? $foo : $bar" is acting on information the caller shouldn't know
> - Same for "?:" and "??"
> - No "match" with "default" as the condition or branch, for the same reason. "match($foo) { $bar => default }" is fine, match(default) { ... }" or "match($foo) { default => ... }" are not.
>
> Note that these can be seen as aspects of the same rule: the aim of the expression should be to transform the default value into another value of the same type, not to pull it out and perform arbitrary operations based on it.
>
>
>
> I believe that leaves us with:
>
>
> - Arithmetic operators: binary + - * / % **, unary + -
> - Bitwise operators: & | ^ << >>  ~
> - Boolean operators: && || and or xor !
> - Conditions with default on the RHS: $foo ? $bar : default, $foo ?: default, $foo ?? default, match($foo) { $bar => default }
> - Parentheses: (((default)))
>
>
>
> Even then, I look at that list and see more problems than use cases. As the RFC points out, library authors already worry about the maintenance burden of named argument support, will they now also need to question whether someone is relying on "default + 1" having some specific effect?
>
> Maybe we should instead require justification for each addition:
>
>
> - Bitwise | is nicely demonstrated in the RFC
> - Bitwise & could probably be justified on similar grounds
> - "$foo ? $bar : default" is discussed in the RFC
> - The other "conditions with default on the RHS" in my shortlist above fit the same basic use case
>
> Beyond that, I'm struggling to think of meaningful uses: "whatever the function sets as its default, do the opposite"; "whatever number the function sets as default, raise it to the power of 3"; etc. Again, they can easily be added in later versions, if a use case is pointed out.
>
>
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]

Hi Rowan, you went through a lot of trouble to write this out, and the reasoning makes sense to me. However, all the nonsensical things you say shouldn’t be allowed are already perfectly allowed today, you just have to type a bunch of boilerplate reflection code. There is no new behavior here, just new syntax.

— Rob

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 16:54, Rob Landers wrote:

> Hi Rowan, you went through a lot of trouble to write this out, and the
> reasoning makes sense to me. However, all the nonsensical things you
> say shouldn’t be allowed are already perfectly allowed today, you just
> have to type a bunch of boilerplate reflection code. There is no new
> behavior here, just new syntax.

Firstly, your response to John was essentially "please give more
details" [https://externals.io/message/125183#125214], and your response
to me is "thanks for the details, but I'm not going to engage with
them". That's a bit frustrating.

Secondly, I don't think "it's possible with half a dozen lines of
reflection, so it's fine for it to be a first-class feature of the
language syntax" is a strong argument. The Reflection API is a bit like
the Advanced Settings panel in a piece of software, it comes with a big
"Proceed with Caution" warning. You only move something from that
Advanced Settings panel to the main UI when it's going to be commonly
used, and generally safe to use. I don't think allowing arbitrary
operations on a value that's declared as the default of some other
function passes that test.

Regards,

--
Rowan Tommins
[IMSoP]

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 18:21, Rowan Tommins [IMSoP] wrote:

> On 25/08/2024 16:54, Rob Landers wrote:
> > Hi Rowan, you went through a lot of trouble to write this out, and the
> > reasoning makes sense to me. However, all the nonsensical things you
> > say shouldn’t be allowed are already perfectly allowed today, you just
> > have to type a bunch of boilerplate reflection code. There is no new
> > behavior here, just new syntax.
>
>
> Firstly, your response to John was essentially "please give more
> details" [https://externals.io/message/125183#125214], and your response
> to me is "thanks for the details, but I'm not going to engage with
> them". That's a bit frustrating.

Oh, my apologies! That wasn’t my intention! With John and yourself, I do agree with you. I’m just trying to understand the logic in limiting it. As in, “I intuitively feel the same way but I don’t know why but maybe you do.” Intuition sucks sometimes.

>
> Secondly, I don't think "it's possible with half a dozen lines of
> reflection, so it's fine for it to be a first-class feature of the
> language syntax" is a strong argument. The Reflection API is a bit like
> the Advanced Settings panel in a piece of software, it comes with a big
> "Proceed with Caution" warning. You only move something from that
> Advanced Settings panel to the main UI when it's going to be commonly
> used, and generally safe to use. I don't think allowing arbitrary
> operations on a value that's declared as the default of some other
> function passes that test.
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]
>

That makes sense, but is it uncommon because it is hard and slow, or because it is genuinely not a common need?

— Rob

John Coggeshall

2 years ago

On Aug 25 2024, at 11:31 am, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:

>
>
> Even then, I look at that list and see more problems than use cases. As the RFC points out, library authors already worry about the maintenance burden of named argument support, will they now also need to question whether someone is relying on "default + 1" having some specific effect?
> Maybe we should instead require justification for each addition:
> - Bitwise | is nicely demonstrated in the RFC
> - Bitwise & could probably be justified on similar grounds
> - "$foo ? $bar : default" is discussed in the RFC
> - The other "conditions with default on the RHS" in my shortlist above fit the same basic use case
>
>

IMO the operations that make sense in this context are:
- Some Bitwise operators: & | ^
- Conditions with default on the RHS: $foo ? $bar : default, $foo ?: default, $foo ?? default, match($foo) { $bar => default }
- Parentheses: (((default)))

> Beyond that, I'm struggling to think of meaningful uses: "whatever the function sets as its default, do the opposite"; "whatever number the function sets as default, raise it to the power of 3"; etc. Again, they can easily be added in later versions, if a use case is pointed out.

I 100% agree.

> G((default)->F()); // lol
> Special-casing the T_DEFAULT grammar would not only bloat the grammar rules but also increase the chance that new expression grammars introduced in future, which could conveniently interoperate with `default`, would be unintentionally excluded by omission.

I won't vote for this RFC if the above code is valid, FWIW. Unlike include , default is a special-case with a very specific purpose -- one that is reaching into someone else's API in a way the developer of that library doesn't explicitly permit. It should not become a fast easy way to inject a new potentially complex dependency which is what allowing a full expression support would allow.
The fact that Reflection allows me to pull out a private member doesn't mean accessing private members of objects should be given its own language syntax.
Frankly, not only should the op list be limited but ideally it should also only be valid based on the type of the upstream API call (e.g. bitwise operators should only be valid if the upstream API call has a type int ).

John Coggeshall

2 years ago

> Special-casing the T_DEFAULT grammar would not only bloat the grammar rules but also increase the chance that new expression grammars introduced in future, which could conveniently interoperate with `default`, would be unintentionally excluded by omission.

Forgot to add that I don't think the fact doing this properly requires a more complex grammar is a strong argument for doing it "the easy way" of allowing all expressions. It's a special case, and that should be reflected in the grammar.

John Bafford

2 years ago

Hi Rowan,

> On Aug 25, 2024, at 11:31, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:
>
> 3) The expression should be passing additional information into the function, not pulling information out of it. The syntax shouldn't be a way to write obfuscated reflection, or invert data flow from callee to caller.
> - No assignments.
> - No ternaries with "default" on the left-hand side - "$foo ? $bar : default" is acting on local knowledge, but "default ? $foo : $bar" is acting on information the caller shouldn't know
> - Same for "?:" and "??"
> - No "match" with "default" as the condition or branch, for the same reason. "match($foo) { $bar => default }" is fine, match(default) { ... }" or "match($foo) { default => ... }" are not.

I think this brings up a good question on what exactly should be intended to be public API. Currently, there's two main ways to effectively write a default parameter:

function foo(int $param = 42) {}
function bar(?int $param) { $param ??= 42; }

In the former, the default value is listed in the function declaration, along with the function name, and parameter type and name, which are already part of the public interface.
In the latter, the default value is an implementation detail of the function, and is not part of the function declaration.

(You could also ?int $param = 42, but I'd argue that at that point, if you really need to distinguish among the set of (unspecified, null, value), you're better off with an ADT, which we don't have yet. And you can also explicitly expose a default value as a static value on a type, when a function itself doesn't want to/shouldn't make the policy decision of what the default is.)

Although I'm not sold on the idea of using default as part of an expression, I would argue that a default function parameter value is fair game to be read and manipulated by callers. If the default value was intended to be private, it shouldn't be in the function declaration.

One important case where reading the default value could be important is in interoperability with different library versions. For example, a library might change a default parameter value between versions. If you're using the library, and want to support both versions, you might both not want to set the value, and yet also care what the default value is from the standpoint of knowing what to expect out of the function.

-John

Rowan Tommins [IMSoP]

2 years ago

On 25/08/2024 18:44, John Bafford wrote:

> Although I'm not sold on the idea of using default as part of an
> expression, I would argue that a default function parameter value is
> fair game to be read and manipulated by callers. If the default value
> was intended to be private, it shouldn't be in the function declaration.

There's an easy argument against this interpretation: child classes can
freely change the default value for a parameter, as long as they do not
make it mandatory. https://3v4l.org/SEsRm

That matches my intuition: that the public API, as a contract, states
that the parameter is optional; the specification of what happens when
it is not provided is an implementation detail.

For comparison, consider constructor property promotion; the caller
shouldn't know or care whether a class is defined as:

public function __construct(private int $bar) {}

or:

private int $my_bar;
public function __construct(int $bar) { $this->my_bar = $bar; }

The syntax sits in the function signature because it's convenient, not
because it's part of the API.

> One important case where reading the default value could be important is
>   in interoperability with different library versions. For example, a
> library might change a default parameter value between versions. If
> you're using the library, and want to support both versions, you might
> both not want to set the value, and yet also care what the default value
>   is from the standpoint of knowing what to expect out of the function.

This seems contradictory to me. If you use the default, you're telling
the library that you don't care about that parameter, and trust it to
provide a default.

If you want to know what the library did with its arguments, reflecting
the signature will never be enough anyway. For example, it's quite
common to write code like this:

function foo(?SomethingInterface $blah = null) {
    if ( $blah === null ) {
        $blah = self::_setup_default_blah();
    }
    // ...
}

A caller can't tell by looking at the signature that a new version of
the library has changed what _setup_default_blah() returns. If the
library doesn't provide an API to get $blah out later, then it's a
private detail that the caller has no business inspecting.


Regards,

--
Rowan Tommins
[IMSoP]

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 20:46, Rowan Tommins [IMSoP] wrote:

> On 25/08/2024 18:44, John Bafford wrote:
>
>> Although I'm not sold on the idea of using default as part of an
>> expression, I would argue that a default function parameter value is
>> fair game to be read and manipulated by callers. If the default value
>> was intended to be private, it shouldn't be in the function declaration.
>
>
> There's an easy argument against this interpretation: child classes can freely change the default value for a parameter, as long as they do not make it mandatory. https://3v4l.org/SEsRm
>
> That matches my intuition: that the public API, as a contract, states that the parameter is optional; the specification of what happens when it is not provided is an implementation detail.
>
> For comparison, consider constructor property promotion; the caller shouldn't know or care whether a class is defined as:
>
> public function __construct(private int $bar) {}
>
> or:
>
>
> private int $my_bar;
> public function __construct(int $bar) { $this->my_bar = $bar; }
>
> The syntax sits in the function signature because it's convenient, not because it's part of the API.
>
>
>
>> One important case where reading the default value could be important is
>>  in interoperability with different library versions. For example, a
>> library might change a default parameter value between versions. If
>> you're using the library, and want to support both versions, you might
>> both not want to set the value, and yet also care what the default value
>>  is from the standpoint of knowing what to expect out of the function.
>
>
> This seems contradictory to me. If you use the default, you're telling the library that you don't care about that parameter, and trust it to provide a default.
>
> If you want to know what the library did with its arguments, reflecting the signature will never be enough anyway. For example, it's quite common to write code like this:
>
>
> function foo(?SomethingInterface $blah = null) {
>     if ( $blah === null ) {
>         $blah = self::_setup_default_blah();
>     }
>     // ...
> }
>
> A caller can't tell by looking at the signature that a new version of the library has changed what _setup_default_blah() returns. If the library doesn't provide an API to get $blah out later, then it's a private detail that the caller has no business inspecting.
>
>
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]

I think you've hit an interesting point here, but probably not what you intended.

For example, let's consider this function:

json_encode(mixed $value, int $flags = 0, int $depth = 512): string|false

Already, you have to look up the default value of depth or set it to something that makes sense, as well as $flags. So you do this:

json_encode($value, JSON_THROW_ON_ERROR, 512);

You are doing this even when you omit the default. If you set it to a variable to spell it out:

$default_flags = 0 | JSON_THROW_ON_ERROR;
$default_depth = 512; // according to docs on DATE

json_encode($value, $default_flags, $default_depth);

Can now be rewritten:

json_encode($value, $default_flags = default | JSON_THROW_ON_ERROR, $default_depth = default);

This isn't just reflection, this is saving me from having to look up the docs/implementation and hardcode values. The implementation is free to change them, and my code will "just work."

Now, let's look at a more non-trivial case from some real-life use-cases, in the form of a plausible story:

public function __construct(
private LoggerInterface|null $logger = null,
private string|null $name = null,
Level|null $level = null,
)

This code constructs a new logger composed from an already existing logger. When constructing it, I may look up what the default values are and decide if I want to override them or not. Otherwise, I will leave it as null.

A coworker and I got to talking about this interface. It kind of sucks, and we don't like it. It's been around for ages, so we are worried about changing it. Specifically, we are wondering if we should use SuperNullLogger as the default instead of null (which happens to just create a NullLogger a few lines later). We are pretty sure making this change won't cause any issues, but to be extra safe, we will do it only on a single code path; further, we are 100% sure we are going to change this signature, so we need to do it in a forward-compatible way. Thus, we will set it to SuperNullLogger if-and-only-if the default value is null:

default ?? new SuperNullLogger()

Now, we can run this in production and see how well it performs. Incidentally, we discover that NullLogger implementation is superior and we can now change the default:

public function __construct(
private LoggerInterface $logger = new NullLogger(),
private string|null $name = null,
Level|null $level = null,
)

That one code path "magically" updates as soon as the library is updated, without having to make further changes. Anything that is hardcoded "null" will break in tests/static analysis, making it easy to locate. Further, we can test other types of NullLoggers just as easily:

default instanceof NullLogger ? new BasicNullLogger() : default

So, yes, I think in isolation the feature might look strange, and some operations might look nonsensical, but I believe there is a use case here that was previously rather hard to do; or statically done via someone looking up some documentation/code and doing a search-and-replace.

— Rob

John Bafford

2 years ago

On Aug 25, 2024, at 14:46, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:

>
> On 25/08/2024 18:44, John Bafford wrote:
>
>> Although I'm not sold on the idea of using default as part of an
>> expression, I would argue that a default function parameter value is
>> fair game to be read and manipulated by callers. If the default value
>> was intended to be private, it shouldn't be in the function declaration.
>
> There's an easy argument against this interpretation: child classes can freely change the default value for a parameter, as long as they do not make it mandatory. https://3v4l.org/SEsRm
> That matches my intuition: that the public API, as a contract, states that the parameter is optional; the specification of what happens when it is not provided is an implementation detail.
> For comparison, consider constructor property promotion; the caller shouldn't know or care whether a class is defined as:
> public function __construct(private int $bar) {}
> or:
> private int $my_bar;
> public function __construct(int $bar) { $this->my_bar = $bar; }
> The syntax sits in the function signature because it's convenient, not because it's part of the API.

This is only by current convention. It used to be that parameter names were not part of the API contract, but now with named parameters, they are. There's no reason default values couldn't (or shouldn't) become part of the API contract in the same way.

(Note that in some other languages, default parameter values are not only part of the API contract, but they're emitted into the clients when compiled, so an API can change/add/remove its default values and the client continues to function as it used to with the value as defined at compile time. This doesn't currently matter for PHP, where you have the full source to anything you run, but could become important later if PHP gained ahead-of-time compiled binary modules.)

>> One important case where reading the default value could be important is
>> in interoperability with different library versions. For example, a
>> library might change a default parameter value between versions. If
>> you're using the library, and want to support both versions, you might
>> both not want to set the value, and yet also care what the default value
>> is from the standpoint of knowing what to expect out of the function.
>
> This seems contradictory to me. If you use the default, you're telling the library that you don't care about that parameter, and trust it to provide a default.
> If you want to know what the library did with its arguments, reflecting the signature will never be enough anyway. For example, it's quite common to write code like this:
> function foo(?SomethingInterface $blah = null) {
>     if ( $blah === null ) {
>         $blah = self::_setup_default_blah();
>     }
>     // ...
> }
> A caller can't tell by looking at the signature that a new version of the library has changed what _setup_default_blah() returns. If the library doesn't provide an API to get $blah out later, then it's a private detail that the caller has no business inspecting.

Well, but that's the private default example I described. In that case you're not intended to be able to reason about what the default is, because it's a private implementation detail of the function, as opposed to being expressed in the parameter list. Although, if it weren't intended to be an implementation detail, the only thing stopping you from writing in the parameter list like this:

function foo(SomethingInterface $blah = self::_setup_default_blah()) {...}

is because PHP doesn't currently allow default values to be computed at runtime. (Maybe it should.)

-John

Rowan Tommins [IMSoP]

2 years ago

On 25 August 2024 21:29:45 BST, John Bafford <jbafford@zort.net> wrote:

>This is only by current convention. It used to be that parameter names were not part of the API contract, but now with named parameters, they are.

Indeed, and it remains highly controversial among library authors, and is even used as a justification for this RFC.

> There's no reason default values couldn't (or shouldn't) become part of the API contract in the same way.

I agree that they *could*, but I am making the case that they *should not*. The ability to specify an optional parameter which is subject to change is a very useful one, frequently used. If a library author *wants* to expose the default value for manipulation by users, they can make it available as a constant, as with e.g. PASSWORD_DEFAULT.

I can definitely see the use in features that enhance the existing use of default parameters to mean "I trust the implementation to do the right thing, and am not interested in specifying this parameter". (And see my earlier post on how bit flags can still fit it into this meaning.)

None of the examples so far have persuaded me that there is sufficient value in extending that to "every optional parameter also acts a public constant that the caller can read out and act on at will".

Rowan Tommins
[IMSoP]

John Coggeshall

2 years ago

On Aug 24 2024, at 12:49 pm, Bilge <bilge@scriptfusion.com> wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>

Seems like you are missing an option for your theme example, which would be to simply extend the Config class?
While I can see the value in the concept of default , I think it's a mistake to allow default to be used as a generic operand as shown in the RFC appendix. It seems to me that the whole point of something like default is to not have to worry about what the upstream API wanted for that value, but the second you start allowing operations where default is an operand you've reintroduced the problem you were trying to avoid if the upstream API were to change the type of what default ultimately resolves to. Worse actually because now I have no idea what default is when I read code without having to dig up the upstream API.
Other thoughts here are what happens when default resolves to an object or enumeration or something complex? Your original example had CuteTheme , so can you call a method of default ?? I could entirely see someone doing something like this for example:
enum Foo:string {
// cases

public function buildSomeValidBasedOnCase(): int { // ... }
}

F(MyClass::makeBasedOnValue(default->buildSomeValidBasedOnCase()))
IMO most operators listed in the appendix should be disallowed. I can see the value of default | JSON_PRETTY_PRINT, but I am pretty strongly opposed to the idea of introducing a "conditional based on the default value of an upstream API call" concept of default >=1 .

Matthew Weier O'Phinney

2 years ago

On Sun, Aug 25, 2024, 9:06 AM John Coggeshall <john@coggeshall.org> wrote:

>
>
> On Aug 24 2024, at 12:49 pm, Bilge <bilge@scriptfusion.com> wrote:
>
> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>
>
> Seems like you are missing an option for your theme example, which would
> be to simply extend the Config  class?
>
> While I can see the value in the concept of default , I think it's a
> mistake to allow default  to be used as a generic operand as shown in the
> RFC appendix. It seems to me that the whole point of something like
> default  is to not have to worry about what the upstream API wanted for
> that value, but the second you start allowing operations where default
> is an operand you've reintroduced the problem you were trying to avoid if
> the upstream API were to change the type of what default  ultimately
> resolves to. Worse actually because now I have no idea what default  is
> when I read code without having to dig up the upstream API.
>

If the underlying API changes the argument type, consumers will have an
issue regardless. For those cases where the expression is simply `default`,
you'd actually be protected from the API change, which is a net benefit
already.

This also protects the user from changes in the argument names.

John Coggeshall

2 years ago

> If the underlying API changes the argument type, consumers will have an issue regardless. For those cases where the expression is simply `default`, you'd actually be protected from the API change, which is a net benefit already.
>
> This also protects the user from changes in the argument names.

As I said, I don't have a particular problem with default as a keyword to express "whatever the default value might be in the function declaration", but I do have some real concerns about its use as an operand in an expression. The RFC provides for a single valid use case of operators (i.e. things like default | JSON_PRETTY_PRINT ), yet calls for a huge array of valid operations, many of which the RFC itself notes don't make much / any sense. I'd personally like to see this RFC dramatically reduce the scope of operations supported with default as an operand initially (e.g. perhaps only bitwise ops), and revisit additional operations as needed down the road. IMO there is a very small subset of all PHP operators that make any sense at all in this context, and even fewer that I think are a good idea to allow even if they might make some sort of sense.

Rob Landers

2 years ago

On Sun, Aug 25, 2024, at 16:58, John Coggeshall wrote:

>
>
>> If the underlying API changes the argument type, consumers will have an issue regardless. For those cases where the expression is simply `default`, you'd actually be protected from the API change, which is a net benefit already.
>>
>> This also protects the user from changes in the argument names.
>
> As I said, I don't have a particular problem with `default`  as a keyword to express "whatever the default value might be in the function declaration", but I do have some real concerns about its use as an operand in an expression. The RFC provides for a single valid use case of operators (i.e. things like `default | JSON_PRETTY_PRINT` ), yet calls for a huge array of valid operations,  many of which the RFC itself notes don't make much / any sense. I'd personally like to see this RFC dramatically reduce the scope of operations supported with `default`  as an operand initially (e.g. perhaps only bitwise ops), and revisit additional operations as needed down the road. IMO there is a very small subset of all PHP operators that make any sense at all in this context, and even fewer that I think are a good idea to allow even if they might make some sort of sense.

Which operants don’t make sense?

— Rob

John Coggeshall

2 years ago

On Aug 25 2024, at 11:11 am, Rob Landers <rob@bottled.codes> wrote:

>
> Which operants don’t make sense?

Well certainly all of the ones toward the end of the appendix in the RFC the RFC itself notes are non-sensical. Personally, I'm not sold on the idea default should be an operand in an expression at all though.
I do see the value of bitwise operators / expressions as highlighted in the RFC. There might be other cases, but I'm wary of them -- I don't think giving developers the ability to write logic and expressions against hard-coded default values in upstream APIs has a lot of merit in most cases. I can tell you I doubt I would ever support the idea of calling methods of default , which I'm still unclear if this RFC is proposing.
I would suggest a compromise where the RFC be refocused to those expressions and operators that explicitly make sense, and leave the rest of them out for now. To me, that basically means "default with bitwise expression support" based on what I've seen so far.
John

Bilge

2 years ago

On 25/08/2024 15:04, John Coggeshall wrote:

> Other thoughts here are what happens when |default| resolves to an
> object or enumeration or something complex? Your original example had
> |CuteTheme| , so can you call a method of |default| ?? I could
> entirely see someone doing something like this for example:
>
> enum Foo:string {
>     // cases
>
>     public function buildSomeValidBasedOnCase(): int { // ... }
> }
>
> F(MyClass::makeBasedOnValue(default->buildSomeValidBasedOnCase()))

As you have written it, no, you will get a parser error: Parse error:
syntax error, unexpected token "->", expecting ")"

However, you can wrap the `default` in parens as in the following example:

class C {
    function F() {
        echo 'lol';
    }
}

function G($V = new C) {}

G((default)->F()); // lol

Andreas Leathley

2 years ago

On 24.08.24 18:49, Bilge wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've
> been cooking for a little while. Considering I don't know C or PHP
> internals, one might think implementing this feature would be
> prohibitively difficult, but considering the amount of help and
> guidance I received from Ilija, Bob and others, it would be truer to
> say it would have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge

Hello Paul,

I think this is an interesting addition to the language. Personally, I
would replace the full expression list at the end of the RFC with more
examples in real-world scenarios for most of these cases. As far as I
skimmed the discussion, there is some worry of "wrong use" (which I do
not necessarily share). Showing more examples could be useful to focus
on how having default being a full expression gives interesting use
cases, instead of talking about what (in isolation) nonsensical code
people might write.

For me there is another question. When using interfaces and classes,
default values can be introduced, like this:

interface CompressionInterface
{
    public function compress(string $data, int $level): string;
}

class GzipCompression implements CompressionInterface
{
    public function compress(string $data, int $level = 4): string
    {
        // do something
    }
}

When I have the GzipCompression class, I would know there is a default
value for $level, but when using the interface there might or might not
be a default value, depending on the implementation. As far as I read
the RFC, using "default" when there is no default would lead to a
runtime exception, but there is no way of finding out if there is a
default if you do not already know. Being able to test that could be
useful, although I am not sure about the syntax for that. In the example
when getting CompressionInterface, I might test for the existence of a
default value of $level and leave it at the default if there is a
default (maybe I know that some implementations have a default value,
others don't). One could test the specific implementation with
instanceof checks, but the advantage of "default" could be that you do
not need to know the implementation and could only adapt to possibly
defined default values.

Bilge

2 years ago

On 26/08/2024 10:03, Andreas Leathley wrote:

> On 24.08.24 18:49, Bilge wrote:
>
> For me there is another question. When using interfaces and classes,
> default values can be introduced, like this:
>
> interface CompressionInterface
> {
>     public function compress(string $data, int $level): string;
> }
>
> class GzipCompression implements CompressionInterface
> {
>     public function compress(string $data, int $level = 4): string
>     {
>         // do something
>     }
> }
>
> When I have the GzipCompression class, I would know there is a default
> value for $level, but when using the interface there might or might not
> be a default value, depending on the implementation. As far as I read
> the RFC, using "default" when there is no default would lead to a
> runtime exception, but there is no way of finding out if there is a
> default if you do not already know. Being able to test that could be
> useful, although I am not sure about the syntax for that. In the example
> when getting CompressionInterface, I might test for the existence of a
> default value of $level and leave it at the default if there is a
> default (maybe I know that some implementations have a default value,
> others don't). One could test the specific implementation with
> instanceof checks, but the advantage of "default" could be that you do
> not need to know the implementation and could only adapt to possibly
> defined default values.

Hi Andreas,

Thanks for this question; I find this super interesting because it's
something we haven't thought about yet. I must admit I completely
overlooked that, whilst an interface /can/ require implementers to
specify a default, in the case that they do not, it is still valid for
implementations to selectively elect to provide one. Therefore I can
append to your example the following case (I removed the `string` return
type for now):

class ZipCompression implements CompressionInterface
{
    public function compress(string $data, int $level)
    {
        var_dump($level);
    }
}

new GzipCompression()->compress('', default ?? 6);
new ZipCompression()->compress('', default ?? 6);

In this case, we get the following output:

int(4)
Fatal error: Uncaught ValueError: Cannot pass default to required
parameter 2 of ZipCompression::compress()

I would like to fix this if possible, because I think this should be
valid, with emphasis on /if possible/, because it may be prohibitively
complex. Will update later.

Cheers,
Bilge

Andreas Leathley

2 years ago

On 26.08.24 11:26, Bilge wrote:

>
> Thanks for this question; I find this super interesting because it's
> something we haven't thought about yet. I must admit I completely
> overlooked that, whilst an interface /can/ require implementers to
> specify a default, in the case that they do not, it is still valid for
> implementations to selectively elect to provide one. Therefore I can
> append to your example the following case (I removed the `string`
> return type for now):
>
> class ZipCompression implements CompressionInterface
> {
>     public function compress(string $data, int $level)
>     {
>         var_dump($level);
>     }
> }
>
> new GzipCompression()->compress('', default ?? 6);
> new ZipCompression()->compress('', default ?? 6);
>
> In this case, we get the following output:
>
> int(4)
> Fatal error: Uncaught ValueError: Cannot pass default to required
> parameter 2 of ZipCompression::compress()
>
> I would like to fix this if possible, because I think this should be
> valid, with emphasis on /if possible/, because it may be prohibitively
> complex. Will update later.
>

That would be a way to fix it, to basically make isset(default) a
possible check if there is no default, similar to an undefined variable
check. It would also recognize a default value of null as not set in the
same way, so one could not differentiate between null and not defined,
but that is in line with the language in general.

Bilge

2 years ago

On 26/08/2024 11:11, Andreas Leathley wrote:

> On 26.08.24 11:26, Bilge wrote:
>>
>> I would like to fix this if possible, because I think this should be
>> valid, with emphasis on /if possible/, because it may be
>> prohibitively complex. Will update later.
>>
> That would be a way to fix it, to basically make isset(default) a
> possible check if there is no default, similar to an undefined
> variable check. It would also recognize a default value of null as not
> set in the same way, so one could not differentiate between null and
> not defined, but that is in line with the language in general.

It would not be possible to write `isset(default)` because `isset()`
does not operate on expressions. Similarly, it would not be possible to
write `default === null ? ... : ...` because you would receive the same
error as above. If we special-case null coalesce then it will literally
only be possible to check with null coalesce.

As for feasibility, I definitely believe it is feasible. Though some may
argue whether it's worth the trouble for this edge case, I would still
like to implement it.

Cheers,
Bilge

Andreas Heigl

2 years ago

Hey folks.

Am 26.08.24 um 11:26 schrieb Bilge:

> On 26/08/2024 10:03, Andreas Leathley wrote:
>> On 24.08.24 18:49, Bilge wrote:
>>
>> For me there is another question. When using interfaces and classes,
>> default values can be introduced, like this:
>>
>> interface CompressionInterface
>> {
>>     public function compress(string $data, int $level): string;
>> }
>>
>> class GzipCompression implements CompressionInterface
>> {
>>     public function compress(string $data, int $level = 4): string
>>     {
>>         // do something
>>     }
>> }
>>
>> When I have the GzipCompression class, I would know there is a default
>> value for $level, but when using the interface there might or might not
>> be a default value, depending on the implementation. As far as I read
>> the RFC, using "default" when there is no default would lead to a
>> runtime exception, but there is no way of finding out if there is a
>> default if you do not already know. Being able to test that could be
>> useful, although I am not sure about the syntax for that. In the example
>> when getting CompressionInterface, I might test for the existence of a
>> default value of $level and leave it at the default if there is a
>> default (maybe I know that some implementations have a default value,
>> others don't). One could test the specific implementation with
>> instanceof checks, but the advantage of "default" could be that you do
>> not need to know the implementation and could only adapt to possibly
>> defined default values.
>
> Hi Andreas,
>
> Thanks for this question; I find this super interesting because it's
> something we haven't thought about yet. I must admit I completely
> overlooked that, whilst an interface /can/ require implementers to
> specify a default, in the case that they do not, it is still valid for
> implementations to selectively elect to provide one. Therefore I can
> append to your example the following case (I removed the `string` return
> type for now):
>
> class ZipCompression implements CompressionInterface
> {
>      public function compress(string $data, int $level)
>      {
>          var_dump($level);
>      }
> }
>
> new GzipCompression()->compress('', default ?? 6);
> new ZipCompression()->compress('', default ?? 6);
>
> In this case, we get the following output:
>
> int(4)
> Fatal error: Uncaught ValueError: Cannot pass default to required
> parameter 2 of ZipCompression::compress()
>
> I would like to fix this if possible, because I think this should be
> valid, with emphasis on /if possible/, because it may be prohibitively
> complex. Will update later.
>
> Cheers,
> Bilge
>

I think I am missing something here. From my understanding we are
*either* coding against the interface and then it should not be possible
to use `default` at all as no default is set in the interface. So the
fatal error is totally valid for me.

*Or* we are coding against the actual implementations. Then it is
totally valid IMO that providing `default` when no default is set in the
concrete implementation of the function signature raises a fatal error.

So in the example above it's either

new GzipCompression()->compress('', default ?? 6);
new ZipCompression()->compress('', default ?? 6);

and it is absolutely valid that the second one triggers a fatal error as
no default is set in the concrete implementation.

Or it's something like

/** @var CompressionInterface $compression */
foreach ([new GzipCompression, new ZipCompression] as $compression) {
$compression->compress('', default ?? 6)
}

in which case it should definitely fail as the interface doesn't provide
a default.

Cheers

Andreas

--
,,,
(o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl                                                       |
| mailto:andreas@heigl.org                  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org                                           |
+---------------------------------------------------------------------+
| https://hei.gl/appointmentwithandreas                               |
+---------------------------------------------------------------------+
| GPG-Key: https://hei.gl/keyandreasheiglorg                          |
+---------------------------------------------------------------------+

Bilge

2 years ago

On 26/08/2024 11:32, Andreas Heigl wrote:

> Hey folks.
>
> Am 26.08.24 um 11:26 schrieb Bilge:
>> On 26/08/2024 10:03, Andreas Leathley wrote:
>>> On 24.08.24 18:49, Bilge wrote:
>>>
>>> For me there is another question. When using interfaces and classes,
>>> default values can be introduced, like this:
>>>
>>> interface CompressionInterface
>>> {
>>>     public function compress(string $data, int $level): string;
>>> }
>>>
>>> class GzipCompression implements CompressionInterface
>>> {
>>>     public function compress(string $data, int $level = 4): string
>>>     {
>>>         // do something
>>>     }
>>> }
>>>
>>> When I have the GzipCompression class, I would know there is a default
>>> value for $level, but when using the interface there might or might not
>>> be a default value, depending on the implementation. As far as I read
>>> the RFC, using "default" when there is no default would lead to a
>>> runtime exception, but there is no way of finding out if there is a
>>> default if you do not already know. Being able to test that could be
>>> useful, although I am not sure about the syntax for that. In the
>>> example
>>> when getting CompressionInterface, I might test for the existence of a
>>> default value of $level and leave it at the default if there is a
>>> default (maybe I know that some implementations have a default value,
>>> others don't). One could test the specific implementation with
>>> instanceof checks, but the advantage of "default" could be that you do
>>> not need to know the implementation and could only adapt to possibly
>>> defined default values.
>>
>> Hi Andreas,
>>
>> Thanks for this question; I find this super interesting because it's
>> something we haven't thought about yet. I must admit I completely
>> overlooked that, whilst an interface /can/ require implementers to
>> specify a default, in the case that they do not, it is still valid
>> for implementations to selectively elect to provide one. Therefore I
>> can append to your example the following case (I removed the `string`
>> return type for now):
>>
>> class ZipCompression implements CompressionInterface
>> {
>>      public function compress(string $data, int $level)
>>      {
>>          var_dump($level);
>>      }
>> }
>>
>> new GzipCompression()->compress('', default ?? 6);
>> new ZipCompression()->compress('', default ?? 6);
>>
>> In this case, we get the following output:
>>
>> int(4)
>> Fatal error: Uncaught ValueError: Cannot pass default to required
>> parameter 2 of ZipCompression::compress()
>>
>> I would like to fix this if possible, because I think this should be
>> valid, with emphasis on /if possible/, because it may be
>> prohibitively complex. Will update later.
>>
>> Cheers,
>> Bilge
>>
>
> I think I am missing something here. From my understanding we are
> *either* coding against the interface and then it should not be
> possible to use `default` at all as no default is set in the
> interface. So the fatal error is totally valid for me.
>
> *Or* we are coding against the actual implementations. Then it is
> totally valid IMO that providing `default` when no default is set in
> the concrete implementation of the function signature raises a fatal
> error.

Hi Andreas,

Thanks so much for pointing this out. Your argument seems absolutely
correct to me, that this is an invalid polymorphic pattern, and as such,
I have stopped development of this feature. I still want to thank the
other Andreas (L) for raising it, and intend to add it to the possible
future scope in the RFC to provide an exception to permit `default` on
the LHS of null-coalesce. We did at least assess that such an exception
would be viable even if the development work would be disproportional to
the benefit, so it's worth noting that down.

Cheers,
Bilge

Rowan Tommins [IMSoP]

2 years ago

On Mon, 26 Aug 2024, at 10:03, Andreas Leathley wrote:

> interface CompressionInterface
> {
>      public function compress(string $data, int $level): string;
> }
>
> class GzipCompression implements CompressionInterface
> {
>      public function compress(string $data, int $level = 4): string
>      {
>          // do something
>      }
> }
>
> When I have the GzipCompression class, I would know there is a default
> value for $level, but when using the interface there might or might not
> be a default value, depending on the implementation.

This isn't unique to defaults; GzipCompression could also widen the type to int|string, for instance, and there's no syntax for detecting that either.

If you have access to change class GzipCompression, you can resolve this by creating an additional interface:

interface SimplifiedCompressionInterface extends CompressionInterface
{
    public function compress(string $data, int $level = 4): string;
}
class GzipCompression implements SimplifiedCompressionInterface ...

Then, if we can agree an implementation, you could write:

/** @var CompressionInterface $comp */
$comp->compress($myData, $comp instanceof SimplifiedCompressionInterface ? default : MY_DEFAULT_LEVEL);


If you don't have access to change the hierarchy, then what you're probably looking for is structural typing, or implicit interfaces - i.e. a way to ask "does this object meet these criteria". For instance, some imaginary pattern matching on the signature:

/** @var CompressionInterface $comp */
$comp->compress($myData, $comp is { compress(string, optional int) } ? default : MY_DEFAULT_LEVEL);


Note how, in both cases, we're not asserting anything about the default value itself, only that the signature defines the parameter as optional. It's actually a bit of a quirk that the interface has to specify a value, rather than just stating this:

interface SimplifiedCompressionInterface extends CompressionInterface
{
    public function compress(string $data, optional int $level): string;
}

Regards,

--
Rowan Tommins
[IMSoP]

Jordi Boggiano

2 years ago

Hey Bilge,

On 24.08.2024 18:49, Bilge wrote:

> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.

Great work overall, I'm all for it and even though it's not something I
saw myself using a whole lot, the json_encode example sold me on it
being more useful than I initially thought.

One question (sorry if someone already asked, I scanned the thread but
it is getting long..):

Taking this example from the RFC:

    function g($p = null) {
        f($p ?? default);
    }

Could you go one step further and use default by default but still allow
null to be passed in?

    function g($p = default) {
        f($p);
    }

I suppose this would mean $p has to hold this "default" value until a
function call is reached, at which point it would resolve to whatever
the default is. This probably complicates things for very little gain
but I had to ask.

Best,
Jordi

Bilge

2 years ago

On 26/08/2024 12:55, Jordi Boggiano wrote:

> Hey Bilge,

Hi :)

>
> On 24.08.2024 18:49, Bilge wrote:
>> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
>> think some of you might enjoy this one. Hit me with any feedback.
>
> Great work overall, I'm all for it and even though it's not something
> I saw myself using a whole lot, the json_encode example sold me on it
> being more useful than I initially thought.

Thanks! I concede, this is one of those tools for your toolbox that you
will seldom reach for, but comes in very handy whenever you do.

>
> One question (sorry if someone already asked, I scanned the thread but
> it is getting long..):

I don't blame you. I'll summarise the main takeaways in the RFC later.

>
> Taking this example from the RFC:
>
>     function g($p = null) {
>         f($p ?? default);
>     }
>
> Could you go one step further and use default by default but still
> allow null to be passed in?
>
>     function g($p = default) {
>         f($p);
>     }
>

No. The RFC has a very specific and singular focus in this regard: to
permit `default` /only/ in function call contexts. That is, although
`default` is a valid expression, it cannot be passed around or stored in
a variable. Since this is a function definition, rather than a call,
this will result in a compiler error. The specific error we get in this
case is: "Fatal error: Constant expression contains invalid operations".

Cheers,
Bilge

Bob Weinand

2 years ago

Hey Jordi,

On 26.8.2024 13:55:52, Jordi Boggiano wrote:

> One question (sorry if someone already asked, I scanned the thread but
> it is getting long..):
>
> Taking this example from the RFC:
>
>     function g($p = null) {
>         f($p ?? default);
>     }
>
> Could you go one step further and use default by default but still
> allow null to be passed in?
>
>     function g($p = default) {
>         f($p);
>     }
>
> I suppose this would mean $p has to hold this "default" value until a
> function call is reached, at which point it would resolve to whatever
> the default is. This probably complicates things for very little gain
> but I had to ask.

First, it would be some sort of spooky action at a distance, likely add
a new zval type etc.; lots of special handling for a likely minor benefit.

Second, I'd expect that bit of syntax do be useful in inheritance - like
you implement or override a parent class/interface method specifying a
default; then you can just use the default of the parent method.


Bob

Stephen Reay

2 years ago

> On 24 Aug 2024, at 23:49, Bilge <bilge@scriptfusion.com> wrote:
>
> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I think some of you might enjoy this one. Hit me with any feedback.
>
> This one already comes complete with working implementation that I've been cooking for a little while. Considering I don't know C or PHP internals, one might think implementing this feature would be prohibitively difficult, but considering the amount of help and guidance I received from Ilija, Bob and others, it would be truer to say it would have been more difficult to fail! Huge thanks to them.
>
> Cheers,
> Bilge
>

Hi,

I noticed someone talking about the various ways the `default` keyword could be used in an expression including in match() and looking in the RFC examples I see it is listed, so I think it's useful to clarify here. I haven't followed the entire thread in depth so I apologise if this was already answered, but I haven't noticed it being mentioned/clarified yet.

Can you clarify in the following, is the arm comparing against match's default or the parameter's default? Or to put it another way, in the second call, If `$arg` is 2, will the match error out due to an unmatched subject, or will it pass 1 to `F`?

function F(int $foo = 1) {}

F(match(default) { default => default });

F(match($arg) { 'a' => 0, default => default });

Cheers

Stephen

Bilge

2 years ago

On 26/08/2024 15:20, Stephen Reay wrote:

> Hi,

Hi :)

>
> I haven't followed the entire thread in depth so I apologise if this
> was already answered, but I haven't noticed it being
> mentioned/clarified yet.

Don't worry, you're right, this is an important topic that I was still
finalising in the past 48 hours and is still an omission from the RFC,
and as such we haven't discussed it on the list yet.

> Can you clarify in the following, is the arm comparing against match's
> default or the parameter's default? Or to put it another way, in the
> second call, If `$arg` is 2, will the match error out due to an
> unmatched subject, or will it pass 1 to `F`?
>
> function F(int $foo =1) {}
>
> F(match(default) {default =>default });
> F(match($arg) {'a' =>0,default =>default });

Thank you for your (excellent) question. The answer is it will pass `1`,
and the reason is as follows.

`F(match(default) { default => default });` is interpreted as `match
(default expression) { default arm => default expression }`, therefore
the first and last `default`s will be substituted with the argument's
default, but not the middle one. However, that is only the case when the
default arm is written exactly as `default`.

You can turn the condition into an expression, in which case all three
will act as expressions and be substituted accordingly, e.g.
`F(match(default) { (int) default => default });`.

Since the default condition is now an expression, you can still have a
default arm in addition to this, e.g. the following would be valid:

F(match(default) {
    (int) default => default,
    default => default,
});

Whilst this is a curiosity, consider that passing match expressions
directly to arguments is something I personally have never witnessed and
that goes doubly for combining it with `default`. So, whilst it is
interesting to know, and important for the RFC to state the specific
semantics of this scenario, the practical applications are presumed slim
to none.

Cheers,
Bilge

Derick Rethans

2 years ago

On Sat, 24 Aug 2024, Bilge wrote:

> Hi gang,
>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I think
> some of you might enjoy this one. Hit me with any feedback.

I liked this up to the point where I saw:

$f = fn ($v = 1, $default = 2) => $v + $default;
var_dump($f(default: default + 1)); // int(4)

Using 'default' as a place holder for (not) passing an argument seems
useful. I am however much uncertain about using composition with the
keyword in expressions.

I also think that implementation would probably be significantly less
complex as it was only used for placeholders. It's likely something that
can be handled in the parser.

Having an opcode for it, that does internal reflection, is what I'm
unsure about.

cheers,
Derick

Bilge

2 years ago

On 27/08/2024 12:56, Derick Rethans wrote:

> Using 'default' as a place holder for (not) passing an argument seems
> useful. I am however much uncertain about using composition with the
> keyword in expressions.

Presumably you mean ternary and null coalesce would still be acceptable?
Otherwise, it is little more than an alternative to named parameters,
which is seldom useful. Ternary and null coalesce are, themselves,
expressions so I expect that is why I was guided towards treating
`default` the same way. Whilst the applications beyond use in
conditionals still strikes me as limited, it also appears equally
harmless.  Though the number of people whom feel its use in expression
should be limited is steadily growing, I'm still waiting on someone to
publish an exclusion list with justification for each exclusion, because
again, I think a plurality of possibilities are harmless. Nevertheless,
an inclusion list was produced by IMSoP, based on what he felt would be
useful, but I think arbitrary exclusions are unintuitive for developers
and likely to limit someone from doing something justifiably useful that
we didn't conceive at the time.

> I also think that implementation would probably be significantly less
> complex as it was only used for placeholders.

Would it? I was told that the alternative was to modify all the SEND
opcodes, which is anything but less complex. The current implementation
seems necessarily simple to me since it just adds a single new opcode
that doesn't interfere with anything else. I say necessarily, because if
it was actually complex, I probably couldn't have written the solution
given my limited ability.

>   It's likely something that can be handled in the parser.

Clearly this problem cannot be solved solely in the parser. If I assume
you're proposing to just allow `default` in conditionals, what is
responsible for compiling that and how will the called function know
what to do with it?

Cheers,
Bilge

Rowan Tommins [IMSoP]

2 years ago

On 27 August 2024 14:14:03 BST, Bilge <bilge@scriptfusion.com> wrote:

> Whilst the applications beyond use in conditionals still strikes me as limited, it also appears equally harmless.  Though the number of people whom feel its use in expression should be limited is steadily growing, I'm still waiting on someone to publish an exclusion list with justification for each exclusion, because again, I think a plurality of possibilities are harmless.

I'm slightly baffled why you're still looking at it from this angle. Possibly, the volume of messages in the thread has made it easy to miss some of the points that have been raised.

There is a fundamental, unavoidable, cost to allowing any expression which relies on the type or value of the parameter's default not changing in subclasses or future versions. It makes changes that are currently guaranteed safe by the principles of the language, into compatibility breaks. As Bruce pointed out, it introduces a contravariant output, contrary to the substitution principle, unless we break a bunch of existing use cases by declaring parameter defaults invariant.

You're not going to see any kind of "exclusion list"of operators, because on closer examination of the impact, we've realised that it's not particular operators that are the problem, it's the entire principle of allowing "default" to be evaluated to a value in the middle of an expression.

The only expressions that are in some sense "safe" are those that can apply equally to any possible type that the function could in future set as the default. In theory, that includes a match statement with an arm of "default => default", e.g.

json_encode($data, default, match(gettype(default)) { 'int' => default | JSON_PRETTY_PRINT, default => default });

Apart from being incredibly hard to read, that's not even useful: the aim is to always enable pretty printing, but the result is "enable pretty print, unless the type of the default happens to change".

So that leaves us with those expressions where "default" is only a result, not an input:

expression ?: default
expression ? expression : default
expression ? default : expression
expression ?? default

Unless I've forgotten something, that's it; that's your list of allowed expressions.

Whether that's possible to enforce, in the parser, the compiler, or the executor, I don't know. But if it's not, my opinion is that the entire feature has an unanticipated problem that makes it unworkable. It would be a shame, because on the face of it I can see the value, but sometimes you just hit a dead end and have to turn back.

Regards,
Rowan Tommins
[IMSoP]

John Bafford

2 years ago

> On Aug 27, 2024, at 10:14, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:
>
> The only expressions that are in some sense "safe" are those that can apply equally to any possible type that the function could in future set as the default. In theory, that includes a match statement with an arm of "default => default", e.g.
>
> json_encode($data, default, match(gettype(default)) { 'int' => default | JSON_PRETTY_PRINT, default => default });
>
> Apart from being incredibly hard to read, that's not even useful: the aim is to always enable pretty printing, but the result is "enable pretty print, unless the type of the default happens to change".

I'm not sure this could even work at all. The "default" parameter to gettype() isn't the default value of the third parameter to json_encode(). It's the default value of the first parameter to gettype(). Which would probably fail, since gettype()'s first parameter doesn't have a default. I suppose this could be solved by specifying an offset or label (e.g. as with `continue 2` in a nested loop), but that would just make it even harder to read.

-John

Rowan Tommins [IMSoP]

2 years ago

On 27/08/2024 16:03, John Bafford wrote:

> I'm not sure this could even work at all. The "default" parameter to
> gettype() isn't the default value of the third parameter to
> json_encode(). It's the default value of the first parameter to
> gettype(). Which would probably fail, since gettype()'s first parameter
> doesn't have a default. I suppose this could be solved by specifying an
> offset or label (e.g. as with `continue 2` in a nested loop), but that
> would just make it even harder to read.

Ah, good catch. So without a pattern-matching "default is int", I'm not
sure how you'd even achieve that safety.


There are a few other examples on this thread that contain the same
mistake, such as MWOP's:

class A {
    public function __construct(private LogInterface $logger = new
DefaultLogger()) { }
}

class ProxiedLogger implements LogInterface { ... }

$a = new A(new ProxyLogger(default));

The "default" wouldn't look anything up in A::__construct, only in
ProxyLogger::__construct. To pass the default out to any kind of
function, you'd have to write some contorted expression like this:

$a = new A( $default=default && false ?: new ProxyLogger($default) );

That's even further into Obfuscated Code Contest territory than "default
=> default", and further reduces the reasonable use cases for expressions.

--
Rowan Tommins
[IMSoP]

Bilge

2 years ago

Hi gang,

On 24/08/2024 17:49, Bilge wrote:

>
> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> think some of you might enjoy this one. Hit me with any feedback.
>

Now the dust has settled, I've updated the RFC to version 1.1. The
premise of the RFC is unchanged, but the proposal has been expanded and
a discussion section added to summarise the ~100 message thread to
capture the major concerns raised in a condensed format. I hope I've
done a good job of fairly and accurately representing your concerns, but
if not please correct me.

Furthermore, a secondary vote has been added. The secondary vote will be
open to all (whether in favour or against the proposal) to capture
alternative implementations you might also be in favour of. If the
primary vote passes, the secondary vote won't matter, but otherwise it
may help guide our sails in future.

Kind regards,
Bilge

John Coggeshall

2 years ago

One thought re-reading the RFC.

abstract class Theme {
public function bar();
}

class CuteTheme extends Theme {
public function foo();
}

class Config {
public function __construct(Theme $theme = new CuteTheme()) {}
}

$a = new Config(default->foo());
In the proposed (updated) RFC would this be proposed to work? If so this should be added to the discussion section as something I think is equally as problematic as union types. I don't think saying "Union and Mixed" is broad enough. In this case it would actually have to only allow Theme (whatever that was) and prevent you from calling foo() because that isn't a member of Theme . Otherwise it's the same problem as union types in a different color.
Coogle
On Aug 29 2024, at 5:52 pm, Bilge <bilge@scriptfusion.com> wrote:

Rowan Tommins [IMSoP]

2 years ago

On Thu, 29 Aug 2024, at 22:52, Bilge wrote:

>> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
>> think some of you might enjoy this one. Hit me with any feedback.
>>
> Now the dust has settled, I've updated the RFC to version 1.1. The
> premise of the RFC is unchanged, but the proposal has been expanded and
> a discussion section added to summarise the ~100 message thread to
> capture the major concerns raised in a condensed format. I hope I've
> done a good job of fairly and accurately representing your concerns, but
> if not please correct me.

Hi,

I will try to find some time over the next few days to write something up, because there are some clear misunderstandings in that current text.

- It's not about union types. There are lots of examples using union types, because they're easier to illustrate than class/interface hierarchies, but "disallowing default to be passed to union types" wouldn't even help with the example directly below that sentence.
- The paragraph about "Default as a contract" misses the point by a mile. There's a world of difference between "the new version of this library has different behaviour" and "the new version of this library gives a TypeError in my code which used to work".
- It's not about "critics" and "preferred versions", or it doesn't need to be. We can say "here are some situations where it might be useful; and here are some situations where it would cause unexpected errors; do we think the benefits of one outweigh the cost/risk of the other?"

I already shared this with you, but for the benefit of the wider audience, here are some examples I've put together of when users might vary the types of defaults, and how that would cause errors with the proposed feature: https://gist.github.com/IMSoP/16e2422d86e3ab513d6b0658009d0c06 I stress again, I'm not trying to win points in a popularity contest here, I'm trying to make sure we properly lay out the pros and cons of the proposal.

It's worth noting that the drawback being pointed out is similar to one encountered with named arguments. Nikita dedicated a substantial part of that RFC to discussing the problem, its possible solutions, and the impact of the proposed direction: this section https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance and this one https://wiki.php.net/rfc/named_params#to_parameter_name_changes_during_inheritance and most of this one https://wiki.php.net/rfc/named_params#backwards_incompatible_changes

Regards,

--
Rowan Tommins
[IMSoP]

Rowan Tommins [IMSoP]

2 years ago

On 29/08/2024 22:52, Bilge wrote:

> On 24/08/2024 17:49, Bilge wrote:
>>
>> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
>> think some of you might enjoy this one. Hit me with any feedback.
>>
> Now the dust has settled, I've updated the RFC to version 1.1. The
> premise of the RFC is unchanged, but the proposal has been expanded
> and a discussion section added to summarise the ~100 message thread to
> capture the major concerns raised in a condensed format. I hope I've
> done a good job of fairly and accurately representing your concerns,
> but if not please correct me.

As promised, I have written up a full explanation of the type safety
issues here: https://wiki.php.net/rfc/default_expression/type_safety

I have tried to write this as a neutral description of the problem and
the possible approaches we could take, to be inserted directly into the
current RFC, rather than as a counter-opinion or a narrative of who said
what.

I have included the 4 options which I believe are the only ones we have;
it is then a matter of opinion which we think is best. For the record,
my opinion remains that option 3 (limit to conditional expressions) is
preferable, but I have assumed the RFC will continue to advocate for
option 1 (allow any expression and assume problems will be rare).

I hope I have explained it clearly enough this time to overcome the
previous misunderstandings of where the issue lies.

Regards,

--
Rowan Tommins
[IMSoP]

Rob Landers

2 years ago

On Sun, Sep 1, 2024, at 14:39, Rowan Tommins [IMSoP] wrote:

> On 29/08/2024 22:52, Bilge wrote:
> > On 24/08/2024 17:49, Bilge wrote:
> >>
> >> New RFC just dropped: https://wiki.php.net/rfc/default_expression. I
> >> think some of you might enjoy this one. Hit me with any feedback.
> >>
> > Now the dust has settled, I've updated the RFC to version 1.1. The
> > premise of the RFC is unchanged, but the proposal has been expanded
> > and a discussion section added to summarise the ~100 message thread to
> > capture the major concerns raised in a condensed format. I hope I've
> > done a good job of fairly and accurately representing your concerns,
> > but if not please correct me.
>
>
> As promised, I have written up a full explanation of the type safety
> issues here: https://wiki.php.net/rfc/default_expression/type_safety
>
> I have tried to write this as a neutral description of the problem and
> the possible approaches we could take, to be inserted directly into the
> current RFC, rather than as a counter-opinion or a narrative of who said
> what.
>
> I have included the 4 options which I believe are the only ones we have;
> it is then a matter of opinion which we think is best. For the record,
> my opinion remains that option 3 (limit to conditional expressions) is
> preferable, but I have assumed the RFC will continue to advocate for
> option 1 (allow any expression and assume problems will be rare).
>
> I hope I have explained it clearly enough this time to overcome the
> previous misunderstandings of where the issue lies.
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]
>

Thank you Rowan,

I wasn't following the discussion closely and didn't realize this was the issue. Thank you for taking the time to describe it.

For option 1:

Is manually copying the default also not type-safe? Is php a type-safe language? I think a lot of the arguments I saw suggested that people don't review libraries and their implementations when upgrading or installing them. This is just a shorthand for manually copy-pasting the default from other code, and this argument really only makes sense to me if there are no reviews before upgrading/using a library.

For option 3:

That being said, this is obviously playing with fire, and there will be people who (ab)use this and get burned; especially if they don't do due-diligence before using libraries. Thus a restriction may make a lot of sense; at least keeping it to the most obvious use cases should prevent the worst case scenarios imagined in this thread.

Realistically, I think we should only consider option (1) or (3). Option (3) -- if it can be done -- is the more conservative approach, and we can observe how it is used. We can always relax the feature in the future, based on feedback.

— Rob

Rowan Tommins [IMSoP]

2 years ago

On 1 September 2024 17:45:57 BST, Rob Landers <rob@bottled.codes> wrote:

>Is manually copying the default also not type-safe? Is php a type-safe language? I think a lot of the arguments I saw suggested that people don't review libraries and their implementations when upgrading or installing them. This is just a shorthand for manually copy-pasting the default from other code, and this argument really only makes sense to me if there are no reviews before upgrading/using a library.

Copying and pasting the default value is no different from providing any other explicit value; whether you choose 69 because you like the number, or because you saw it was the current default, you are passing an integer. And if you write 23*3, you're just writing the same integer a different way.

PHP guarantees the type safety of this under inheritance by enforcing contravariance of input: if you try to write a subclass that would not accept an integer, when a parent class would, PHP will refuse to compile your subclass.

Similarly, if the class provides a non-final method like `getDefaultFlags(): int` then it is type safe to call that and assume the value will always be an integer, because PHP enforces covariance of output: a subclass may not return a value that would not have been allowed by the parent class.

If you were designing a language where default values were explicitly available as outputs, you would need to make them covariant, which is option 2.

Obviously, the language cannot directly control the compatibility promises of third party libraries, but these principles are well enough known that I would expect popular projects to base their versioning / deprecation policies on them.

Regards,
Rowan Tommins
[IMSoP]