Bug 772328 - function statements should restate, not shadow, formal parameters (r=ejpbruel)

--HG--
extra : rebase_source : a6425a0e317ffd1437f0f6230d03f3487edc8389
This commit is contained in:
Luke Wagner 2012-07-24 13:44:23 -07:00
parent a706d68d60
commit e8ac77b8f5
5 changed files with 205 additions and 185 deletions

View File

@ -815,12 +815,6 @@ EmitAtomIncDec(JSContext *cx, JSAtom *atom, JSOp op, BytecodeEmitter *bce)
return true;
}
static bool
EmitFunctionOp(JSContext *cx, JSOp op, uint32_t index, BytecodeEmitter *bce)
{
return EmitIndex32(cx, op, index, bce);
}
static bool
EmitObjectOp(JSContext *cx, ObjectBox *objbox, JSOp op, BytecodeEmitter *bce)
{
@ -1034,8 +1028,7 @@ BytecodeEmitter::noteClosedVar(ParseNode *pn)
#ifdef DEBUG
JS_ASSERT(shouldNoteClosedName(pn));
Definition *dn = (Definition *)pn;
JS_ASSERT(dn->kind() == Definition::VAR || dn->kind() == Definition::CONST ||
dn->kind() == Definition::FUNCTION);
JS_ASSERT(dn->kind() == Definition::VAR || dn->kind() == Definition::CONST);
JS_ASSERT(pn->pn_cookie.slot() < sc->bindings.numVars());
for (size_t i = 0; i < closedVars.length(); ++i)
JS_ASSERT(closedVars[i] != pn->pn_cookie.slot());
@ -1211,12 +1204,14 @@ BindNameToSlot(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
{
JS_ASSERT(pn->isKind(PNK_NAME) || pn->isKind(PNK_INTRINSICNAME));
/* Don't attempt if 'pn' is already bound or deoptimized or a nop. */
JSOp op = pn->getOp();
if (pn->isBound() || pn->isDeoptimized() || op == JSOP_NOP)
JS_ASSERT_IF(pn->isKind(PNK_FUNCTION), pn->isBound());
/* Don't attempt if 'pn' is already bound or deoptimized or a function. */
if (pn->isBound() || pn->isDeoptimized())
return true;
/* JSOP_CALLEE is pre-bound by definition. */
JSOp op = pn->getOp();
JS_ASSERT(op != JSOP_CALLEE);
JS_ASSERT(JOF_OPTYPE(op) == JOF_ATOM);
@ -1236,8 +1231,6 @@ BindNameToSlot(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
return true;
}
JS_ASSERT_IF(dn->kind() == Definition::CONST, pn->pn_dflags & PND_CONST);
/*
* Turn attempts to mutate const-declared bindings into get ops (for
* pre-increment and pre-decrement ops, our caller will have to emit
@ -1331,9 +1324,6 @@ BindNameToSlot(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
* rewrite pn_op and update pn accordingly.
*/
switch (dn->kind()) {
case Definition::UNKNOWN:
return true;
case Definition::ARG:
switch (op) {
case JSOP_NAME: op = JSOP_GETARG; break;
@ -1348,55 +1338,6 @@ BindNameToSlot(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
break;
case Definition::VAR:
if (dn->isOp(JSOP_CALLEE)) {
JS_ASSERT(op != JSOP_CALLEE);
/*
* Currently, the ALIASEDVAR ops do not support accessing the
* callee of a DeclEnvObject, so use NAME.
*/
if (dn->pn_cookie.level() != bce->script->staticLevel)
return true;
JS_ASSERT(bce->sc->fun()->flags & JSFUN_LAMBDA);
JS_ASSERT(pn->pn_atom == bce->sc->fun()->atom);
/*
* Leave pn->isOp(JSOP_NAME) if bce->fun is heavyweight to
* address two cases: a new binding introduced by eval, and
* assignment to the name in strict mode.
*
* var fun = (function f(s) { eval(s); return f; });
* assertEq(fun("var f = 42"), 42);
*
* ECMAScript specifies that a function expression's name is bound
* in a lexical environment distinct from that used to bind its
* named parameters, the arguments object, and its variables. The
* new binding for "var f = 42" shadows the binding for the
* function itself, so the name of the function will not refer to
* the function.
*
* (function f() { "use strict"; f = 12; })();
*
* Outside strict mode, assignment to a function expression's name
* has no effect. But in strict mode, this attempt to mutate an
* immutable binding must throw a TypeError. We implement this by
* not optimizing such assignments and by marking such functions as
* heavyweight, ensuring that the function name is represented in
* the scope chain so that assignment will throw a TypeError.
*/
if (!bce->sc->funIsHeavyweight()) {
op = JSOP_CALLEE;
pn->pn_dflags |= PND_CONST;
}
pn->setOp(op);
pn->pn_dflags |= PND_BOUND;
return true;
}
/* FALL THROUGH */
case Definition::FUNCTION:
case Definition::CONST:
case Definition::LET:
switch (op) {
@ -1411,8 +1352,55 @@ BindNameToSlot(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
}
break;
default:
JS_NOT_REACHED("unexpected dn->kind()");
case Definition::NAMED_LAMBDA:
JS_ASSERT(dn->isOp(JSOP_CALLEE));
JS_ASSERT(op != JSOP_CALLEE);
/*
* Currently, the ALIASEDVAR ops do not support accessing the
* callee of a DeclEnvObject, so use NAME.
*/
if (dn->pn_cookie.level() != bce->script->staticLevel)
return true;
JS_ASSERT(bce->sc->fun()->flags & JSFUN_LAMBDA);
JS_ASSERT(pn->pn_atom == bce->sc->fun()->atom);
/*
* Leave pn->isOp(JSOP_NAME) if bce->fun is heavyweight to
* address two cases: a new binding introduced by eval, and
* assignment to the name in strict mode.
*
* var fun = (function f(s) { eval(s); return f; });
* assertEq(fun("var f = 42"), 42);
*
* ECMAScript specifies that a function expression's name is bound
* in a lexical environment distinct from that used to bind its
* named parameters, the arguments object, and its variables. The
* new binding for "var f = 42" shadows the binding for the
* function itself, so the name of the function will not refer to
* the function.
*
* (function f() { "use strict"; f = 12; })();
*
* Outside strict mode, assignment to a function expression's name
* has no effect. But in strict mode, this attempt to mutate an
* immutable binding must throw a TypeError. We implement this by
* not optimizing such assignments and by marking such functions as
* heavyweight, ensuring that the function name is represented in
* the scope chain so that assignment will throw a TypeError.
*/
if (!bce->sc->funIsHeavyweight()) {
op = JSOP_CALLEE;
pn->pn_dflags |= PND_CONST;
}
pn->setOp(op);
pn->pn_dflags |= PND_BOUND;
return true;
case Definition::PLACEHOLDER:
return true;
}
/*
@ -4850,7 +4838,7 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
* for the already-emitted function definition prolog opcode. See
* comments in EmitStatementList.
*/
JS_ASSERT(pn->isOp(JSOP_NOP));
JS_ASSERT(pn->functionIsHoisted());
JS_ASSERT(bce->sc->inFunction());
return EmitFunctionDefNop(cx, bce, pn->pn_index);
}
@ -4893,12 +4881,12 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
/* Make the function object a literal in the outer script's pool. */
unsigned index = bce->objectList.add(pn->pn_funbox);
/* Emit a bytecode pointing to the closure object in its immediate. */
if (pn->getOp() != JSOP_NOP) {
/* Non-hoisted functions simply emit their respective op. */
if (!pn->functionIsHoisted()) {
if (pn->pn_funbox->inGenexpLambda && NewSrcNote(cx, bce, SRC_GENEXP) < 0)
return false;
return EmitFunctionOp(cx, pn->getOp(), index, bce);
return EmitIndex32(cx, pn->getOp(), index, bce);
}
/*
@ -4911,16 +4899,15 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
* definitions can be scheduled before generating the rest of code.
*/
if (!bce->sc->inFunction()) {
JS_ASSERT(!bce->topStmt);
JS_ASSERT(pn->pn_cookie.isFree());
if (pn->pn_cookie.isFree()) {
bce->switchToProlog();
if (!EmitFunctionOp(cx, JSOP_DEFFUN, index, bce))
return false;
if (!UpdateLineNumberNotes(cx, bce, pn->pn_pos.begin.lineno))
return false;
bce->switchToMain();
}
JS_ASSERT(pn->getOp() == JSOP_NOP);
JS_ASSERT(!bce->topStmt);
bce->switchToProlog();
if (!EmitIndex32(cx, JSOP_DEFFUN, index, bce))
return false;
if (!UpdateLineNumberNotes(cx, bce, pn->pn_pos.begin.lineno))
return false;
bce->switchToMain();
/* Emit NOP for the decompiler. */
if (!EmitFunctionDefNop(cx, bce, index))
@ -4928,18 +4915,29 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
} else {
#ifdef DEBUG
BindingIter bi(cx, bce->sc->bindings.lookup(cx, fun->atom->asPropertyName()));
JS_ASSERT(bi->kind == VARIABLE || bi->kind == CONSTANT);
JS_ASSERT(bi->kind == VARIABLE || bi->kind == CONSTANT || bi->kind == ARGUMENT);
JS_ASSERT(bi.frameIndex() < JS_BIT(20));
#endif
pn->pn_index = index;
if (bce->shouldNoteClosedName(pn) && !bce->noteClosedVar(pn))
return false;
if (bce->shouldNoteClosedName(pn)) {
Definition::Kind kind = ((Definition *)pn)->kind();
if (kind == Definition::ARG) {
if (!bce->noteClosedArg(pn))
return false;
} else {
JS_ASSERT(kind == Definition::VAR);
if (!bce->noteClosedVar(pn))
return false;
}
}
if (NewSrcNote(cx, bce, SRC_CONTINUE) < 0)
return false;
if (!EmitIndexOp(cx, JSOP_LAMBDA, index, bce))
return false;
if (!EmitVarOp(cx, pn, JSOP_SETLOCAL, bce))
JS_ASSERT(pn->getOp() == JSOP_GETLOCAL || pn->getOp() == JSOP_GETARG);
JSOp setOp = pn->getOp() == JSOP_GETLOCAL ? JSOP_SETLOCAL : JSOP_SETARG;
if (!EmitVarOp(cx, pn, setOp, bce))
return false;
if (Emit1(cx, bce, JSOP_POP) < 0)
return false;
@ -6069,17 +6067,9 @@ frontend::EmitTree(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn)
// Currently this is used only for functions, as compile-as-we go
// mode for scripts does not allow separate emitter passes.
for (ParseNode *pn2 = pnchild; pn2; pn2 = pn2->pn_next) {
if (pn2->isKind(PNK_FUNCTION)) {
if (pn2->isOp(JSOP_NOP)) {
if (!EmitTree(cx, bce, pn2))
return false;
} else {
// JSOP_DEFFUN in a top-level block with function
// definitions appears, for example, when "if (true)"
// is optimized away from "if (true) function x() {}".
// See bug 428424.
JS_ASSERT(pn2->isOp(JSOP_DEFFUN));
}
if (pn2->isKind(PNK_FUNCTION) && pn2->functionIsHoisted()) {
if (!EmitTree(cx, bce, pn2))
return false;
}
}
}

View File

@ -778,6 +778,16 @@ struct ParseNode {
return pn_cookie.slot();
}
bool functionIsHoisted() const {
JS_ASSERT(pn_arity == PN_FUNC);
JS_ASSERT(isOp(JSOP_LAMBDA) || // lambda, genexpr
isOp(JSOP_DEFFUN) || // non-body-level function statement
isOp(JSOP_NOP) || // body-level function stmt in global code
isOp(JSOP_GETLOCAL) || // body-level function stmt in function code
isOp(JSOP_GETARG)); // body-level function redeclaring formal
return !(isOp(JSOP_LAMBDA) || isOp(JSOP_DEFFUN));
}
inline bool test(unsigned flag) const;
bool isLet() const { return test(PND_LET); }
@ -1359,18 +1369,23 @@ struct Definition : public ParseNode
return pn_cookie.isFree();
}
enum Kind { VAR, CONST, LET, FUNCTION, ARG, UNKNOWN };
enum Kind { VAR, CONST, LET, ARG, NAMED_LAMBDA, PLACEHOLDER };
bool canHaveInitializer() { return int(kind()) <= int(LET) || kind() == ARG; }
bool canHaveInitializer() { return int(kind()) <= int(ARG); }
static const char *kindString(Kind kind);
Kind kind() {
if (getKind() == PNK_FUNCTION)
return FUNCTION;
if (getKind() == PNK_FUNCTION) {
if (isOp(JSOP_GETARG))
return ARG;
return VAR;
}
JS_ASSERT(getKind() == PNK_NAME);
if (isOp(JSOP_NOP))
return UNKNOWN;
if (isOp(JSOP_CALLEE))
return NAMED_LAMBDA;
if (isPlaceholder())
return PLACEHOLDER;
if (isOp(JSOP_GETARG))
return ARG;
if (isConst())

View File

@ -854,6 +854,22 @@ MakeAssignment(ParseNode *pn, ParseNode *rhs, Parser *parser)
static bool
MakeDefIntoUse(Definition *dn, ParseNode *pn, JSAtom *atom, Parser *parser)
{
TreeContext *tc = parser->tc;
/*
* In a function, dn must have been bound to an argument or local to be in
* tc->decls. pn is going to take dn's place in tc->decls, so copy over the
* cookie and op so that pn gets the same binding.
*/
if (tc->sc->inFunction()) {
JS_ASSERT(!dn->pn_cookie.isFree());
JS_ASSERT(dn->isBound());
pn->pn_cookie = dn->pn_cookie;
JS_ASSERT(JOF_OPTYPE(dn->getOp()) == JOF_QARG || JOF_OPTYPE(dn->getOp()) == JOF_LOCAL);
pn->setOp(JOF_OPTYPE(dn->getOp()) == JOF_QARG ? JSOP_GETARG : JSOP_GETLOCAL);
pn->pn_dflags |= PND_BOUND;
}
/* Turn pn into a definition. */
parser->tc->decls.updateFirst(atom, (Definition *) pn);
pn->setDefn(true);
@ -883,9 +899,8 @@ MakeDefIntoUse(Definition *dn, ParseNode *pn, JSAtom *atom, Parser *parser)
*
* both asserts are valid.
*/
if (dn->kind() == Definition::FUNCTION) {
JS_ASSERT(dn->getKind() == PNK_FUNCTION);
JS_ASSERT(dn->isOp(JSOP_NOP));
if (dn->getKind() == PNK_FUNCTION) {
JS_ASSERT(dn->functionIsHoisted());
pn->dn_uses = dn->pn_link;
parser->prepareNodeForMutation(dn);
dn->setKind(PNK_NOP);
@ -1147,6 +1162,7 @@ LeaveFunction(ParseNode *fn, Parser *parser, PropertyName *funName = NULL,
return false;
dn->pn_dflags |= PND_BOUND;
foundCallee = 1;
JS_ASSERT(dn->kind() == Definition::NAMED_LAMBDA);
continue;
}
@ -1461,25 +1477,24 @@ Parser::functionDef(HandlePropertyName funName, FunctionType type, FunctionSynta
pn->pn_cookie.makeFree();
pn->pn_dflags = 0;
/*
* Record names for function statements in tc->decls so we know when to
* avoid optimizing variable references that might name a function.
*/
/* Function statements add a binding to the enclosing scope. */
bool bodyLevel = tc->atBodyLevel();
if (kind == Statement) {
/*
* Handle redeclaration and optimize cases where we can statically bind the
* function (thereby avoiding JSOP_DEFFUN and dynamic name lookup).
*/
if (Definition *dn = tc->decls.lookupFirst(funName)) {
Definition::Kind dn_kind = dn->kind();
JS_ASSERT(!dn->isUsed());
JS_ASSERT(dn->isDefn());
if (context->hasStrictOption() || dn_kind == Definition::CONST) {
if (context->hasStrictOption() || dn->kind() == Definition::CONST) {
JSAutoByteString name;
Reporter reporter = (dn_kind != Definition::CONST)
Reporter reporter = (dn->kind() != Definition::CONST)
? &Parser::reportStrictWarning
: &Parser::reportError;
if (!js_AtomToPrintableString(context, funName, &name) ||
!(this->*reporter)(NULL, JSMSG_REDECLARED_VAR, Definition::kindString(dn_kind),
!(this->*reporter)(NULL, JSMSG_REDECLARED_VAR, Definition::kindString(dn->kind()),
name.ptr()))
{
return NULL;
@ -1502,17 +1517,11 @@ Parser::functionDef(HandlePropertyName funName, FunctionType type, FunctionSynta
* pre-created definition node for this function that primaryExpr
* put in tc->lexdeps on first forward reference, and recycle pn.
*/
if (Definition *fn = tc->lexdeps.lookupDefn(funName)) {
JS_ASSERT(fn->isDefn());
fn->setKind(PNK_FUNCTION);
fn->setArity(PN_FUNC);
fn->pn_pos.begin = pn->pn_pos.begin;
/*
* Set fn->pn_pos.end too, in case of error before we parse the
* closing brace. See bug 640075.
*/
fn->pn_pos.end = pn->pn_pos.end;
fn->pn_body = NULL;
@ -1525,41 +1534,64 @@ Parser::functionDef(HandlePropertyName funName, FunctionType type, FunctionSynta
if (!Define(pn, funName, tc))
return NULL;
/*
* A function directly inside another's body needs only a local
* variable to bind its name to its value, and not an activation object
* property (it might also need the activation property, if the outer
* function contains with statements, e.g., but the stack slot wins
* when BytecodeEmitter.cpp's BindNameToSlot can optimize a JSOP_NAME
* into a JSOP_GETLOCAL bytecode).
*/
if (tc->sc->inFunction()) {
unsigned varIndex = tc->sc->bindings.numVars();
if (!tc->sc->bindings.addVariable(context, funName))
return NULL;
if (!pn->pn_cookie.set(context, tc->staticLevel, varIndex))
return NULL;
pn->setOp(JSOP_GETLOCAL);
}
}
/*
* A function directly inside another's body needs only a local
* variable to bind its name to its value, and not an activation object
* property (it might also need the activation property, if the outer
* function contains with statements, e.g., but the stack slot wins
* when BytecodeEmitter.cpp's BindNameToSlot can optimize a JSOP_NAME
* into a JSOP_GETLOCAL bytecode).
* As a SpiderMonkey-specific extension, non-body-level function
* statements (e.g., functions in an "if" or "while" block) are
* dynamically bound when control flow reaches the statement. The
* emitter normally emits functions in two passes (see PNK_ARGSBODY).
* To distinguish
*/
if (bodyLevel && tc->sc->inFunction()) {
/*
* Define a local in the outer function so that BindNameToSlot
* can properly optimize accesses. Note that we need a local
* variable, not an argument, for the function statement. Thus
* we add a variable even if a parameter with the given name
* already exists.
*/
BindingIter bi(context, tc->sc->bindings.lookup(context, funName));
if (!bi || bi->kind != CONSTANT) {
unsigned varIndex;
if (!bi || bi->kind == ARGUMENT) {
varIndex = tc->sc->bindings.numVars();
if (!tc->sc->bindings.addVariable(context, funName))
return NULL;
} else {
JS_ASSERT(bi->kind == VARIABLE);
varIndex = bi.frameIndex();
}
if (bodyLevel) {
JS_ASSERT(pn->functionIsHoisted());
JS_ASSERT_IF(tc->sc->inFunction(), !pn->pn_cookie.isFree());
JS_ASSERT_IF(!tc->sc->inFunction(), pn->pn_cookie.isFree());
} else {
JS_ASSERT(tc->sc->strictModeState != StrictMode::STRICT);
JS_ASSERT(pn->pn_cookie.isFree());
tc->sc->setFunMightAliasLocals();
tc->sc->setFunHasExtensibleScope();
tc->sc->setFunIsHeavyweight();
pn->setOp(JSOP_DEFFUN);
if (!pn->pn_cookie.set(context, tc->staticLevel, varIndex))
/*
* Instead of setting bindingsAccessedDynamically, which would be
* overly conservative, remember the names of all function
* statements and mark any bindings with the same as aliased at the
* end of functionBody.
*/
if (!tc->funcStmts) {
tc->funcStmts = context->new_<FuncStmtSet>(context);
if (!tc->funcStmts || !tc->funcStmts->init())
return NULL;
pn->pn_dflags |= PND_BOUND;
}
if (!tc->funcStmts->put(funName))
return NULL;
}
/* No further binding (in BindNameToSlot) is needed for functions. */
pn->pn_dflags |= PND_BOUND;
} else {
/* A function expression does not introduce any binding. */
pn->setOp(JSOP_LAMBDA);
}
TreeContext *outertc = tc;
@ -1720,47 +1752,10 @@ Parser::functionDef(HandlePropertyName funName, FunctionType type, FunctionSynta
outertc->sc->setFunIsHeavyweight();
}
JSOp op = JSOP_NOP;
if (kind == Expression) {
op = JSOP_LAMBDA;
} else {
if (!bodyLevel) {
/*
* Extension: in non-strict mode code, a function statement not at
* the top level of a function body or whole program, e.g., in a
* compound statement such as the "then" part of an "if" statement,
* binds a closure only if control reaches that sub-statement.
*/
JS_ASSERT(outertc->sc->strictModeState != StrictMode::STRICT);
op = JSOP_DEFFUN;
outertc->sc->setFunMightAliasLocals();
outertc->sc->setFunHasExtensibleScope();
outertc->sc->setFunIsHeavyweight();
/*
* Instead of setting bindingsAccessedDynamically, which would be
* overly conservative, remember the names of all function
* statements and mark any bindings with the same as aliased at the
* end of functionBody.
*/
if (!outertc->funcStmts) {
outertc->funcStmts = context->new_<FuncStmtSet>(context);
if (!outertc->funcStmts || !outertc->funcStmts->init())
return NULL;
}
if (!outertc->funcStmts->put(funName))
return NULL;
}
}
pn->pn_funbox = funbox;
pn->setOp(op);
pn->pn_body->append(body);
pn->pn_body->pn_pos = body->pn_pos;
JS_ASSERT_IF(!outertc->sc->inFunction() && bodyLevel && kind == Statement,
pn->pn_cookie.isFree());
pn->pn_blockid = outertc->blockid();
if (!LeaveFunction(pn, this, funName, kind))

View File

@ -0,0 +1,20 @@
function f(x) {
function x() {}
arguments[0] = 42;
return x;
}
assertEq(f(0), 42);
function g(x) {
function x() {}
assertEq(arguments[0], x);
}
g(0);
var caught = false;
try {
(function h(x) { function x() {} }).blah.baz;
} catch (e) {
caught = true;
}
assertEq(caught, true);

View File

@ -376,7 +376,7 @@ js_DumpPC(JSContext *cx)
return ok;
}
JSBool
JS_FRIEND_API(JSBool)
js_DumpScript(JSContext *cx, JSScript *script_)
{
Sprinter sprinter(cx);
@ -4819,7 +4819,7 @@ Decompile(SprintStack *ss, jsbytecode *pc, int nb)
* syntactically appear.
*/
jsbytecode *nextpc = pc + JSOP_LAMBDA_LENGTH;
LOCAL_ASSERT(*nextpc == JSOP_SETLOCAL || *nextpc == JSOP_SETALIASEDVAR);
LOCAL_ASSERT(*nextpc == JSOP_SETLOCAL || *nextpc == JSOP_SETALIASEDVAR || *nextpc == JSOP_SETARG);
nextpc += js_CodeSpec[*nextpc].length;
LOCAL_ASSERT(*nextpc == JSOP_POP);
nextpc += JSOP_POP_LENGTH;