Skip to content

Warn if continue is used on switch #3364

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions Zend/tests/continue_targeting_switch_warning.phpt
Original file line number Diff line number Diff line change
@@ -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
37 changes: 31 additions & 6 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_compile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down