Skip to content

RFC: Explicit fallthrough for switch() #21821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
TimWolla wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

RFC: Explicit fallthrough for switch() #21821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Zend/tests/switch/continue_targeting_switch_warning.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ function test() {
case 0:
while ($xyz) {
continue 2; // INVALID
}
} fallthrough;
case 1:
while ($xyz) {
continue;
}
} fallthrough;
case 2:
while ($xyz) {
break 2;
Expand All @@ -42,11 +42,11 @@ function test() {
case 0:
while ($xyz) {
continue 2; // INVALID
}
} fallthrough;
case 1:
while ($xyz) {
continue 3;
}
} fallthrough;
case 2:
while ($xyz) {
break 2;
Expand Down
64 changes: 64 additions & 0 deletions Zend/tests/switch/fallthrough_warning.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
--TEST--
Warning on switch case falling through
--FILE--
<?php

function test($foo) {
switch ($foo) {
case 0:
case 1:
echo "a", PHP_EOL;
break;
case 2:
echo "b", PHP_EOL;
case 3:
echo "c", PHP_EOL;
break;
case 4:
echo "d", PHP_EOL;
fallthrough;
case 5:
echo "e", PHP_EOL;
break;
case 6:
echo "f", PHP_EOL;
return;
case 7:
echo "g", PHP_EOL;
continue;
case 8:
echo "h", PHP_EOL;
goto end;
case 9:
echo "i", PHP_EOL;
throw new \Exception("exception");
}
end:
}

for ($i = 0; $i <= 9; $i++) {
try {
test($i);
} catch (\Throwable $e) {
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
}
}

?>
--EXPECTF--
Warning: Non-empty case falls through to the next case, terminate the case with "fallthrough;" if this is intentional in %s on line 9

Warning: "continue" targeting switch is equivalent to "break" in %s on line 25
a
a
b
c
c
d
e
e
f
g
h
i
Exception: exception
26 changes: 24 additions & 2 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6616,6 +6616,7 @@ static void zend_compile_switch(zend_ast *ast) /* {{{ */
jmpnz_opnums = safe_emalloc(sizeof(uint32_t), cases->children, 0);
for (i = 0; i < cases->children; ++i) {
zend_ast *case_ast = cases->child[i];
ZEND_ASSERT(case_ast->kind == ZEND_AST_SWITCH_CASE);
zend_ast *cond_ast = case_ast->child[0];
znode cond_node;

Expand Down Expand Up @@ -6659,8 +6660,9 @@ static void zend_compile_switch(zend_ast *ast) /* {{{ */

for (i = 0; i < cases->children; ++i) {
zend_ast *case_ast = cases->child[i];
ZEND_ASSERT(case_ast->kind == ZEND_AST_SWITCH_CASE);
zend_ast *cond_ast = case_ast->child[0];
zend_ast *stmt_ast = case_ast->child[1];
zend_ast_list *stmt_ast = zend_ast_get_list(case_ast->child[1]);

if (cond_ast) {
zend_update_jump_target_to_next(jmpnz_opnums[i]);
Expand Down Expand Up @@ -6688,7 +6690,27 @@ static void zend_compile_switch(zend_ast *ast) /* {{{ */
}
}

zend_compile_stmt(stmt_ast);
if (stmt_ast->children > 0 && i < (cases->children - 1)) {
zend_ast *last_stmt = stmt_ast->child[stmt_ast->children - 1];
while (last_stmt && last_stmt->kind == ZEND_AST_STMT_LIST) {
zend_ast_list *list = zend_ast_get_list(last_stmt);
last_stmt = list->child[list->children - 1];
}
if (last_stmt == NULL
|| (last_stmt->kind != ZEND_AST_BREAK
&& last_stmt->kind != ZEND_AST_CONTINUE
&& last_stmt->kind != ZEND_AST_RETURN
&& last_stmt->kind != ZEND_AST_THROW
&& last_stmt->kind != ZEND_AST_GOTO)) {
if (!(last_stmt->kind == ZEND_AST_CONST && zend_string_equals_literal(zend_ast_get_str(last_stmt->child[0]), "fallthrough"))) {
CG(zend_lineno) = case_ast->lineno;
zend_error(E_WARNING,
"Non-empty case falls through to the next case, terminate the case with \"fallthrough;\" if this is intentional");
}
}
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@TimWolla TimWolla Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@bwoebi bwoebi Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Copy Markdown
Member Author

@TimWolla TimWolla Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@bwoebi bwoebi Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

zend_compile_stmt((zend_ast*)stmt_ast);
}

if (!has_default_case) {
Expand Down
2 changes: 2 additions & 0 deletions Zend/zend_constants.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,5 @@
* @undocumentable
*/
const NULL = null;

const fallthrough = null;
3 changes: 2 additions & 1 deletion Zend/zend_constants_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 1 addition & 21 deletions ext/mysqli/tests/mysqli_fetch_array_large.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,26 +86,6 @@ memory_limit=-1
return true;
}

function parse_memory_limit($limit) {

$val = trim($limit);
$last = strtolower($val[strlen($val)-1]);

switch($last) {
// The 'G' modifier is available since PHP 5.1.0
case 'g':
$val *= 1024;
case 'm':
$val *= 1024;
case 'k':
$val *= 1024;
default:
break;
}
return $val;
}


function test_fetch($host, $user, $passwd, $db, $port, $socket, $engine, $flags = null) {

$link = mysqli_init();
Expand All @@ -123,7 +103,7 @@ memory_limit=-1

$package_size = 524288;
$offset = 3;
$limit = (ini_get('memory_limit') > 0) ? parse_memory_limit(ini_get('memory_limit')) : pow(2, 32);
$limit = (ini_get('memory_limit') > 0) ? ini_parse_quantity(ini_get('memory_limit')) : pow(2, 32);

/* try to respect php.ini but make run time a soft limit */
$max_runtime = (ini_get('max_execution_time') > 0) ? ini_get('max_execution_time') : 30;
Expand Down
3 changes: 0 additions & 3 deletions ext/mysqli/tests/mysqli_fetch_field_flags.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,6 @@ mysqli_close($link);
// TODO - check exact version!
$expected_flags = trim(str_replace('UNSIGNED', '', $expected_flags));
}

default:
break;
}

list($missing_flags, $unexpected_flags, $flags_found) = checkFlags($field->flags, $expected_flags, $flags);
Expand Down
2 changes: 2 additions & 0 deletions ext/mysqli/tests/mysqli_fork.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ if (!have_innodb($link))
case 0:
/* child */
exit(0);
break;

default:
/* parent */
Expand Down Expand Up @@ -124,6 +125,7 @@ if (!have_innodb($link))
exit(mysqli_errno($plink));

exit(0);
break;

default:
/* parent */
Expand Down
3 changes: 2 additions & 1 deletion ext/opcache/tests/ssa_bug_007.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ function render($properties) {
}
?>
OK
--EXPECT--
--EXPECTF--
Warning: Non-empty case falls through to the next case, terminate the case with "fallthrough;" if this is intentional in %s on line %d
OK
3 changes: 2 additions & 1 deletion ext/opcache/tests/switch_string_free_opt.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ function test($a) {
}
?>
===DONE===
--EXPECT--
--EXPECTF--
Warning: Non-empty case falls through to the next case, terminate the case with "fallthrough;" if this is intentional in %s on line %d
===DONE===
9 changes: 5 additions & 4 deletions run-tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ function main(): void
break;
}
$i--;
// no break
fallthrough;
case 'w':
$failed_tests_file = fopen($argv[++$i], 'w+t');
break;
Expand Down Expand Up @@ -602,10 +602,11 @@ function main(): void
case '--version':
echo '$Id$' . "\n";
exit(1);
break;

default:
echo "Illegal switch '$switch' specified!\n";
// no break
fallthrough;
case 'h':
case '-help':
case '--help':
Expand Down Expand Up @@ -1520,7 +1521,7 @@ function run_all_tests_parallel(array $test_files, array $env, ?string $redir_te
}
}
$junit->mergeResults($message["junit"]);
// no break
fallthrough;
case "ready":
// Schedule sequential tests only once we are down to one worker.
if (count($workerProcs) === 1 && $sequentialTests) {
Expand Down Expand Up @@ -1616,7 +1617,7 @@ function run_all_tests_parallel(array $test_files, array $env, ?string $redir_te
];
$error_consts = array_combine(array_map('constant', $error_consts), $error_consts);
error("Worker $i reported unexpected {$error_consts[$message['errno']]}: $message[errstr] in $message[errfile] on line $message[errline]");
// no break
fallthrough;
default:
kill_children($workerProcs);
error("Unrecognised message type '$message[type]' from worker $i");
Expand Down
Loading