diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 9d9d938aca2..67e578f267c 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -1830,18 +1830,16 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) if (pn->pn_op == JSOP_QNAMEPART) return JS_TRUE; - /* - * We can't optimize if we are compiling a with statement and its body, - * or we're in a catch block whose exception variable has the same name - * as this node. FIXME: we should be able to optimize catch vars to be - * block-locals. - */ tc = &cg->treeContext; atom = pn->pn_atom; stmt = js_LexicalLookup(tc, atom, &slot); if (stmt) { - if (stmt->type == STMT_WITH) + /* We can't optimize if we are inside a with statement. */ + if (stmt->type == STMT_WITH) { + JS_ASSERT_IF(tc->flags & TCF_IN_FUNCTION, + tc->flags & TCF_FUN_HEAVYWEIGHT); return JS_TRUE; + } JS_ASSERT(stmt->flags & SIF_SCOPE); JS_ASSERT(slot >= 0); @@ -1867,14 +1865,6 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) return JS_TRUE; } - /* - * We can't optimize if var and closure (a local function not in a larger - * expression and not at top-level within another's body) collide. - * XXX suboptimal: keep track of colliding names and deoptimize only those - */ - if (tc->flags & TCF_FUN_CLOSURE_VS_VAR) - return JS_TRUE; - if (!(tc->flags & TCF_IN_FUNCTION)) { JSStackFrame *caller; @@ -1989,51 +1979,50 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) return JS_TRUE; } - if (tc->flags & TCF_IN_FUNCTION) { - /* - * We are compiling a function body and may be able to optimize name - * to stack slot. Look for an argument or variable in the function and - * rewrite pn_op and update pn accordingly. - */ - localKind = js_LookupLocal(cx, tc->u.fun, atom, &index); - if (localKind != JSLOCAL_NONE) { - op = PN_OP(pn); - if (localKind == JSLOCAL_ARG) { - switch (op) { - case JSOP_NAME: op = JSOP_GETARG; break; - case JSOP_SETNAME: op = JSOP_SETARG; break; - case JSOP_INCNAME: op = JSOP_INCARG; break; - case JSOP_NAMEINC: op = JSOP_ARGINC; break; - case JSOP_DECNAME: op = JSOP_DECARG; break; - case JSOP_NAMEDEC: op = JSOP_ARGDEC; break; - case JSOP_FORNAME: op = JSOP_FORARG; break; - case JSOP_DELNAME: op = JSOP_FALSE; break; - default: JS_NOT_REACHED("arg"); - } - pn->pn_const = JS_FALSE; - } else { - JS_ASSERT(localKind == JSLOCAL_VAR || - localKind == JSLOCAL_CONST); - switch (op) { - case JSOP_NAME: op = JSOP_GETLOCAL; break; - case JSOP_SETNAME: op = JSOP_SETLOCAL; break; - case JSOP_SETCONST: op = JSOP_SETLOCAL; break; - case JSOP_INCNAME: op = JSOP_INCLOCAL; break; - case JSOP_NAMEINC: op = JSOP_LOCALINC; break; - case JSOP_DECNAME: op = JSOP_DECLOCAL; break; - case JSOP_NAMEDEC: op = JSOP_LOCALDEC; break; - case JSOP_FORNAME: op = JSOP_FORLOCAL; break; - case JSOP_DELNAME: op = JSOP_FALSE; break; - default: JS_NOT_REACHED("local"); - } - pn->pn_const = (localKind == JSLOCAL_CONST); + /* + * We are compiling a function body and may be able to optimize name to + * stack slot. Look for an argument or variable in the function and + * rewrite pn_op and update pn accordingly. + */ + JS_ASSERT(tc->flags & TCF_IN_FUNCTION); + localKind = js_LookupLocal(cx, tc->u.fun, atom, &index); + if (localKind != JSLOCAL_NONE) { + op = PN_OP(pn); + if (localKind == JSLOCAL_ARG) { + switch (op) { + case JSOP_NAME: op = JSOP_GETARG; break; + case JSOP_SETNAME: op = JSOP_SETARG; break; + case JSOP_INCNAME: op = JSOP_INCARG; break; + case JSOP_NAMEINC: op = JSOP_ARGINC; break; + case JSOP_DECNAME: op = JSOP_DECARG; break; + case JSOP_NAMEDEC: op = JSOP_ARGDEC; break; + case JSOP_FORNAME: op = JSOP_FORARG; break; + case JSOP_DELNAME: op = JSOP_FALSE; break; + default: JS_NOT_REACHED("arg"); } - pn->pn_op = op; - pn->pn_slot = index; - return JS_TRUE; + pn->pn_const = JS_FALSE; + } else { + JS_ASSERT(localKind == JSLOCAL_VAR || + localKind == JSLOCAL_CONST); + switch (op) { + case JSOP_NAME: op = JSOP_GETLOCAL; break; + case JSOP_SETNAME: op = JSOP_SETLOCAL; break; + case JSOP_SETCONST: op = JSOP_SETLOCAL; break; + case JSOP_INCNAME: op = JSOP_INCLOCAL; break; + case JSOP_NAMEINC: op = JSOP_LOCALINC; break; + case JSOP_DECNAME: op = JSOP_DECLOCAL; break; + case JSOP_NAMEDEC: op = JSOP_LOCALDEC; break; + case JSOP_FORNAME: op = JSOP_FORLOCAL; break; + case JSOP_DELNAME: op = JSOP_FALSE; break; + default: JS_NOT_REACHED("local"); + } + pn->pn_const = (localKind == JSLOCAL_CONST); } - tc->flags |= TCF_FUN_USES_NONLOCALS; + pn->pn_op = op; + pn->pn_slot = index; + return JS_TRUE; } + tc->flags |= TCF_FUN_USES_NONLOCALS; arguments_check: /* diff --git a/js/src/jsemit.h b/js/src/jsemit.h index 4e39f6cc91c..3cf7d45edd8 100644 --- a/js/src/jsemit.h +++ b/js/src/jsemit.h @@ -188,7 +188,8 @@ struct JSTreeContext { /* tree context for semantic checks */ #define TCF_RETURN_EXPR 0x02 /* function has 'return expr;' */ #define TCF_RETURN_VOID 0x04 /* function has 'return;' */ #define TCF_IN_FOR_INIT 0x08 /* parsing init expr of for; exclude 'in' */ -#define TCF_FUN_CLOSURE_VS_VAR 0x10 /* function and var with same name */ +#define TCF_NO_SCRIPT_RVAL 0x10 /* API caller does not want result value + from global script */ #define TCF_FUN_USES_NONLOCALS 0x20 /* function refers to non-local names */ #define TCF_FUN_HEAVYWEIGHT 0x40 /* function needs Call object per call */ #define TCF_FUN_IS_GENERATOR 0x80 /* parsed yield statement in function */ @@ -198,8 +199,6 @@ struct JSTreeContext { /* tree context for semantic checks */ #define TCF_COMPILE_N_GO 0x800 /* compiler-and-go mode of script, can optimize name references based on scope chain */ -#define TCF_NO_SCRIPT_RVAL 0x1000 /* API caller does not want result value - from global script */ /* * Flags to propagate out of the blocks. */ @@ -210,8 +209,7 @@ struct JSTreeContext { /* tree context for semantic checks */ */ #define TCF_FUN_FLAGS (TCF_FUN_IS_GENERATOR | \ TCF_FUN_HEAVYWEIGHT | \ - TCF_FUN_USES_NONLOCALS | \ - TCF_FUN_CLOSURE_VS_VAR) + TCF_FUN_USES_NONLOCALS) /* * Flags field, not stored in JSTreeContext.flags, for passing staticDepth diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 36761abe7cc..91baddc8740 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -884,7 +884,12 @@ call_resolve(JSContext *cx, JSObject *obj, jsval idval, uintN flags, localKind = js_LookupLocal(cx, fun, JSID_TO_ATOM(id), &slot); if (localKind != JSLOCAL_NONE) { JS_ASSERT((uint16) slot == slot); - attrs = JSPROP_PERMANENT | JSPROP_SHARED; + + /* + * We follow 10.2.3 of ECMA 262 v3 and make argument and variable + * properties of the Call objects enumerable. + */ + attrs = JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_SHARED; if (localKind == JSLOCAL_ARG) { JS_ASSERT(slot < fun->nargs); getter = js_GetCallArg; diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 5b5cc9119d7..d3d928c50d4 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -5714,6 +5714,13 @@ js_Interpret(JSContext *cx) END_CASE(JSOP_DEFVAR) BEGIN_CASE(JSOP_DEFFUN) + { + JSPropertyOp getter, setter; + bool doSet; + JSObject *pobj; + JSProperty *prop; + uint32 old; + /* * A top-level function defined in Global or Eval code (see * ECMA-262 Ed. 3), or else a SpiderMonkey extension: a named @@ -5752,7 +5759,7 @@ js_Interpret(JSContext *cx) * paths from here must flow through the "Restore fp->scopeChain" * code below the OBJ_DEFINE_PROPERTY call. */ - MUST_FLOW_THROUGH("restore"); + MUST_FLOW_THROUGH("restore_scope"); fp->scopeChain = obj; rval = OBJECT_TO_JSVAL(obj); @@ -5769,10 +5776,17 @@ js_Interpret(JSContext *cx) * and setters do not need a slot, their value is stored elsewhere * in the property itself, not in obj slots. */ + setter = getter = JS_PropertyStub; flags = JSFUN_GSFLAG2ATTR(fun->flags); if (flags) { + /* Function cannot be both getter a setter. */ + JS_ASSERT(flags == JSPROP_GETTER || flags == JSPROP_SETTER); attrs |= flags | JSPROP_SHARED; rval = JSVAL_VOID; + if (flags == JSPROP_GETTER) + getter = JS_EXTENSION (JSPropertyOp) obj; + else + setter = JS_EXTENSION (JSPropertyOp) obj; } /* @@ -5791,33 +5805,54 @@ js_Interpret(JSContext *cx) * as well as multiple HTML script tags. */ id = ATOM_TO_JSID(fun->atom); - ok = js_CheckRedeclaration(cx, parent, id, attrs, NULL, NULL); - if (ok) { - if (attrs == JSPROP_ENUMERATE) { - JS_ASSERT(fp->flags & JSFRAME_EVAL); - ok = OBJ_SET_PROPERTY(cx, parent, id, &rval); - } else { - JS_ASSERT(attrs & JSPROP_PERMANENT); + prop = NULL; + ok = js_CheckRedeclaration(cx, parent, id, attrs, &pobj, &prop); + if (!ok) + goto restore_scope; - ok = OBJ_DEFINE_PROPERTY(cx, parent, id, rval, - (flags & JSPROP_GETTER) - ? JS_EXTENSION (JSPropertyOp) obj - : JS_PropertyStub, - (flags & JSPROP_SETTER) - ? JS_EXTENSION (JSPropertyOp) obj - : JS_PropertyStub, - attrs, - NULL); + /* + * We deviate from 10.1.2 in ECMA 262 v3 and under eval use for + * function declarations OBJ_SET_PROPERTY, not OBJ_DEFINE_PROPERTY, + * to preserve the JSOP_PERMANENT attribute of existing properties + * and make sure that such properties cannot be deleted. + * + * We also use OBJ_SET_PROPERTY for the existing properties of + * Call objects with matching attributes to preserve the native + * getters and setters that store the value of the property in the + * interpreter frame, see bug 467495. + */ + doSet = (attrs == JSPROP_ENUMERATE); + JS_ASSERT_IF(doSet, fp->flags & JSFRAME_EVAL); + if (prop) { + if (parent == pobj && + OBJ_GET_CLASS(cx, parent) == &js_CallClass && + (old = ((JSScopeProperty *) prop)->attrs, + !(old & (JSPROP_GETTER|JSPROP_SETTER)) && + (old & (JSPROP_ENUMERATE|JSPROP_PERMANENT)) == attrs)) { + /* + * js_CheckRedeclaration must reject attempts to add a + * getter or setter to an existing property without a + * getter or setter. + */ + JS_ASSERT(!(attrs & ~(JSPROP_ENUMERATE|JSPROP_PERMANENT))); + JS_ASSERT(!(old & JSPROP_READONLY)); + doSet = JS_TRUE; } + OBJ_DROP_PROPERTY(cx, pobj, prop); } + ok = doSet + ? OBJ_SET_PROPERTY(cx, parent, id, &rval) + : OBJ_DEFINE_PROPERTY(cx, parent, id, rval, getter, setter, + attrs, NULL); + restore_scope: /* Restore fp->scopeChain now that obj is defined in fp->varobj. */ - MUST_FLOW_LABEL(restore) fp->scopeChain = obj2; if (!ok) { cx->weakRoots.newborn[GCX_OBJECT] = NULL; goto error; } + } END_CASE(JSOP_DEFFUN) BEGIN_CASE(JSOP_DEFLOCALFUN) diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 5addc8f744e..6cf1630764a 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -1167,8 +1167,6 @@ FunctionDef(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, return NULL; } } - if (!AT_TOP_LEVEL(tc) && prevop == JSOP_DEFVAR) - tc->flags |= TCF_FUN_CLOSURE_VS_VAR; } else { ale = js_IndexAtom(cx, funAtom, &tc->decls); if (!ale) @@ -1645,8 +1643,6 @@ BindVarOrConst(JSContext *cx, BindData *data, JSAtom *atom, JSTreeContext *tc) return JS_FALSE; } } - if (op == JSOP_DEFVAR && prevop == JSOP_DEFFUN) - tc->flags |= TCF_FUN_CLOSURE_VS_VAR; } if (!ale) { ale = js_IndexAtom(cx, atom, &tc->decls);