Bug 889628, part 2 - Do not look at lhs->getOp() when selecting opcodes for destructuring assignment, except if lhs is a name and we just called BindNameToSlot. r=Waldo.

--HG--
extra : rebase_source : 955e32774099d85cab5b5d6b83a9d5233493fa08
This commit is contained in:
Jason Orendorff 2013-07-15 10:28:35 -05:00
parent d7d180d58c
commit 1d75ca964d
2 changed files with 69 additions and 71 deletions

View File

@ -2731,65 +2731,90 @@ EmitDestructuringLHS(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn,
if (Emit1(cx, bce, JSOP_POP) < 0)
return false;
}
} else if (emitOption == PushInitialValues) {
// The lhs is a simple name so the to-be-destructured value is
// its initial value and there is nothing to do.
JS_ASSERT(pn->getOp() == JSOP_GETLOCAL);
JS_ASSERT(pn->pn_dflags & PND_BOUND);
} else {
if (emitOption == PushInitialValues) {
/*
* The lhs is a simple name so the to-be-destructured value is
* its initial value and there is nothing to do.
*/
JS_ASSERT(pn->getOp() == JSOP_GETLOCAL);
JS_ASSERT(pn->pn_dflags & PND_BOUND);
return true;
}
// All paths below must pop after assigning to the lhs.
/* All paths below must pop after assigning to the lhs. */
if (pn->isKind(PNK_NAME)) {
switch (pn->getKind()) {
case PNK_NAME:
if (!BindNameToSlot(cx, bce, pn))
return false;
/* Allow 'const [x,y] = o', make 'const x,y; [x,y] = o' a nop. */
// Allow 'const [x,y] = o', make 'const x,y; [x,y] = o' a nop.
if (pn->isConst() && !pn->isDefn())
return Emit1(cx, bce, JSOP_POP) >= 0;
}
switch (pn->getOp()) {
case JSOP_SETNAME:
case JSOP_SETGNAME:
/*
* NB: pn is a PN_NAME node, not a PN_BINARY. Nevertheless,
* we want to emit JSOP_ENUMELEM, which has format JOF_ELEM.
* So here and for JSOP_ENUMCONSTELEM, we use EmitElemOp.
*/
switch (pn->getOp()) {
case JSOP_SETNAME:
case JSOP_SETGNAME:
// This is like ordinary assignment, but with one difference.
//
// In `a = b`, we first determine a binding for `a` (using
// JSOP_BINDNAME or JSOP_BINDGNAME), then we evaluate `b`, then
// a JSOP_SETNAME instruction.
//
// In `[a] = [b]`, per spec, `b` is evaluated first, then we
// determine a binding for `a`. Then we need to do assignment--
// but the operands are on the stack in the wrong order for
// JSOP_SETPROP, so we use JSOP_ENUMELEM instead.
//
// EmitElemOp ordinarily works with PNK_ELEM nodes, naturally,
// but it has special code to handle PNK_NAME nodes in this one
// case.
if (!EmitElemOp(cx, pn, JSOP_ENUMELEM, bce))
return false;
break;
case JSOP_SETCONST:
// As above.
if (!EmitElemOp(cx, pn, JSOP_ENUMCONSTELEM, bce))
return false;
break;
case JSOP_SETLOCAL:
case JSOP_SETARG:
if (!EmitVarOp(cx, pn, pn->getOp(), bce))
return false;
if (Emit1(cx, bce, JSOP_POP) < 0)
return false;
break;
default:
MOZ_ASSUME_UNREACHABLE("EmitDestructuringLHS: bad name op");
}
break;
case PNK_DOT:
case PNK_ELEM:
// See the (PNK_NAME, JSOP_SETNAME) case above.
//
// In `a.x = b`, `a` is evaluated first, then `b`, then a
// JSOP_SETPROP instruction.
//
// In `[a.x] = [b]`, per spec, `b` is evaluated before `a`. Then we
// need a property set -- but the operands are on the stack in the
// wrong order for JSOP_SETPROP, so we use JSOP_ENUMELEM instead.
//
// EmitElemOp has special code to handle PNK_DOT nodes in this one
// case.
if (!EmitElemOp(cx, pn, JSOP_ENUMELEM, bce))
return false;
break;
case JSOP_SETCONST:
if (!EmitElemOp(cx, pn, JSOP_ENUMCONSTELEM, bce))
return false;
break;
case JSOP_SETLOCAL:
case JSOP_SETARG:
if (!EmitVarOp(cx, pn, pn->getOp(), bce))
return false;
if (Emit1(cx, bce, JSOP_POP) < 0)
return false;
break;
case JSOP_CALL:
case JSOP_EVAL:
case JSOP_FUNCALL:
case JSOP_FUNAPPLY:
case PNK_CALL:
JS_ASSERT(pn->pn_xflags & PNX_SETCALL);
if (!EmitTree(cx, bce, pn))
return false;
/*
* We just emitted JSOP_SETCALL which will always throw.
* Pop the call return value and the RHS.
*/
// Pop the call return value and the RHS, presumably for the
// benefit of bytecode analysis. (The interpreter will never reach
// these instructions since we just emitted JSOP_SETCALL, which
// always throws. It's possible no analyses actually depend on this
// either.)
if (Emit1(cx, bce, JSOP_POP) < 0)
return false;
if (Emit1(cx, bce, JSOP_POP) < 0)
@ -2797,16 +2822,7 @@ EmitDestructuringLHS(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn,
break;
default:
{
if (!EmitTree(cx, bce, pn))
return false;
if (!EmitElemOpBase(cx, bce, JSOP_ENUMELEM))
return false;
break;
}
case JSOP_ENUMELEM:
JS_ASSERT(0);
MOZ_ASSUME_UNREACHABLE("EmitDestructuringLHS: bad lhs kind");
}
}

View File

@ -2919,24 +2919,6 @@ Parser<FullParseHandler>::bindDestructuringVar(BindData<FullParseHandler> *data,
return true;
}
/*
* Here, we are destructuring {... P: Q, ...} = R, where P is any id, Q is any
* LHS expression except a destructuring initialiser, and R is on the stack.
* Because R is already evaluated, the usual LHS-specialized bytecodes won't
* work. After pushing R[P] we need to evaluate Q's "reference base" QB and
* then push its property name QN. At this point the stack looks like
*
* [... R, R[P], QB, QN]
*
* We need to set QB[QN] = R[P]. This is a job for JSOP_ENUMELEM, which takes
* its operands with left-hand side above right-hand side:
*
* [rval, lval, xval]
*
* and pops all three values, setting lval[xval] = rval. But we cannot select
* JSOP_ENUMELEM yet, because the LHS may turn out to be an arg or local var,
* which can be optimized further. So we select JSOP_SETNAME.
*/
template <>
bool
Parser<FullParseHandler>::bindDestructuringLHS(ParseNode *pn)