Bug 622321 - While { x: 1, x: 1 } is a syntax error only in strict mode, any other name collision between property assignments in an object literal is a syntax error regardless whether the literal is in strict mode code or not. r=dmandelin
authorJeff Walden <jwalden@mit.edu>
Sat, 01 Jan 2011 02:41:00 -0600
changeset 60213 220ced5ad4fb7452861bb48f9214edd4df7a6e7a
parent 60212 daaf6a3b8782d1a17ecfe5556c576651f71f7130
child 60214 b33fddc6066e0428d1f18f1694f557d0e59fd348
push id17881
push usercleary@mozilla.com
push dateFri, 07 Jan 2011 19:57:21 +0000
treeherdermozilla-central@54576be62860 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmandelin
bugs622321
milestone2.0b9pre
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 622321 - While { x: 1, x: 1 } is a syntax error only in strict mode, any other name collision between property assignments in an object literal is a syntax error regardless whether the literal is in strict mode code or not. r=dmandelin
browser/components/safebrowsing/content/sb-loader.js
js/src/jsparse.cpp
js/src/tests/ecma_5/strict/11.1.5.js
--- a/browser/components/safebrowsing/content/sb-loader.js
+++ b/browser/components/safebrowsing/content/sb-loader.js
@@ -30,18 +30,16 @@
 # decision by deleting the provisions above and replace them with the notice
 # and other provisions required by the GPL or the LGPL. If you do not delete
 # the provisions above, a recipient may use your version of this file under
 # the terms of any one of the MPL, the GPL or the LGPL.
 #
 # ***** END LICENSE BLOCK *****
 
 var safebrowsing = {
-  appContext: null,
-
   startup: function() {
     setTimeout(function() {
       safebrowsing.deferredStartup();
     }, 2000);
     window.removeEventListener("load", safebrowsing.startup, false);
   },
 
   deferredStartup: function() {
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -8418,23 +8418,25 @@ Parser::primaryExpr(TokenKind tt, JSBool
       }
 
       case TOK_LC:
       {
         JSBool afterComma;
         JSParseNode *pnval;
 
         /*
-         * A map from property names we've seen thus far to bit masks.
-         * (We use ALE_INDEX/ALE_SET_INDEX).  An atom's mask includes
-         * JSPROP_SETTER if we've seen a setter for it, JSPROP_GETTER
-         * if we've seen as getter, and both of those if we've just
-         * seen an ordinary value.
+         * A map from property names we've seen thus far to a mask of property
+         * assignment types, stored and retrieved with ALE_SET_INDEX/ALE_INDEX.
          */
         JSAutoAtomList seen(tc->parser);
+        enum AssignmentType {
+            GET     = 0x1,
+            SET     = 0x2,
+            VALUE   = 0x4 | GET | SET
+        };
 
         pn = ListNode::create(tc);
         if (!pn)
             return NULL;
         pn->pn_type = TOK_RC;
         pn->pn_op = JSOP_NEWINIT;
         pn->makeEmpty();
 
@@ -8443,26 +8445,18 @@ Parser::primaryExpr(TokenKind tt, JSBool
             JSAtom *atom;
             tt = tokenStream.getToken(TSF_KEYWORD_IS_NAME);
             switch (tt) {
               case TOK_NUMBER:
                 pn3 = NullaryNode::create(tc);
                 if (!pn3)
                     return NULL;
                 pn3->pn_dval = tokenStream.currentToken().t_dval;
-                if (tc->needStrictChecks()) {
-                    /*
-                     * Use string-valued atoms for detecting duplicate
-                     * properties so that 1 and "1" properly collide.
-                     */
-                    if (!js_ValueToAtom(context, DoubleValue(pn3->pn_dval), &atom))
-                        return NULL;
-                } else {
-                    atom = NULL; /* for the compiler */
-                }
+                if (!js_ValueToAtom(context, DoubleValue(pn3->pn_dval), &atom))
+                    return NULL;
                 break;
               case TOK_NAME:
                 {
                     atom = tokenStream.currentToken().t_atom;
                     if (atom == context->runtime->atomState.getAtom)
                         op = JSOP_GETTER;
                     else if (atom == context->runtime->atomState.setAtom)
                         op = JSOP_SETTER;
@@ -8475,26 +8469,18 @@ Parser::primaryExpr(TokenKind tt, JSBool
                         pn3 = NameNode::create(atom, tc);
                         if (!pn3)
                             return NULL;
                     } else if (tt == TOK_NUMBER) {
                         pn3 = NullaryNode::create(tc);
                         if (!pn3)
                             return NULL;
                         pn3->pn_dval = tokenStream.currentToken().t_dval;
-                        if (tc->needStrictChecks()) {
-                            /*
-                             * Use string-valued atoms for detecting duplicate
-                             * properties so that 1 and "1" properly collide.
-                             */
-                            if (!js_ValueToAtom(context, DoubleValue(pn3->pn_dval), &atom))
-                                return NULL;
-                        } else {
-                            atom = NULL; /* for the compiler */
-                        }
+                        if (!js_ValueToAtom(context, DoubleValue(pn3->pn_dval), &atom))
+                            return NULL;
                     } else {
                         tokenStream.ungetToken();
                         goto property_name;
                     }
 
                     pn->pn_xflags |= PNX_NONCONST;
 
                     /* NB: Getter function in { get x(){} } is unnamed. */
@@ -8548,50 +8534,59 @@ Parser::primaryExpr(TokenKind tt, JSBool
 
             pn2 = JSParseNode::newBinaryOrAppend(TOK_COLON, op, pn3, pnval, tc);
           skip:
             if (!pn2)
                 return NULL;
             pn->append(pn2);
 
             /*
-             * In strict mode code, check for duplicate property names.  Treat
-             * getters and setters as distinct attributes of each property.  A
-             * plain old value conflicts with a getter or a setter.
+             * 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.
              */
-            if (tc->needStrictChecks()) {
-                unsigned attributesMask;
-                if (op == JSOP_INITPROP) {
-                    attributesMask = JSPROP_GETTER | JSPROP_SETTER;
-                } else if (op == JSOP_GETTER) {
-                    attributesMask = JSPROP_GETTER;
-                } else if (op == JSOP_SETTER) {
-                    attributesMask = JSPROP_SETTER;
-                } else {
-                    JS_NOT_REACHED("bad opcode in object initializer");
-                    attributesMask = 0;
+            AssignmentType assignType;
+            if (op == JSOP_INITPROP) {
+                assignType = VALUE;
+            } else if (op == JSOP_GETTER) {
+                assignType = GET;
+            } else if (op == JSOP_SETTER) {
+                assignType = SET;
+            } else {
+                JS_NOT_REACHED("bad opcode in object initializer");
+                assignType = VALUE; /* try to error early */
+            }
+
+            if (JSAtomListElement *ale = seen.lookup(atom)) {
+                AssignmentType oldAssignType = AssignmentType(ALE_INDEX(ale));
+                if ((oldAssignType & assignType) &&
+                    (oldAssignType != VALUE || assignType != VALUE || tc->needStrictChecks()))
+                {
+                    JSAutoByteString name;
+                    if (!js_AtomToPrintableString(context, atom, &name))
+                        return NULL;
+
+                    uintN flags = (oldAssignType == VALUE &&
+                                   assignType == VALUE &&
+                                   !tc->inStrictMode())
+                                  ? JSREPORT_WARNING
+                                  : JSREPORT_ERROR;
+                    if (!ReportCompileErrorNumber(context, &tokenStream, NULL, flags,
+                                                  JSMSG_DUPLICATE_PROPERTY, name.ptr()))
+                    {
+                        return NULL;
+                    }
                 }
-
-                JSAtomListElement *ale = seen.lookup(atom);
-                if (ale) {
-                    if (ALE_INDEX(ale) & attributesMask) {
-                        JSAutoByteString name;
-                        if (!js_AtomToPrintableString(context, atom, &name) ||
-                            !ReportStrictModeError(context, &tokenStream, tc, NULL,
-                                                   JSMSG_DUPLICATE_PROPERTY, name.ptr())) {
-                            return NULL;
-                        }
-                    }
-                    ALE_SET_INDEX(ale, attributesMask | ALE_INDEX(ale));
-                } else {
-                    ale = seen.add(tc->parser, atom);
-                    if (!ale)
-                        return NULL;
-                    ALE_SET_INDEX(ale, attributesMask);
-                }
+                ALE_SET_INDEX(ale, assignType | oldAssignType);
+            } else {
+                ale = seen.add(tc->parser, atom);
+                if (!ale)
+                    return NULL;
+                ALE_SET_INDEX(ale, assignType);
             }
 
             tt = tokenStream.getToken();
             if (tt == TOK_RC)
                 goto end_obj_init;
             if (tt != TOK_COMMA) {
                 reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_CURLY_AFTER_LIST);
                 return NULL;
--- a/js/src/tests/ecma_5/strict/11.1.5.js
+++ b/js/src/tests/ecma_5/strict/11.1.5.js
@@ -89,57 +89,82 @@ assertEq(testLenientAndStrict('({a:1, b:
                               parseRaisesException(SyntaxError)),
          true);
 
 /*
  * Getters, setters, and value properties should conflict only when
  * appropriate.
  */
 assertEq(testLenientAndStrict('({get x() {}, x:1})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({x:1, get x() {}})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({set x(q) {}, x:1})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({x:1, set x(q) {}})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
+                              parseRaisesException(SyntaxError)),
+         true);
+
+assertEq(testLenientAndStrict('({1:1, set 1(q) {}})',
+                              parseRaisesException(SyntaxError),
+                              parseRaisesException(SyntaxError)),
+         true);
+
+assertEq(testLenientAndStrict('({set 1(q) {}, 1:1})',
+                              parseRaisesException(SyntaxError),
+                              parseRaisesException(SyntaxError)),
+         true);
+
+assertEq(testLenientAndStrict('({"1":1, set 1(q) {}})',
+                              parseRaisesException(SyntaxError),
+                              parseRaisesException(SyntaxError)),
+         true);
+
+assertEq(testLenientAndStrict('({set 1(q) {}, "1":1})',
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({get x() {}, set x(q) {}})',
                               parsesSuccessfully,
                               parsesSuccessfully),
          true);
 
 assertEq(testLenientAndStrict('({set x(q) {}, get x() {}})',
                               parsesSuccessfully,
                               parsesSuccessfully),
          true);
 
 assertEq(testLenientAndStrict('({get x() {}, set x(q) {}, x:1})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({set x(q) {}, get x() {}, x:1})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({get x() {}, get x() {}})',
-                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError),
+                              parseRaisesException(SyntaxError)),
+         true);
+
+assertEq(testLenientAndStrict('({set x() {}, set x() {}})',
+                              parseRaisesException(SyntaxError),
                               parseRaisesException(SyntaxError)),
          true);
 
 assertEq(testLenientAndStrict('({get x() {}, set x(q) {}, y:1})',
                               parsesSuccessfully,
                               parsesSuccessfully),
          true);