RFC: Explicit fallthrough for switch()

RFC: https://wiki.php.net/rfc/switch-case-fallthrough

Author: TimWolla

Zend/tests/switch/continue_targeting_switch_warning.phpt

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

Zend/tests/switch/fallthrough_warning.phpt

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

Zend/zend_compile.c

@@ -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;
 
@@ -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]);
@@ -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) {

Zend/zend_constants.stub.php

@@ -141,3 +141,5 @@
  * @undocumentable
  */
 const NULL = null;
+
+const fallthrough = null;

Zend/zend_constants_arginfo.h

@@ -438,7 +438,7 @@ function main(): void
                         break;
                     }
                     $i--;
-                // no break
+                    fallthrough;
                 case 'w':
                     $failed_tests_file = fopen($argv[++$i], 'w+t');
                     break;
@@ -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':
@@ -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) {
@@ -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");