[RFC][DISCUSSION] switch expression

php.internals

Ilija Tovilo

6 years ago
Hi internals Apparently my previous emails were classified as junk (very flattering). Hopefully that issue is fixed now. I'd like to move the RFC switch expression to "under discussion". It tries to address some of the common issues with the switch statement by introducing a new switch expression. https://wiki.php.net/rfc/switch_expression I'm looking forward to your constructive feedback! Ilija

Dan Ackroyd

6 years ago
On Sat, 28 Mar 2020 at 12:40, Ilija Tovilo <tovilo.ilija@gmail.com> wrote:
> > Hi internals > > I'd like to move the RFC switch expression to "under discussion". It > tries to address some of the common issues with the switch statement > by introducing a new switch expression. > > https://wiki.php.net/rfc/switch_expression
Hi Ilija, The switch statement is definitely something that has issues. However I think the RFC as written is not a good starting point. 1) The behaviour of switch with regards to type conversion is the biggest thing I care about. If the RFC isn't going to touch that, then it's way less interesting to me. 2) In general, I think that using the switch keyword with slightly different behaviour is not a good idea. Even if we had 'editions' in PHP, having subtly different behaviour is very confusing. A better approach would be to use a new keyword (maybe 'select') that has the correct behaviour from the start. 3) Fallthrough: "The fallthrough behavior can't reasonably be changed in the switch statement because it would break a lot of code. However this RFC porposes allowing multiple conditions per case so that the intention of running the same code can be expressed more clearly. " I don't think this is the right approach. Most new languages have the rules of: * each 'case' statement has an implicit break statement at the end of that case. * they provide an explicit 'fallthrough' keyword for where fall through is desired. In general I think there are lots of details that need to be thought through. I don't think this email list is a great place to work through all of those details. Email is not a great medium for this type of discussion in general, and causes a lot of noise for people who are not taking part in that work. Some of the people who are interested in working on how to implement generics in PHP have been doing that in a github repo https://github.com/PHPGenerics/php-generics-rfc/issues If you have the energy to work on this, please consider opening a similar repo as a place for people to discuss ideas and work through all the details, as that would be far more likely to have a productive discussion and produce the best possible RFC*. oh, and obviously link to that repo here so people can join that conversation. cheers Dan Ack * btw it might be worth thinking about pattern matching as well. Currently the below is supported, but is also horrific coding (imo) standard. function checkValue(int $x, bool $errorIfOverTen) { switch (true) { case $x < 5: return "too small\n"; case $x > 10 && $errorIfOverTen: return "value is over ten, which is not allowed.\n"; default: return "Ok\n"; } } echo checkValue(3, false); echo checkValue(11, false); echo checkValue(11, true); output is // too small // Ok // value is over ten, which is not allowed.

Ilija Tovilo

6 years ago
Hi Dan First of all, thank you for your feedback. I appreciate your time. I think it's worth noting that the switch expression solves the problems it does implicitly, not directly. 1. Returning values: Of course, this is the main argument for the switch expression (regardless of the other issues present in the statement version). 2. Fallthrough: A fallthrough for the expression version doesn't make sense because the point of every case is to return some value. A fallthrough would cause the value to be discarded, returning the value from the next case that breaks. Furthermore since every case only accepts a single value there's not even a way to express a break right now (as it is a statement). 3. Inexhaustiveness: The switch expression has to evaluate to something. When no case is executed we can either make it return null or throw an exception. Since it is better to be explicit I chose the latter. The only issue that doesn't have any direct justification for being different from the statement is the type coercion which is why I decided against changing it. It's a one line fix so believe me that I was tempted. But I think it's important to keep them consistent.
> 1) The behaviour of switch with regards to type conversion is the > biggest thing I care about. If the RFC isn't going to touch that, then > it's way less interesting to me.
I understand. As you mentioned I'm trying not to make the distinction between the statement and expression arbitrary.
> 2) In general, I think that using the switch keyword with slightly > different behaviour is not a good idea. Even if we had 'editions' in > PHP, having subtly different behaviour is very confusing. A better > approach would be to use a new keyword (maybe 'select') that has the > correct behaviour from the start.
I used the switch keyword for the simple reason that it very much still is a switch. It uses the same AST, same code generation, same opcodes.
> 3) Fallthrough: > > "The fallthrough behavior can't reasonably be changed in the switch > statement because it would break a lot of code. However this RFC > porposes allowing multiple conditions per case so that the intention > of running the same code can be expressed more clearly. " > > I don't think this is the right approach. Most new languages have the rules of: > > * each 'case' statement has an implicit break statement at the end of that case. > * they provide an explicit 'fallthrough' keyword for where fall > through is desired.
This sound like an extension of the approach described in the RFC. There's nothing stopping us from allowing a fallthrough keyword, although I personally don't see a huge need for it. Either way, the fallthrough keyword still wouldn't make sense in the switch expression.
> * btw it might be worth thinking about pattern matching as well.
I've mentioned pattern matching briefly in my original email. There are a few use cases in PHP right now, namely array destructuring, class destructuring (public properties) and ranges. Most PHP libraries make extensive use of accessors so at the moment I don't think the complexity of the feature justifies the limited use cases. This may very well change in the future (for example if PHP ever gets C# style get/set accessors).
> Currently the below is supported, but is also horrific coding (imo)
After this RFC your function could be rewritten like this: ``` function checkValue(int $x, bool $errorIfOverTen) { return true switch { $x < 5 => "too small\n", $x > 10 && $errorIfOverTen => "value is over ten, which is not allowed.\n", default => "Ok\n", }; } ``` which doesn't look too bad IMO. It has been suggested to make the true optional in this case. I still haven't fully formed an opinion on that. It would make the syntax ambiguous again for sure. I hope that clears up some of the points you've mentioned. Ilija

Arnold Daniels

6 years ago
Op za 28 mrt. 2020 13:40 schreef Ilija Tovilo <tovilo.ilija@gmail.com>:
> Hi internals > > Apparently my previous emails were classified as junk (very > flattering). Hopefully that issue is fixed now. > > I'd like to move the RFC switch expression to "under discussion". It > tries to address some of the common issues with the switch statement > by introducing a new switch expression. > > https://wiki.php.net/rfc/switch_expression > > I'm looking forward to your constructive feedback! > > Ilija > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
A minor remark; `InvalidArgumentException` really isn't applicable. No function is called, so there are no arguments. You'd throw an `UnexpectedValueException`. However, operations don't ever throw exceptions, but throw an error instead.

Ilija Tovilo

6 years ago
Hi Arnold
> However, operations don't ever throw exceptions, but throw an error instead.
I replaced it with a new, more fitting UnhandledSwitchCaseError. Thanks! Ilija