Everything

php

Adding explicit intent for SWITCH/CASE fall through?

php.internals

Mike Schinkel

6 years ago

Hi All,

Before creating an RFC I wanted to get reactions to the idea of adding FALLTHROUGH option to SWITCH/CASE for when the developer explicitly want logic to fall through to the next case and does not want to use a BREAK.

My simples motivation for this feature is that my IDE (PhpStorm) always flags CASE statements w/o BREAKs because it cannot know when I intend for the logic to fallthrough and not break.  If there was an optional FALLTHROUGH then my IDE could flag any CASE statements that do not have a FALLTHROUGH or BREAK — which would indicate an error of my intent — and avoid flagging all the situations where I intentionally want it to fall through.

Beyond that, this might also be useful for other static analysis tools such as PhpStan.
Ust
Additionally it could be added in 8.0 and then in 8.1 flagged with a warning when a CASE does not have one for the two, but only if people don't object to this. While I know some on this list feel strongly that warnings must come with to future plans to generate errors I would personally be 100% okay if it indefinitely generated only a warning.

In summary I want a mechanism to be allow me to explicitly indicate my intent with respect to SELECT/CASE statements.  What do you think? Is this worthy of an RFC, or is there some reason a majority would object to such an addition?

Thanks in advance.

-Mike

Mark Randall

6 years ago

On 17/10/2019 21:54, Mike Schinkel wrote:

> Additionally it could be added in 8.0 and then in 8.1 flagged with a warning when a CASE does not have one for the two, but only if people don't object to this. While I know some on this list feel strongly that warnings must come with to future plans to generate errors I would personally be 100% okay if it indefinitely generated only a warning.

Would you put this at switch level, or case level?

Unless your plan is to redefine how switch works by default, which would
be quite ambitious, I'm not sure it makes a great deal of sense to add a
token that would effectively no-op in all circumstances.

It sounds like a job for a static analysis comment:

/** @DisableSwitchStatementFallthroughWarning */
switch ($x) {
case 1:
$x = doSomething();

case 2:
doLaLaLaLa();
break;

default:
doTralala();
}

--
Mark Randall

Bishop Bettini

6 years ago

On Thu, Oct 17, 2019 at 4:54 PM Mike Schinkel <mike@newclarity.net> wrote:

> Before creating an RFC I wanted to get reactions to the idea of adding
> FALLTHROUGH option to SWITCH/CASE for when the developer explicitly want
> logic to fall through to the next case and does not want to use a BREAK.
>
> My simples motivation for this feature is that my IDE (PhpStorm) always
> flags CASE statements w/o BREAKs because it cannot know when I intend for
> the logic to fallthrough and not break.  If there was an optional
> FALLTHROUGH then my IDE could flag any CASE statements that do not have a
> FALLTHROUGH or BREAK — which would indicate an error of my intent — and
> avoid flagging all the situations where I intentionally want it to fall
> through.
>
> Beyond that, this might also be useful for other static analysis tools
> such as PhpStan.
> Ust
> Additionally it could be added in 8.0 and then in 8.1 flagged with a
> warning when a CASE does not have one for the two, but only if people don't
> object to this. While I know some on this list feel strongly that warnings
> must come with to future plans to generate errors I would personally be
> 100% okay if it indefinitely generated only a warning.
>
> In summary I want a mechanism to be allow me to explicitly indicate my
> intent with respect to SELECT/CASE statements.  What do you think? Is this
> worthy of an RFC, or is there some reason a majority would object to such
> an addition?
>

+1. It's 100% opt-in. People can keep using implicit fall-through, their
own /* fallthrough */ comments, or the new token. There's virtually no
overhead in the compile phase and it doesn't mess with the jump table
optimizations. There's precedent for this desire in PHP static analyzers
[1] as well as other major projects: eg, the Linux Foundation just
completed a 2 year project to remove implicit fall-through in the Linux
kernel [2] (*).

That said, some questions:

Do you envision this token accepting an optional argument, as break does,
to fall-through multiple nesting levels? Eg:

<?php
$x = $y = $z = 1;
switch ($x) {
case 1:
switch ($y) {
case 1:
switch ($z) {
case 1: fallthrough 3;
case 2: echo 'z2';
}
}
break;
case 2: echo 'x2'; break;
}
?>

What do you envision the result of indicating fall-through to no subsequent
case? Eg:

<?php
switch ($x) {
case 1: fallthrough;
}
?>

Would a "pragma" (of sorts) be a part of the concept?

<?php declare(strict_break=1);
switch ($x) {
case 1: echo 'x1'; // ImplicitBreakError thrown here
case 2: echo 'x2';
}
?>

[1]:https://github.com/phan/phan/issues/2550
[2]:https://lwn.net/Articles/794944/

(*) Fun trivia: PHP is mentioned in that LWN article about naming
conventions.

Rowan Collins

6 years ago

Hi Bishop,

I'm not the proposer, but would like to answer your questions anyway.

On Fri, 18 Oct 2019 at 04:14, Bishop Bettini <bishop@php.net> wrote:

> Do you envision this token accepting an optional argument, as break does,
> to fall-through multiple nesting levels? Eg:
>
> <?php
> $x = $y = $z = 1;
> switch ($x) {
> case 1:
>     switch ($y) {
>     case 1:
>         switch ($z) {
>         case 1: fallthrough 3;
>         case 2: echo 'z2';
>         }
>     }
>     break;
> case 2: echo 'x2'; break;
> }
> ?>
>

I think we should keep it simple - we can always add more features later if
a real need reveals itself.

I note that the Go version linked to explicitly forbids the use of
"fallthrough" in a conditional, and a multi-level fallthrough is
effectively the same thing: the control flow in your example is something
like if ( $y==1 && $z == 1 ) { fallthrough; } else { break; } I would have
thought that by the time you've got nested switch statements with
conditional fallthrough on multiple levels, the code is going to be very
hard to reason about, so unless there's a key use case, I'd leave the
complexity out of the language.

There might be use cases for single-level conditional fallthrough, but I'd
lean towards defining it as Go has: as a marker at the end of the case
block only. By far the most common use of it I've seen is like Mike's:

case x:
additional pre-processing
fallthrough
case y:
shared processing
break

> What do you envision the result of indicating fall-through to no subsequent
> case? Eg:
>
> <?php
> switch ($x) {
> case 1: fallthrough;
> }
>

I think it would be perfectly fine for this to do nothing. Currently you
can write both

switch ($x) {
case 1:
}

and

switch ($x) {
case 1: break;
}



In general, I really like this idea; I've always wished switch worked this
way.

Regards,

--
Rowan Tommins
[IMSoP]

Mike Schinkel

6 years ago

> On Oct 17, 2019, at 11:14 PM, Bishop Bettini <bishop@php.net> wrote:
>
> +1. It's 100% opt-in. People can keep using implicit fall-through, their own /* fallthrough */ comments, or the new token. There's virtually no overhead in the compile phase and it doesn't mess with the jump table optimizations. There's precedent for this desire in PHP static analyzers [1] as well as other major projects: eg, the Linux Foundation just completed a 2 year project to remove implicit fall-through in the Linux kernel [2] (*).
>
> That said, some questions:

Hi Bishop,

Thank you for the questions.

> Do you envision this token accepting an optional argument, as break does, to fall-through multiple nesting levels? Eg:

No.  I had not envisioned anything more than a single statement that would indicate to PHP and other tools that a developer intends to _not_ use a break.

What you suggest had not occurred to me, and is an interesting idea. But I think I would prefer — like Rowan Tommins argued — that we should keep it simple.

Being able to fallthrough many levels would be an extra feature we can already accomplish in other, more explicit ways. I would rather focus on targeting a use-case we cannot currently address, i.e. for PHP itself and other tools to be able to flag lack of an explicit case exiting action.

> What do you envision the result of indicating fall-through to no subsequent case? Eg:
>
> <?php
> switch ($x) {
> case 1: fallthrough;
> }
> ?>

Since it would have no effect that could be flagged or not flagged as a warning.

It might be nice to allow it so that refactoring is less likely to result in a warning.

> Would a "pragma" (of sorts) be a part of the concept?
>
> <?php declare(strict_break=1);
> switch ($x) {
> case 1: echo 'x1'; // ImplicitBreakError thrown here
> case 2: echo 'x2';
> }
> ?>

Yes, but recent discussions on this list lamented the concern that we could end up with tens if not hundreds of pragmas, and I concur that would be no fun.

So how we would indicate is an open question PHP should answer before adding a pragma for fallthrough. As an aside, I would be excited to see that discussion start in earnest.

Also, I am sure you remember that Nikita proposed an idea loosely described as "Editions" and that might be the way to go. Imagine the following:

<?php declare(edition=2020);
switch ($x) {
case 1: echo 'x1'; // ImplicitBreakError thrown here
case 2: echo 'x2';
}

Or another approach could just be version specific:

<?php declare(strict[version]=php8);
switch ($x) {
case 1: echo 'x1'; // ImplicitBreakError thrown here
case 2: echo 'x2';
}

For these though, the devil would be in the details.

-Mike