Restore append-only fun->u.i.names rule by binding destructured-to vars for destructuring args after binding all args (619003, r=igor).

This commit is contained in:
Brendan Eich 2010-12-27 15:10:58 -08:00
parent faaefcf02b
commit 0e94dad967
6 changed files with 92 additions and 69 deletions

View File

@ -3777,6 +3777,13 @@ js_EmitFunctionScript(JSContext *cx, JSCodeGenerator *cg, JSParseNode *body)
CG_SWITCH_TO_MAIN(cg);
}
/*
* Strict mode functions' arguments objects copy initial parameter values.
* We create arguments objects lazily -- but that doesn't work for strict
* mode functions where a parameter might be modified and arguments might
* be accessed. For such functions we synthesize an access to arguments to
* initialize it with the original parameter values.
*/
if (cg->needsEagerArguments()) {
CG_SWITCH_TO_PROLOG(cg);
if (js_Emit1(cx, cg, JSOP_ARGUMENTS) < 0 || js_Emit1(cx, cg, JSOP_POP) < 0)

View File

@ -3145,54 +3145,21 @@ JSFunction::addLocal(JSContext *cx, JSAtom *atom, JSLocalKind kind)
return false;
}
Shape **listp = &u.i.names;
Shape *parent = *listp;
jsid id;
/*
* The destructuring formal parameter parser adds a null atom, which we
* encode as an INT id. The parser adds such locals after adding vars for
* the destructured-to parameter bindings -- those must be vars to avoid
* aliasing arguments[i] for any i -- so we must switch u.i.names to a
* dictionary list to cope with insertion "in the middle" of an index-named
* shape for the object or array argument.
*/
bool findArgInsertionPoint = false;
if (!atom) {
JS_ASSERT(kind == JSLOCAL_ARG);
if (u.i.nvars != 0) {
/*
* A dictionary list needed only if the destructing pattern wasn't
* empty, i.e., there were vars for its destructured-to bindings.
*/
if (!parent->inDictionary() && !(parent = Shape::newDictionaryList(cx, listp)))
return false;
findArgInsertionPoint = true;
}
id = INT_TO_JSID(nargs);
} else {
if (kind == JSLOCAL_ARG && parent->inDictionary())
findArgInsertionPoint = true;
id = ATOM_TO_JSID(atom);
}
if (findArgInsertionPoint) {
while (parent->parent && parent->getter() != GetCallArg) {
++parent->slot;
JS_ASSERT(parent->slot == parent->slotSpan);
++parent->slotSpan;
listp = &parent->parent;
parent = *listp;
}
}
Shape child(id, getter, setter, slot, attrs, Shape::HAS_SHORTID, *indexp);
Shape *shape = parent->getChild(cx, child, listp);
Shape *shape = u.i.names->getChild(cx, child, &u.i.names);
if (!shape)
return false;
JS_ASSERT_IF(!shape->inDictionary(), u.i.names == shape);
JS_ASSERT(u.i.names == shape);
++*indexp;
return true;
}

View File

@ -214,6 +214,11 @@ struct JSFunction : public JSObject_Slots2
const js::Shape *lastVar() const;
const js::Shape *lastUpvar() const { return u.i.names; }
/*
* The parser builds shape paths for functions, usable by Call objects at
* runtime, by calling addLocal. All locals of ARG kind must be addLocal'ed
* before any VAR kind, and VAR before UPVAR.
*/
bool addLocal(JSContext *cx, JSAtom *atom, JSLocalKind kind);
/*

View File

@ -1788,8 +1788,6 @@ static JSBool
BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom,
JSTreeContext *tc)
{
JSParseNode *pn;
/* Flag tc so we don't have to lookup arguments on every use. */
if (atom == tc->parser->context->runtime->atomState.argumentsAtom)
tc->flags |= TCF_FUN_PARAM_ARGUMENTS;
@ -1798,25 +1796,21 @@ BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom,
JS_ASSERT(tc->inFunction());
JSLocalKind localKind = tc->fun()->lookupLocal(cx, atom, NULL);
if (localKind != JSLOCAL_NONE) {
if (tc->decls.lookup(atom)) {
ReportCompileErrorNumber(cx, TS(tc->parser), NULL, JSREPORT_ERROR,
JSMSG_DESTRUCT_DUP_ARG);
return JS_FALSE;
}
JS_ASSERT(!tc->decls.lookup(atom));
pn = data->pn;
if (!Define(pn, atom, tc))
return JS_FALSE;
/*
* Distinguish destructured-to binding nodes as vars, not args, by setting
* pn_op to JSOP_SETLOCAL. Parser::functionDef checks for this pn_op value
* when processing the destructuring-assignment AST prelude induced by such
* destructuring args in Parser::functionArguments.
*/
data->pn->pn_op = JSOP_SETLOCAL;
uintN index = tc->fun()->u.i.nvars;
if (!BindLocalVariable(cx, tc->fun(), atom, JSLOCAL_VAR, true))
return JS_FALSE;
pn->pn_op = JSOP_SETLOCAL;
pn->pn_cookie.set(tc->staticLevel, index);
pn->pn_dflags |= PND_BOUND;
return JS_TRUE;
return Define(data->pn, atom, tc);
}
#endif /* JS_HAS_DESTRUCTURING */
@ -2721,6 +2715,12 @@ LeaveFunction(JSParseNode *fn, JSTreeContext *funtc, JSAtom *funAtom = NULL,
static bool
DefineGlobal(JSParseNode *pn, JSCodeGenerator *cg, JSAtom *atom);
/*
* FIXME? this Parser method was factored from Parser::functionDef with minimal
* change, hence the funtc ref param, funbox, and fun. It probably should match
* functionBody, etc., and use tc, tc->funbox, and tc->fun() instead of taking
* explicit parameters.
*/
bool
Parser::functionArguments(JSTreeContext &funtc, JSFunctionBox *funbox, JSFunction *fun,
JSParseNode **listp)
@ -2766,7 +2766,7 @@ Parser::functionArguments(JSTreeContext &funtc, JSFunctionBox *funbox, JSFunctio
* Adjust fun->nargs to count the single anonymous positional
* parameter that is to be destructured.
*/
jsint slot = fun->nargs;
uintN slot = fun->nargs;
if (!fun->addLocal(context, NULL, JSLOCAL_ARG))
return false;
@ -2780,7 +2780,7 @@ Parser::functionArguments(JSTreeContext &funtc, JSFunctionBox *funbox, JSFunctio
return false;
rhs->pn_type = TOK_NAME;
rhs->pn_op = JSOP_GETARG;
rhs->pn_cookie.set(funtc.staticLevel, uint16(slot));
rhs->pn_cookie.set(funtc.staticLevel, slot);
rhs->pn_dflags |= PND_BOUND;
JSParseNode *item =
@ -2803,25 +2803,28 @@ Parser::functionArguments(JSTreeContext &funtc, JSFunctionBox *funbox, JSFunctio
case TOK_NAME:
{
JSAtom *atom = tokenStream.currentToken().t_atom;
if (!DefineArg(funbox->node, atom, fun->nargs, &funtc))
return false;
#ifdef JS_HAS_DESTRUCTURING
/*
* ECMA-262 requires us to support duplicate parameter names, but if the
* parameter list includes destructuring, we consider the code to have
* opted in to higher standards, and forbid duplicates. We may see a
* "opted in" to higher standards, and forbid duplicates. We may see a
* destructuring parameter later, so always note duplicates now.
*
* Duplicates are warned about (strict option) or cause errors (strict
* mode code), but we do those tests in one place below, after having
* parsed the body in case it begins with a "use strict"; directive.
*/
if (fun->lookupLocal(context, atom, NULL) != JSLOCAL_NONE) {
if (funtc.decls.lookup(atom)) {
duplicatedArg = atom;
if (destructuringArg)
goto report_dup_and_destructuring;
}
#endif
if (!DefineArg(funbox->node, atom, fun->nargs, &funtc))
return false;
if (!fun->addLocal(context, atom, JSLOCAL_ARG))
return false;
break;
@ -2982,10 +2985,38 @@ Parser::functionDef(JSAtom *funAtom, FunctionType type, uintN lambda)
JSFunction *fun = (JSFunction *) funbox->object;
/* Now parse formal argument list and compute fun->nargs. */
JSParseNode *prolog = NULL;
if (!functionArguments(funtc, funbox, fun, &prolog))
JSParseNode *prelude = NULL;
if (!functionArguments(funtc, funbox, fun, &prelude))
return NULL;
#if JS_HAS_DESTRUCTURING
/*
* If there were destructuring formal parameters, bind the destructured-to
* local variables now that we've parsed all the regular and destructuring
* formal parameters. Because JSFunction::addLocal must be called first for
* all ARGs, then all VARs, finally all UPVARs, we can't bind vars induced
* by formal parameter destructuring until after Parser::functionArguments
* has returned.
*/
if (prelude) {
JSAtomListIterator iter(&funtc.decls);
while (JSAtomListElement *ale = iter()) {
JSParseNode *apn = ALE_DEFN(ale);
/* Filter based on pn_op -- see BindDestructuringArg, above. */
if (apn->pn_op != JSOP_SETLOCAL)
continue;
uintN index = fun->u.i.nvars;
if (!BindLocalVariable(context, fun, apn->pn_atom, JSLOCAL_VAR, true))
return NULL;
apn->pn_cookie.set(funtc.staticLevel, index);
apn->pn_dflags |= PND_BOUND;
}
}
#endif
if (type == GETTER && fun->nargs > 0) {
reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_ACCESSOR_WRONG_ARGS,
"getter", "no", "s");
@ -3027,14 +3058,6 @@ Parser::functionDef(JSAtom *funAtom, FunctionType type, uintN lambda)
#endif
pn->pn_pos.end = tokenStream.currentToken().pos.end;
/*
* Strict mode functions' arguments objects copy initial parameter values.
* We create arguments objects lazily -- but that doesn't work for strict
* mode functions where a parameter might be modified and arguments might
* be accessed. For such functions we synthesize an access to arguments to
* initialize it with the original parameter values.
*/
/*
* Fruit of the poisonous tree: if a closure calls eval, we consider the
* parent to call eval. We need this for two reasons: (1) the Jaegermonkey
@ -3054,11 +3077,11 @@ Parser::functionDef(JSAtom *funAtom, FunctionType type, uintN lambda)
#if JS_HAS_DESTRUCTURING
/*
* If there were destructuring formal parameters, prepend the initializing
* comma expression that we synthesized to body. If the body is a return
* comma expression that we synthesized to body. If the body is a return
* node, we must make a special TOK_SEQ node, to prepend the destructuring
* code without bracing the decompilation of the function body.
*/
if (prolog) {
if (prelude) {
if (body->pn_arity != PN_LIST) {
JSParseNode *block;
@ -3078,7 +3101,7 @@ Parser::functionDef(JSAtom *funAtom, FunctionType type, uintN lambda)
item->pn_type = TOK_SEMI;
item->pn_pos.begin = item->pn_pos.end = body->pn_pos.begin;
item->pn_kid = prolog;
item->pn_kid = prelude;
item->pn_next = body->pn_head;
body->pn_head = item;
if (body->pn_tail == &body->pn_head)

View File

@ -60,5 +60,6 @@ script regress-617405-2.js
script regress-618572.js
skip-if(!xulRuntime.shell) script regress-618576.js # uses evalcx
fails-if(!xulRuntime.shell) script regress-618652.js
script regress-619003.js
script regress-620376-1.js
script regress-620376-2.js

View File

@ -0,0 +1,20 @@
/*
* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/licenses/publicdomain/
*/
var expect = 'SyntaxError: duplicate argument is mixed with destructuring pattern';
var actual = 'No error';
var a = [];
// Test up to 200 to cover tunables such as js::PropertyTree::MAX_HEIGHT.
for (var i = 0; i < 200; i++) {
a.push("b" + i);
try {
eval("(function ([" + a.join("],[") + "],a,a){})");
} catch (e) {
actual = '' + e;
}
assertEq(actual, expect);
}
reportCompare(0, 0, "ok");