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
+108 −37
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");
}
}
}

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