Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-13 21:35:39 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 499524: Always check for duplicates when destructuring params are present. r=igor

Browse Source 
Detect duplicate names in parameter lists that include destructuring
parameters, regardless of whether the duplication becomes before or
after the destructuring. Let strict mode complaints take care of
themselves after the body has been parsed.

In BindDestructuringArg, there should never be an entry in tc->decls
for the given name if the call to js_LookupLocal didn't detect a
duplicate argument, so we can simply assert that tc->decl.lookup
returns NULL, instead of checking it.

In HashLocalName, we can tighten the assertion: both the new and
existing entries must be JSLOCAL_ARG, since we detect all non-ARG
(i.e., destructuring) duplicates early.
This commit is contained in:
Jim Blandy 2009-11-26 10:23:52 -08:00
parent 87294afa65
commit 5bfdb45b63
4 changed files with 77 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
js/src/jsfun.cpp
View File

@ -2700,7 +2700,7 @@ HashLocalName(JSContext *cx, JSLocalNameMap *map, JSAtom *name,
}
if (entry->name) {
JS_ASSERT(entry->name == name);
JS_ASSERT(entry->localKind == JSLOCAL_ARG);
JS_ASSERT(entry->localKind == JSLOCAL_ARG && localKind == JSLOCAL_ARG);
dup = (JSNameIndexPair *) cx->malloc(sizeof *dup);
if (!dup)
return JS_FALSE;

51
js/src/jsparse.cpp
View File

@ -1731,7 +1731,6 @@ static JSBool
BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom,
JSTreeContext *tc)
{
JSAtomListElement *ale;
JSParseNode *pn;
/* Flag tc so we don't have to lookup arguments on every use. */
@ -1741,10 +1740,6 @@ BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom,
tc->flags |= TCF_FUN_PARAM_EVAL;
JS_ASSERT(tc->flags & TCF_IN_FUNCTION);
ale = tc->decls.lookup(atom);
pn = data->pn;
if (!ale && !Define(pn, atom, tc))
return JS_FALSE;
JSLocalKind localKind = js_LookupLocal(cx, tc->fun, atom, NULL);
if (localKind != JSLOCAL_NONE) {
@ -1752,6 +1747,11 @@ BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom,
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;
uintN index = tc->fun->u.i.nvars;
if (!BindLocalVariable(cx, tc->fun, atom, JSLOCAL_VAR, true))
@ -2578,7 +2578,8 @@ FunctionDef(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc,
JSAtomListElement *ale;
#if JS_HAS_DESTRUCTURING
JSParseNode *item, *list = NULL;
bool destructuringArg = false, duplicatedArg = false;
bool destructuringArg = false;
JSAtom *duplicatedArg = NULL;
#endif
/* Make a TOK_FUNCTION node. */
@ -2807,27 +2808,26 @@ FunctionDef(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc,
case TOK_NAME:
{
/*
* Check for a duplicate parameter name, a "feature" that
* ECMA-262 requires. This is a SpiderMonkey strict warning,
* and a strict mode error, as of ECMAScript 5.
*
* Further, if any argument is a destructuring pattern, forbid
* duplicates. We will report the error either now if we have
* seen a destructuring pattern already, or later when we find
* the first pattern.
*/
JSAtom *atom = CURRENT_TOKEN(ts).t_atom;
if (tc->needStrictChecks() &&
js_LookupLocal(cx, fun, atom, NULL) != JSLOCAL_NONE) {
#if JS_HAS_DESTRUCTURING
if (destructuringArg)
goto report_dup_and_destructuring;
duplicatedArg = true;
#endif
}
if (!DefineArg(pn, atom, fun->nargs, &funtc))
return NULL;
#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
* 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.
*/
if (js_LookupLocal(cx, fun, atom, NULL) != JSLOCAL_NONE) {
duplicatedArg = atom;
if (destructuringArg)
goto report_dup_and_destructuring;
}
#endif
if (!js_AddLocal(cx, fun, atom, JSLOCAL_ARG))
return NULL;
break;
@ -2842,7 +2842,8 @@ FunctionDef(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc,
#if JS_HAS_DESTRUCTURING
report_dup_and_destructuring:
js_ReportCompileErrorNumber(cx, TS(tc->compiler), NULL,
JSDefinition *dn = ALE_DEFN(funtc.decls.lookup(duplicatedArg));
js_ReportCompileErrorNumber(cx, TS(tc->compiler), dn,
JSREPORT_ERROR,
JSMSG_DESTRUCT_DUP_ARG);
return NULL;

5
js/src/tests/js1_8/regress/jstests.list
View File

@ -10,8 +10,8 @@ script regress-433279-03.js
skip script regress-442333-01.js
script regress-452491.js
script regress-453492.js
skip-if(isDebugBuild) fails-if(!isDebugBuild) script regress-455981-01.js
skip-if(isDebugBuild) fails-if(!isDebugBuild) script regress-455981-02.js
script regress-455981-01.js
script regress-455981-02.js
script regress-457065-01.js
script regress-457065-02.js
script regress-458076.js
@ -81,3 +81,4 @@ script regress-479353.js
script regress-479740.js
script regress-481800.js
script regress-483749.js
script regress-499524.js

47
js/src/tests/js1_8/regress/regress-499524.js Normal file
View File

@ -0,0 +1,47 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/*
* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/licenses/publicdomain/
*/
function isSyntaxError(code) {
try {
eval(code);
return false;
} catch (exception) {
if (SyntaxError.prototype.isPrototypeOf(exception))
return true;
throw exception;
};
};
/*
* Duplicate parameter names must be tolerated (as per ES3), unless
* the parameter list uses destructuring, in which case we claim the
* user has opted in to a modicum of sanity, and we forbid duplicate
* parameter names.
*/
assertEq(isSyntaxError("function f(x,x){}"), false);
assertEq(isSyntaxError("function f(x,[x]){})"), true);
assertEq(isSyntaxError("function f(x,{y:x}){})"), true);
assertEq(isSyntaxError("function f(x,{x}){})"), true);
assertEq(isSyntaxError("function f([x],x){})"), true);
assertEq(isSyntaxError("function f({y:x},x){})"), true);
assertEq(isSyntaxError("function f({x},x){})"), true);
assertEq(isSyntaxError("function f([x,x]){}"), true);
assertEq(isSyntaxError("function f({x,x}){}"), true);
assertEq(isSyntaxError("function f({y:x,z:x}){}"), true);
assertEq(isSyntaxError("function f(x,x,[y]){}"), true);
assertEq(isSyntaxError("function f(x,x,{y}){}"), true);
assertEq(isSyntaxError("function f([y],x,x){}"), true);
assertEq(isSyntaxError("function f({y},x,x){}"), true);
assertEq(isSyntaxError("function f(a,b,c,d,e,f,g,h,b,[y]){}"), true);
assertEq(isSyntaxError("function f([y],a,b,c,d,e,f,g,h,a){}"), true);
assertEq(isSyntaxError("function f([a],b,c,d,e,f,g,h,i,a){}"), true);
assertEq(isSyntaxError("function f(a,b,c,d,e,f,g,h,i,[a]){}"), true);