Bug 963641 - Remove PNK_MUTATEPROTO, and just make JSOP_MUTATEPROTO be generated for the PNK_INITPROP+__proto__ combination. r=jorendorff
authorJeff Walden <jwalden@mit.edu>
Fri, 24 Jan 2014 19:54:53 -0800
changeset 181917 1e170276a033e726e0f7e43e1967832c67d79133
parent 181916 513791fc2754aa6d30a220986acaa4c93d592ad1
child 181918 78d5cf8a1ac62b46c48e763baad3414eabaed5eb
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs963641
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 963641 - Remove PNK_MUTATEPROTO, and just make JSOP_MUTATEPROTO be generated for the PNK_INITPROP+__proto__ combination. r=jorendorff
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/FullParseHandler.h
js/src/frontend/ParseNode.h
js/src/frontend/Parser.cpp
js/src/tests/ecma_5/extensions/destructuring-__proto__-shorthand-assignment-before-var.js
js/src/tests/ecma_5/extensions/destructuring-__proto__-shorthand-assignment.js
js/src/tests/ecma_5/extensions/destructuring-__proto__-target-assignment.js
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2903,39 +2903,38 @@ EmitDestructuringDecl(ExclusiveContext *
     if (!BindNameToSlot(cx, bce, pn))
         return false;
 
     JS_ASSERT(!pn->isOp(JSOP_CALLEE));
     return MaybeEmitVarDecl(cx, bce, prologOp, pn, nullptr);
 }
 
 static bool
-EmitDestructuringDecls(ExclusiveContext *cx, BytecodeEmitter *bce, JSOp prologOp, ParseNode *pn)
-{
-    ParseNode *pn2, *pn3;
-    DestructuringDeclEmitter emitter;
-
-    if (pn->isKind(PNK_ARRAY)) {
-        for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
-            if (pn2->isKind(PNK_ELISION))
+EmitDestructuringDecls(ExclusiveContext *cx, BytecodeEmitter *bce, JSOp prologOp,
+                       ParseNode *pattern)
+{
+    if (pattern->isKind(PNK_ARRAY)) {
+        for (ParseNode *element = pattern->pn_head; element; element = element->pn_next) {
+            if (element->isKind(PNK_ELISION))
                 continue;
-            emitter = (pn2->isKind(PNK_NAME))
-                      ? EmitDestructuringDecl
-                      : EmitDestructuringDecls;
-            if (!emitter(cx, bce, prologOp, pn2))
+            DestructuringDeclEmitter emitter =
+                element->isKind(PNK_NAME) ? EmitDestructuringDecl : EmitDestructuringDecls;
+            if (!emitter(cx, bce, prologOp, element))
                 return false;
         }
-    } else {
-        JS_ASSERT(pn->isKind(PNK_OBJECT));
-        for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
-            pn3 = pn2->pn_right;
-            emitter = pn3->isKind(PNK_NAME) ? EmitDestructuringDecl : EmitDestructuringDecls;
-            if (!emitter(cx, bce, prologOp, pn3))
-                return false;
-        }
+        return true;
+    }
+
+    MOZ_ASSERT(pattern->isKind(PNK_OBJECT));
+    for (ParseNode *member = pattern->pn_head; member; member = member->pn_next) {
+        ParseNode *target = member->pn_right;
+        DestructuringDeclEmitter emitter =
+            target->isKind(PNK_NAME) ? EmitDestructuringDecl : EmitDestructuringDecls;
+        if (!emitter(cx, bce, prologOp, target))
+            return false;
     }
     return true;
 }
 
 static bool
 EmitDestructuringOpsHelper(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn,
                            VarEmitOption emitOption);
 
@@ -3122,35 +3121,39 @@ EmitDestructuringOpsHelper(ExclusiveCont
         doElemOp = true;
         if (pn->isKind(PNK_ARRAY)) {
             if (!EmitNumberOp(cx, index, bce))
                 return false;
             pn3 = pn2;
         } else {
             JS_ASSERT(pn->isKind(PNK_OBJECT));
             JS_ASSERT(pn2->isKind(PNK_COLON));
-            pn3 = pn2->pn_left;
-            if (pn3->isKind(PNK_NUMBER)) {
-                if (!EmitNumberOp(cx, pn3->pn_dval, bce))
+
+            ParseNode *key = pn2->pn_left;
+            if (key->isKind(PNK_NUMBER)) {
+                if (!EmitNumberOp(cx, key->pn_dval, bce))
                     return false;
             } else {
+                MOZ_ASSERT(key->isKind(PNK_STRING) || key->isKind(PNK_NAME));
+                PropertyName *name = key->pn_atom->asPropertyName();
+
                 // The parser already checked for atoms representing indexes and
                 // used PNK_NUMBER instead, but also watch for ids which TI treats
-                // as indexes for simpliciation of downstream analysis.
-                JS_ASSERT(pn3->isKind(PNK_STRING) || pn3->isKind(PNK_NAME));
-                jsid id = NameToId(pn3->pn_atom->asPropertyName());
+                // as indexes for simplification of downstream analysis.
+                jsid id = NameToId(name);
                 if (id != types::IdToTypeId(id)) {
-                    if (!EmitTree(cx, bce, pn3))
+                    if (!EmitTree(cx, bce, key))
                         return false;
                 } else {
-                    if (!EmitAtomOp(cx, pn3, JSOP_GETPROP, bce))
+                    if (!EmitAtomOp(cx, name, JSOP_GETPROP, bce))
                         return false;
                     doElemOp = false;
                 }
             }
+
             pn3 = pn2->pn_right;
         }
 
         if (doElemOp) {
             /*
              * Ok, get the value of the matching property name.  This leaves
              * that value on top of the value being destructured, so the stack
              * is one deeper than when we started.
@@ -5909,26 +5912,16 @@ EmitObject(ExclusiveContext *cx, Bytecod
     if (bce->script->compileAndGo()) {
         gc::AllocKind kind = GuessObjectGCKind(pn->pn_count);
         obj = NewBuiltinClassInstance(cx, &JSObject::class_, kind);
         if (!obj)
             return false;
     }
 
     for (ParseNode *pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
-        /* Handle __proto__ specially because it's not binary. */
-        if (pn2->isKind(PNK_MUTATEPROTO)) {
-            if (!EmitTree(cx, bce, pn2->pn_kid))
-                return false;
-            obj = nullptr;
-            if (!Emit1(cx, bce, JSOP_MUTATEPROTO))
-                return false;
-            continue;
-        }
-
         /* Emit an index for t[2] for later consumption by JSOP_INITELEM. */
         ParseNode *pn3 = pn2->pn_left;
         bool isIndex = false;
         if (pn3->isKind(PNK_NUMBER)) {
             if (!EmitNumberOp(cx, pn3->pn_dval, bce))
                 return false;
             isIndex = true;
         } else {
@@ -5963,23 +5956,32 @@ EmitObject(ExclusiveContext *cx, Bytecod
               case JSOP_INITPROP_GETTER: op = JSOP_INITELEM_GETTER; break;
               case JSOP_INITPROP_SETTER: op = JSOP_INITELEM_SETTER; break;
               default: MOZ_ASSUME_UNREACHABLE("Invalid op");
             }
             if (Emit1(cx, bce, op) < 0)
                 return false;
         } else {
             JS_ASSERT(pn3->isKind(PNK_NAME) || pn3->isKind(PNK_STRING));
+
+            // If we have { __proto__: expr }, implement prototype mutation.
+            if (op == JSOP_INITPROP && pn3->pn_atom == cx->names().proto) {
+                obj = nullptr;
+                if (Emit1(cx, bce, JSOP_MUTATEPROTO) < 0)
+                    return false;
+                continue;
+            }
+
             jsatomid index;
             if (!bce->makeAtomIndex(pn3->pn_atom, &index))
                 return false;
 
-            MOZ_ASSERT((op == JSOP_INITPROP_GETTER || op == JSOP_INITPROP_SETTER) ||
-                        pn3->pn_atom != cx->names().proto,
-                       "__proto__ shouldn't have been generated as an initprop");
+            MOZ_ASSERT(op == JSOP_INITPROP ||
+                       op == JSOP_INITPROP_GETTER ||
+                       op == JSOP_INITPROP_SETTER);
 
             if (obj) {
                 JS_ASSERT(!obj->inDictionaryMode());
                 Rooted<jsid> id(cx, AtomToId(pn3->pn_atom));
                 RootedValue undefinedValue(cx, UndefinedValue());
                 if (!DefineNativeProperty(cx, obj, id, undefinedValue, nullptr,
                                           nullptr, JSPROP_ENUMERATE, 0, 0))
                 {
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -230,24 +230,16 @@ class FullParseHandler
     ParseNode *newObjectLiteral(uint32_t begin) {
         ParseNode *literal = new_<ListNode>(PNK_OBJECT, TokenPos(begin, begin + 1));
         // Later in this stack: remove dependency on this opcode.
         if (literal)
             literal->setOp(JSOP_NEWINIT);
         return literal;
     }
 
-    bool addPrototypeMutation(ParseNode *literal, uint32_t begin, ParseNode *expr) {
-        ParseNode *mutation = newUnary(PNK_MUTATEPROTO, JSOP_NOP, begin, expr);
-        if (!mutation)
-            return false;
-        literal->append(mutation);
-        return true;
-    }
-
     bool addPropertyDefinition(ParseNode *literal, ParseNode *name, ParseNode *expr) {
         ParseNode *propdef = newBinary(PNK_COLON, name, expr, JSOP_INITPROP);
         if (!propdef)
             return false;
         literal->append(propdef);
         return true;
     }
 
--- a/js/src/frontend/ParseNode.h
+++ b/js/src/frontend/ParseNode.h
@@ -134,17 +134,16 @@ class UpvarCookie
     F(EXPORT_SPEC) \
     F(EXPORT_BATCH_SPEC) \
     F(SEQ) \
     F(FORIN) \
     F(FOROF) \
     F(FORHEAD) \
     F(ARGSBODY) \
     F(SPREAD) \
-    F(MUTATEPROTO) \
     \
     /* Unary operators. */ \
     F(TYPEOF) \
     F(VOID) \
     F(NOT) \
     F(BITNOT) \
     \
     /* \
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3094,42 +3094,41 @@ Parser<FullParseHandler>::checkDestructu
                     }
                 }
                 if (!ok)
                     return false;
             }
         }
     } else {
         JS_ASSERT(left->isKind(PNK_OBJECT));
-        for (ParseNode *pair = left->pn_head; pair; pair = pair->pn_next) {
-            JS_ASSERT(pair->isKind(PNK_COLON));
-            ParseNode *pn = pair->pn_right;
-
-            if (pn->isKind(PNK_ARRAY) || pn->isKind(PNK_OBJECT)) {
-                ok = checkDestructuring(data, pn, false);
+        for (ParseNode *member = left->pn_head; member; member = member->pn_next) {
+            MOZ_ASSERT(member->isKind(PNK_COLON));
+            ParseNode *expr = member->pn_right;
+
+            if (expr->isKind(PNK_ARRAY) || expr->isKind(PNK_OBJECT)) {
+                ok = checkDestructuring(data, expr, false);
             } else if (data) {
-                if (!pn->isKind(PNK_NAME)) {
-                    report(ParseError, false, pn, JSMSG_NO_VARIABLE_NAME);
+                if (!expr->isKind(PNK_NAME)) {
+                    report(ParseError, false, expr, JSMSG_NO_VARIABLE_NAME);
                     return false;
                 }
-                ok = bindDestructuringVar(data, pn);
+                ok = bindDestructuringVar(data, expr);
             } else {
                 /*
-                 * If right and left point to the same node, then this is
-                 * destructuring shorthand ({x} = ...). In that case,
-                 * identifierName was not used to parse 'x' so 'x' has not been
-                 * officially linked to its def or registered in lexdeps. Do
-                 * that now.
+                 * If this is a destructuring shorthand ({x} = ...), then
+                 * identifierName wasn't used to parse |x|.  As a result, |x|
+                 * hasn't been officially linked to its def or registered in
+                 * lexdeps.  Do that now.
                  */
-                if (pair->pn_right == pair->pn_left) {
-                    RootedPropertyName name(context, pn->pn_atom->asPropertyName());
-                    if (!noteNameUse(name, pn))
+                if (member->pn_right == member->pn_left) {
+                    RootedPropertyName name(context, expr->pn_atom->asPropertyName());
+                    if (!noteNameUse(name, expr))
                         return false;
                 }
-                ok = checkAndMarkAsAssignmentLhs(pn, KeyedDestructuringAssignment);
+                ok = checkAndMarkAsAssignmentLhs(expr, KeyedDestructuringAssignment);
             }
             if (!ok)
                 return false;
         }
     }
 
     /*
      * The catch/finally handler implementation in the interpreter assumes
@@ -3654,18 +3653,19 @@ Parser<FullParseHandler>::letStatement()
 
     /* Check for a let statement or let expression. */
     ParseNode *pn;
     if (tokenStream.peekToken() == TOK_LP) {
         pn = letBlock(LetStatement);
         JS_ASSERT_IF(pn, pn->isKind(PNK_LET) || pn->isKind(PNK_SEMI));
         JS_ASSERT_IF(pn && pn->isKind(PNK_LET) && pn->pn_expr->getOp() != JSOP_POPNV,
                      pn->pn_expr->isOp(JSOP_POPN));
-    } else 
+    } else {
         pn = letDeclaration();
+    }
     return pn;
 }
 
 template <>
 SyntaxParseHandler::Node
 Parser<SyntaxParseHandler>::letStatement()
 {
     JS_ALWAYS_FALSE(abortIfSyntaxParser());
@@ -6802,17 +6802,16 @@ Parser<ParseHandler>::objectLiteral()
     RootedAtom atom(context);
     for (;;) {
         TokenKind ltok = tokenStream.getToken(TokenStream::KeywordIsName);
         if (ltok == TOK_RC)
             break;
 
         JSOp op = JSOP_INITPROP;
         Node propname;
-        uint32_t begin;
         switch (ltok) {
           case TOK_NUMBER:
             atom = DoubleToAtom(context, tokenStream.currentToken().number());
             if (!atom)
                 return null();
             propname = newNumber(tokenStream.currentToken());
             break;
 
@@ -6821,20 +6820,16 @@ Parser<ParseHandler>::objectLiteral()
             if (atom == context->names().get) {
                 op = JSOP_INITPROP_GETTER;
             } else if (atom == context->names().set) {
                 op = JSOP_INITPROP_SETTER;
             } else {
                 propname = handler.newIdentifier(atom, pos());
                 if (!propname)
                     return null();
-                if (atom == context->names().proto) {
-                    begin = pos().begin;
-                    op = JSOP_MUTATEPROTO;
-                }
                 break;
             }
 
             // We have parsed |get| or |set|. Look for an accessor property
             // name next.
             TokenKind tt = tokenStream.getToken(TokenStream::KeywordIsName);
             if (tt == TOK_NAME) {
                 atom = tokenStream.currentName();
@@ -6893,41 +6888,37 @@ Parser<ParseHandler>::objectLiteral()
             break;
           }
 
           default:
             report(ParseError, false, null(), JSMSG_BAD_PROP_ID);
             return null();
         }
 
-        if (op == JSOP_INITPROP || op == JSOP_MUTATEPROTO) {
+        if (op == JSOP_INITPROP) {
             TokenKind tt = tokenStream.getToken();
             Node propexpr;
             if (tt == TOK_COLON) {
                 propexpr = assignExpr();
                 if (!propexpr)
                     return null();
 
                 if (foldConstants && !FoldConstants(context, &propexpr, this))
                     return null();
 
                 /*
                  * Treat initializers which mutate __proto__ as non-constant,
                  * so that we can later assume singleton objects delegate to
                  * the default Object.prototype.
                  */
-                if (!handler.isConstant(propexpr) || op == JSOP_MUTATEPROTO)
+                if (!handler.isConstant(propexpr) || atom == context->names().proto)
                     handler.setListFlag(literal, PNX_NONCONST);
 
-                if (op == JSOP_MUTATEPROTO
-                    ? !handler.addPrototypeMutation(literal, begin, propexpr)
-                    : !handler.addPropertyDefinition(literal, propname, propexpr))
-                {
+                if (!handler.addPropertyDefinition(literal, propname, propexpr))
                     return null();
-                }
             }
 #if JS_HAS_DESTRUCTURING_SHORTHAND
             else if (ltok == TOK_NAME && (tt == TOK_COMMA || tt == TOK_RC)) {
                 /*
                  * Support, e.g., |var {x, y} = o| as destructuring shorthand
                  * for |var {x: x, y: y} = o|, per proposed JS2/ES4 for JS1.8.
                  */
                 if (!abortIfSyntaxParser())
@@ -6963,17 +6954,17 @@ Parser<ParseHandler>::objectLiteral()
 
         /*
          * Check for duplicate property names.  Duplicate data properties
          * only conflict in strict mode.  Duplicate getter or duplicate
          * setter halves always conflict.  A data property conflicts with
          * any part of an accessor property.
          */
         AssignmentType assignType;
-        if (op == JSOP_INITPROP || op == JSOP_MUTATEPROTO)
+        if (op == JSOP_INITPROP)
             assignType = VALUE;
         else if (op == JSOP_INITPROP_GETTER)
             assignType = GET;
         else if (op == JSOP_INITPROP_SETTER)
             assignType = SET;
         else
             MOZ_ASSUME_UNREACHABLE("bad opcode in object initializer");
 
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/extensions/destructuring-__proto__-shorthand-assignment-before-var.js
@@ -0,0 +1,49 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = 'destructuring-__proto__-shorthand-assignment-before-var.js';
+var BUGNUMBER = 963641;
+var summary = "{ __proto__ } should work as a destructuring assignment pattern";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function objectWithProtoProperty(v)
+{
+  var obj = {};
+  return Object.defineProperty(obj, "__proto__",
+                               {
+                                 enumerable: true,
+                                 configurable: true,
+                                 writable: true,
+                                 value: v
+                               });
+}
+
+({ __proto__ } = objectWithProtoProperty(17));
+assertEq(__proto__, 17);
+
+var { __proto__ } = objectWithProtoProperty(42);
+assertEq(__proto__, 42);
+
+function nested()
+{
+  ({ __proto__ } = objectWithProtoProperty(undefined));
+  assertEq(__proto__, undefined);
+
+  var { __proto__ } = objectWithProtoProperty("fnord");
+  assertEq(__proto__, "fnord");
+}
+nested();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/extensions/destructuring-__proto__-shorthand-assignment.js
@@ -0,0 +1,49 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = 'destructuring-__proto__-shorthand-assignment.js';
+var BUGNUMBER = 963641;
+var summary = "{ __proto__ } should work as a destructuring assignment pattern";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function objectWithProtoProperty(v)
+{
+  var obj = {};
+  return Object.defineProperty(obj, "__proto__",
+                               {
+                                 enumerable: true,
+                                 configurable: true,
+                                 writable: true,
+                                 value: v
+                               });
+}
+
+var { __proto__ } = objectWithProtoProperty(42);
+assertEq(__proto__, 42);
+
+({ __proto__ } = objectWithProtoProperty(17));
+assertEq(__proto__, 17);
+
+function nested()
+{
+  var { __proto__ } = objectWithProtoProperty("fnord");
+  assertEq(__proto__, "fnord");
+
+  ({ __proto__ } = objectWithProtoProperty(undefined));
+  assertEq(__proto__, undefined);
+}
+nested();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/extensions/destructuring-__proto__-target-assignment.js
@@ -0,0 +1,50 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = 'destructuring-__proto__-target--assignment.js';
+var BUGNUMBER = 963641;
+var summary =
+  "{ __proto__: target } should work as a destructuring assignment pattern";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function objectWithProtoProperty(v)
+{
+  var obj = {};
+  return Object.defineProperty(obj, "__proto__",
+                               {
+                                 enumerable: true,
+                                 configurable: true,
+                                 writable: true,
+                                 value: v
+                               });
+}
+
+var { __proto__: target } = objectWithProtoProperty(null);
+assertEq(target, null);
+
+({ __proto__: target } = objectWithProtoProperty("aacchhorrt"));
+assertEq(target, "aacchhorrt");
+
+function nested()
+{
+  var { __proto__: target } = objectWithProtoProperty(3.141592654);
+  assertEq(target, 3.141592654);
+
+  ({ __proto__: target } = objectWithProtoProperty(-0));
+  assertEq(target, -0);
+}
+nested();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");