Warn if continue is used on switch

Supersedes RFC https://wiki.php.net/rfc/continue_on_switch_deprecation by generating a warning instead of deprecating and removing this functionality.
php · Jun 24, 2018 · 04e3523b7d095341f65ed5e71a3cac82fca690e4 · cmb69 · Aug 17, 2018 · 04e3523
1 parent 41a6625
commit 04e3523b7d095341f65ed5e71a3cac82fca690e4
Unified Split

Showing with 94 additions and 6 deletions.

+13 −0 UPGRADING

+49 −0 Zend/tests/continue_targeting_switch_warning.phpt

+31 −6 Zend/zend_compile.c

+1 −0 Zend/zend_compile.h
diff --git a/UPGRADING b/UPGRADING
@@ -44,6 +44,19 @@ Core:
    the following "FOO;" will cause a syntax error. This issue can always be
    resolved by choosing an ending label that does not occur within the contents
    of the string.
+  . "continue" statements targeting "switch" control flow structures will now
+    generate a warning. In PHP such "continue" statements are equivalent to
+    "break", while they behave as "continue 2" in other languages.
+
+        while ($foo) {
+            switch ($bar) {
+                case "baz":
+                    continue;
+                    // Warning: "continue" targeting switch is equivalent to
+                                "break". Did you mean to use "continue 2"?
+            }
+        }
+
  . Array accesses of type $obj["123"], where $obj implements ArrayAccess and
    "123" is an integral string literal will no longer result in an implicit
    conversion to integer, i.e., $obj->offsetGet("123") will be called instead
diff --git a/Zend/tests/continue_targeting_switch_warning.phpt b/Zend/tests/continue_targeting_switch_warning.phpt
@@ -0,0 +1,49 @@
+--TEST--
+Warning on continue targeting switch
+--FILE--
+<?php
+
+function test() {
+    switch ($foo) {
+        case 0:
+            continue; // INVALID
+        case 1:
+            break;
+    }
+     
+    while ($foo) {
+        switch ($bar) {
+            case 0:
+                continue; // INVALID
+            case 1:
+                continue 2;
+            case 2:
+                break;
+        }
+    }
+     
+    while ($foo) {
+        switch ($bar) {
+            case 0:
+                while ($xyz) {
+                    continue 2; // INVALID
+                }
+            case 1:
+                while ($xyz) {
+                    continue 3;
+                }
+            case 2:
+                while ($xyz) {
+                    break 2;
+                }
+        }
+    }
+}
+
+?>
+--EXPECTF--
+Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in %s on line 6
+
+Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in %s on line 14
+
+Warning: "continue 2" targeting switch is equivalent to "break 2". Did you mean to use "continue 3"? in %s on line 26
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -649,7 +649,8 @@ static void zend_end_live_range(zend_op_array *op_array, uint32_t offset, uint32
 }
 /* }}} */

-static inline void zend_begin_loop(zend_uchar free_opcode, const znode *loop_var) /* {{{ */
+static inline void zend_begin_loop(
+		zend_uchar free_opcode, const znode *loop_var, zend_bool is_switch) /* {{{ */
 {
 	zend_brk_cont_element *brk_cont_element;
 	int parent = CG(context).current_brk_cont;
@@ -658,6 +659,7 @@ static inline void zend_begin_loop(zend_uchar free_opcode, const znode *loop_var
 	CG(context).current_brk_cont = CG(context).last_brk_cont;
 	brk_cont_element = get_next_brk_cont_element();
 	brk_cont_element->parent = parent;
+	brk_cont_element->is_switch = is_switch;

 	if (loop_var && (loop_var->op_type & (IS_VAR|IS_TMP_VAR))) {
 		uint32_t start = get_next_op_number(CG(active_op_array));
@@ -4585,6 +4587,29 @@ void zend_compile_break_continue(zend_ast *ast) /* {{{ */
 				depth, depth == 1 ? "" : "s");
 		}
 	}
+
+	if (ast->kind == ZEND_AST_CONTINUE) {
+		int d, cur = CG(context).current_brk_cont;
+		for (d = depth - 1; d > 0; d--) {
+			cur = CG(context).brk_cont_array[cur].parent;
+			ZEND_ASSERT(cur != -1);
+		}
+
+		if (CG(context).brk_cont_array[cur].is_switch) {
+			if (depth == 1) {
+				zend_error(E_WARNING,
+					"\"continue\" targeting switch is equivalent to \"break\". " \
+					"Did you mean to use \"continue %d\"?",
+					depth + 1);
+			} else {
+				zend_error(E_WARNING,
+					"\"continue %d\" targeting switch is equivalent to \"break %d\". " \
+					"Did you mean to use \"continue %d\"?",
+					depth, depth, depth + 1);
+			}
+		}
+	}
+
 	opline = zend_emit_op(NULL, ast->kind == ZEND_AST_BREAK ? ZEND_BRK : ZEND_CONT, NULL, NULL);
 	opline->op1.num = CG(context).current_brk_cont;
 	opline->op2.num = depth;
@@ -4697,7 +4722,7 @@ void zend_compile_while(zend_ast *ast) /* {{{ */

 	opnum_jmp = zend_emit_jump(0);

-	zend_begin_loop(ZEND_NOP, NULL);
+	zend_begin_loop(ZEND_NOP, NULL, 0);

 	opnum_start = get_next_op_number(CG(active_op_array));
 	zend_compile_stmt(stmt_ast);
@@ -4720,7 +4745,7 @@ void zend_compile_do_while(zend_ast *ast) /* {{{ */
 	znode cond_node;
 	uint32_t opnum_start, opnum_cond;

-	zend_begin_loop(ZEND_NOP, NULL);
+	zend_begin_loop(ZEND_NOP, NULL, 0);

 	opnum_start = get_next_op_number(CG(active_op_array));
 	zend_compile_stmt(stmt_ast);
@@ -4771,7 +4796,7 @@ void zend_compile_for(zend_ast *ast) /* {{{ */

 	opnum_jmp = zend_emit_jump(0);

-	zend_begin_loop(ZEND_NOP, NULL);
+	zend_begin_loop(ZEND_NOP, NULL, 0);

 	opnum_start = get_next_op_number(CG(active_op_array));
 	zend_compile_stmt(stmt_ast);
@@ -4834,7 +4859,7 @@ void zend_compile_foreach(zend_ast *ast) /* {{{ */
 	opnum_reset = get_next_op_number(CG(active_op_array));
 	opline = zend_emit_op(&reset_node, by_ref ? ZEND_FE_RESET_RW : ZEND_FE_RESET_R, &expr_node, NULL);

-	zend_begin_loop(ZEND_FE_FREE, &reset_node);
+	zend_begin_loop(ZEND_FE_FREE, &reset_node, 0);

 	opnum_fetch = get_next_op_number(CG(active_op_array));
 	opline = zend_emit_op(NULL, by_ref ? ZEND_FE_FETCH_RW : ZEND_FE_FETCH_R, &reset_node, NULL);
@@ -4989,7 +5014,7 @@ void zend_compile_switch(zend_ast *ast) /* {{{ */

 	zend_compile_expr(&expr_node, expr_ast);

-	zend_begin_loop(ZEND_FREE, &expr_node);
+	zend_begin_loop(ZEND_FREE, &expr_node, 1);

 	case_node.op_type = IS_TMP_VAR;
 	case_node.u.op.var = get_temporary_variable(CG(active_op_array));
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -158,6 +158,7 @@ typedef struct _zend_brk_cont_element {
 	int cont;
 	int brk;
 	int parent;
+	zend_bool is_switch;
 } zend_brk_cont_element;

 typedef struct _zend_label {