Re: break/continue $var

Hi Dmitry, From a BC point of view, the impact will be minimal. The only project I found relying on this syntax is XPath class [1]. Please add a note the upgrade guide as well in trunk, maybe in a BC break sections. [1] http://google.com/codesearch?hl=en&sa=N&q=lang:php+break\s{1,}%28\%24\w%29 Cheers, On Wed, Nov 24, 2010 at 1:11 PM, Dmitry Stogov <dmitry@zend.com> wrote: > the proposed patch (backport from php6) is attached. > I think this feature is completely useless and it was agreed to remove it > about 5 years ago. > > Derick, can I commit the patch today? > > Thanks. Dmitry. > > Dmitry Stogov wrote: >> >> Hi, >> >> Previously we decided to remove "break/continue $var" syntax. >> I even implemented it in PHP6 brunch, however it wasn't backported into >>  trunk. Could I do it? >> >> Thanks. Dmitry. >> > > Index: Zend/zend_execute.c > =================================================================== > --- Zend/zend_execute.c (revision 305710) > +++ Zend/zend_execute.c (working copy) > @@ -1362,21 +1362,12 @@ >        } >  } > > -static inline zend_brk_cont_element* zend_brk_cont(zval *nest_levels_zval, > int array_offset, const zend_op_array *op_array, const temp_variable *Ts > TSRMLS_DC) > +static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int > array_offset, const zend_op_array *op_array, const temp_variable *Ts > TSRMLS_DC) >  { >        zval tmp; > -       int nest_levels, original_nest_levels; > +       int original_nest_levels = nest_levels; >        zend_brk_cont_element *jmp_to; > > -       if (nest_levels_zval->type != IS_LONG) { > -               tmp = *nest_levels_zval; > -               zval_copy_ctor(&tmp); > -               convert_to_long(&tmp); > -               nest_levels = tmp.value.lval; > -       } else { > -               nest_levels = nest_levels_zval->value.lval; > -       } > -       original_nest_levels = nest_levels; >        do { >                if (array_offset==-1) { >                        zend_error_noreturn(E_ERROR, "Cannot break/continue > %d level%s", original_nest_levels, (original_nest_levels == 1) ? "" : > "s"); > Index: Zend/zend_vm_execute.h > =================================================================== > --- Zend/zend_vm_execute.h      (revision 305710) > +++ Zend/zend_vm_execute.h      (working copy) > @@ -1304,11 +1304,10 @@ >  static int ZEND_FASTCALL >  ZEND_BRK_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS) >  { >        USE_OPLINE > - >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(opline->op2.zv, opline->op1.opline_num, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->op1.opline_num, >                           EX(op_array), EX_Ts() TSRMLS_CC); > >        ZEND_VM_JMP(EX(op_array)->opcodes + el->brk); > @@ -1317,11 +1316,10 @@ >  static int ZEND_FASTCALL >  ZEND_CONT_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS) >  { >        USE_OPLINE > - >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(opline->op2.zv, opline->op1.opline_num, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->op1.opline_num, >                           EX(op_array), EX_Ts() TSRMLS_CC); > >        ZEND_VM_JMP(EX(op_array)->opcodes + el->cont); > @@ -1334,7 +1332,7 @@ >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(opline->op2.zv, opline->extended_value, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->extended_value, >                           EX(op_array), EX_Ts() TSRMLS_CC); > >        brk_opline = EX(op_array)->opcodes + el->brk; > @@ -1477,32 +1475,6 @@ >  } > > > -static int ZEND_FASTCALL >  ZEND_BRK_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > -       zend_free_op free_op2; > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_tmp(opline->op2.var, EX_Ts(), > &free_op2 TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > -       zval_dtor(free_op2.var); > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->brk); > -} > - > -static int ZEND_FASTCALL >  ZEND_CONT_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > -       zend_free_op free_op2; > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_tmp(opline->op2.var, EX_Ts(), > &free_op2 TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > -       zval_dtor(free_op2.var); > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->cont); > -} > - >  static int ZEND_FASTCALL >  ZEND_FETCH_CLASS_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS) >  { >        USE_OPLINE > @@ -1599,32 +1571,6 @@ >  } > > > -static int ZEND_FASTCALL >  ZEND_BRK_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > -       zend_free_op free_op2; > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_var(opline->op2.var, EX_Ts(), > &free_op2 TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > -       if (free_op2.var) {zval_ptr_dtor(&free_op2.var);}; > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->brk); > -} > - > -static int ZEND_FASTCALL >  ZEND_CONT_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > -       zend_free_op free_op2; > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_var(opline->op2.var, EX_Ts(), > &free_op2 TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > -       if (free_op2.var) {zval_ptr_dtor(&free_op2.var);}; > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->cont); > -} > - >  static int ZEND_FASTCALL >  ZEND_FETCH_CLASS_SPEC_UNUSED_HANDLER(ZEND_OPCODE_HANDLER_ARGS) >  { >        USE_OPLINE > @@ -1754,32 +1700,6 @@ >  } > > > -static int ZEND_FASTCALL >  ZEND_BRK_SPEC_CV_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > - > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_cv_BP_VAR_R(EX_CVs(), > opline->op2.var TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > - > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->brk); > -} > - > -static int ZEND_FASTCALL >  ZEND_CONT_SPEC_CV_HANDLER(ZEND_OPCODE_HANDLER_ARGS) > -{ > -       USE_OPLINE > - > -       zend_brk_cont_element *el; > - > -       SAVE_OPLINE(); > -       el = zend_brk_cont(_get_zval_ptr_cv_BP_VAR_R(EX_CVs(), > opline->op2.var TSRMLS_CC), opline->op1.opline_num, > -                          EX(op_array), EX_Ts() TSRMLS_CC); > - > -       ZEND_VM_JMP(EX(op_array)->opcodes + el->cont); > -} > - >  static int ZEND_FASTCALL >  ZEND_BW_NOT_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS) >  { >        USE_OPLINE > @@ -36791,55 +36711,55 @@ >        ZEND_NULL_HANDLER, >        ZEND_NULL_HANDLER, >        ZEND_BRK_SPEC_CONST_HANDLER, > -       ZEND_BRK_SPEC_TMP_HANDLER, > -       ZEND_BRK_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_BRK_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_BRK_SPEC_CONST_HANDLER, > -       ZEND_BRK_SPEC_TMP_HANDLER, > -       ZEND_BRK_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_BRK_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_BRK_SPEC_CONST_HANDLER, > -       ZEND_BRK_SPEC_TMP_HANDLER, > -       ZEND_BRK_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_BRK_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_BRK_SPEC_CONST_HANDLER, > -       ZEND_BRK_SPEC_TMP_HANDLER, > -       ZEND_BRK_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_BRK_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_BRK_SPEC_CONST_HANDLER, > -       ZEND_BRK_SPEC_TMP_HANDLER, > -       ZEND_BRK_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_BRK_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_CONT_SPEC_CONST_HANDLER, > -       ZEND_CONT_SPEC_TMP_HANDLER, > -       ZEND_CONT_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_CONT_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_CONT_SPEC_CONST_HANDLER, > -       ZEND_CONT_SPEC_TMP_HANDLER, > -       ZEND_CONT_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_CONT_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_CONT_SPEC_CONST_HANDLER, > -       ZEND_CONT_SPEC_TMP_HANDLER, > -       ZEND_CONT_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_CONT_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_CONT_SPEC_CONST_HANDLER, > -       ZEND_CONT_SPEC_TMP_HANDLER, > -       ZEND_CONT_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_CONT_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_CONT_SPEC_CONST_HANDLER, > -       ZEND_CONT_SPEC_TMP_HANDLER, > -       ZEND_CONT_SPEC_VAR_HANDLER, >        ZEND_NULL_HANDLER, > -       ZEND_CONT_SPEC_CV_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, > +       ZEND_NULL_HANDLER, >        ZEND_BOOL_SPEC_CONST_HANDLER, >        ZEND_BOOL_SPEC_CONST_HANDLER, >        ZEND_BOOL_SPEC_CONST_HANDLER, > Index: Zend/zend_compile.c > =================================================================== > --- Zend/zend_compile.c (revision 305711) > +++ Zend/zend_compile.c (working copy) > @@ -4302,6 +4302,11 @@ >        opline->op1.opline_num = CG(context).current_brk_cont; >        SET_UNUSED(opline->op1); >        if (expr) { > +               if (expr->op_type != IS_CONST) { > +                       zend_error(E_COMPILE_ERROR, "'%s' operator > with > non-constant operand is no longer supported", op == ZEND_BRK ? "break" : > "continue"); > +               } else if (Z_TYPE(expr->u.constant) != IS_LONG || > Z_LVAL(expr->u.constant) < 1) { > +                       zend_error(E_COMPILE_ERROR, "'%s' operator > accepts > only positive numbers", op == ZEND_BRK ? "break" : "continue"); > +               } >                SET_NODE(opline->op2, expr); >        } else { >                LITERAL_LONG(opline->op2, 1); > Index: Zend/zend_vm_def.h > =================================================================== > --- Zend/zend_vm_def.h  (revision 305710) > +++ Zend/zend_vm_def.h  (working copy) > @@ -3171,27 +3171,25 @@ >        ZEND_VM_NEXT_OPCODE(); >  } > > -ZEND_VM_HANDLER(50, ZEND_BRK, ANY, CONST|TMP|VAR|CV) > +ZEND_VM_HANDLER(50, ZEND_BRK, ANY, CONST) >  { >        USE_OPLINE > -       zend_free_op free_op2; >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(GET_OP2_ZVAL_PTR(BP_VAR_R), > opline->op1.opline_num, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->op1.opline_num, >                           EX(op_array), EX_Ts() TSRMLS_CC); >        FREE_OP2(); >        ZEND_VM_JMP(EX(op_array)->opcodes + el->brk); >  } > > -ZEND_VM_HANDLER(51, ZEND_CONT, ANY, CONST|TMP|VAR|CV) > +ZEND_VM_HANDLER(51, ZEND_CONT, ANY, CONST) >  { >        USE_OPLINE > -       zend_free_op free_op2; >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(GET_OP2_ZVAL_PTR(BP_VAR_R), > opline->op1.opline_num, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->op1.opline_num, >                           EX(op_array), EX_Ts() TSRMLS_CC); >        FREE_OP2(); >        ZEND_VM_JMP(EX(op_array)->opcodes + el->cont); > @@ -3204,7 +3202,7 @@ >        zend_brk_cont_element *el; > >        SAVE_OPLINE(); > -       el = zend_brk_cont(opline->op2.zv, opline->extended_value, > +       el = zend_brk_cont(Z_LVAL_P(opline->op2.zv), opline->extended_value, >                           EX(op_array), EX_Ts() TSRMLS_CC); > >        brk_opline = EX(op_array)->opcodes + el->brk; > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org