RFC: Explicit fallthrough for switch()#21821
Conversation
7175190 to
85e9dc5
Compare
85e9dc5 to
69b289f
Compare
69b289f to
e537db1
Compare
| zend_error(E_WARNING, | ||
| "Non-empty case falls through to the next case, terminate the case with \"fallthrough;\" if this is intentional"); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's a bit unfortunate this won't work for if (foo) { return $x; } else { return $y; } (that's solvable), or functions with the never return type that is officially supported to indicate exactly this (that's not solvable). A runtime check would solve that. Static analysis could bridge the gap to make this more ergonomic.
There was a problem hiding this comment.
It's a bit unfortunate this won't work for
if (foo) { return $x; } else { return $y; }(that's solvable),
We considered that, but felt that going down that route would require a full control-flow analysis here, since folks would rightfully expect nested if() to work correctly as well. And perhaps also switch() with a default:. Or an exhaustive switch (which isn't solvable).
In the end just adding a break; below such an if() would also make it easier to scan the code because the case is clearly terminated on the expected nesting level.
functions with the never return type that is officially supported to indicate exactly this (that's not solvable)
It would be solvable for internal functions (since they are guaranteed to be available at compile time), but not for userland functions.
I also was a little annoyed needing to put a break; after exit();, but overall functions returning : never are few and far-between. It's really only exit() and perhaps pcntl_exec(), if the latter would use exceptions.
I could add support for internal functions being : never, but I'm not sure if this would add or remove to the consistency 🤔
There was a problem hiding this comment.
Unreachable breaks can also be misleading though. throw new UnreachableError(); would be better, but this error is not built-in. assert(0, "Unreachable"); also won't work.
There was a problem hiding this comment.
@TimWolla I think I agree that emitting a warning at runtime / throwing an exception when fallthrough is missing is more reasonable for PHP.
It also prevents you from putting break there to make the runtime happy, and it actually going down the wrong path accidentally, because it did not actually throw/return/whatever due to a bug.
There was a problem hiding this comment.
It also prevents you from putting break there to make the runtime happy, and it actually going down the wrong path accidentally, because it did not actually throw/return/whatever due to a bug.
Sorry, I don't understand what you're trying to say there. That's worded too abstract.
There was a problem hiding this comment.
Basically "insert a (runtime) warning/throw exception before any case statement". Have fallthrough; jump over this.
So, if normal control flow reaches that code path, due to a missing fallthrough or missing break, or return or whatever, there's a warning/exception emitted.
RFC: https://wiki.php.net/rfc/switch-case-fallthrough