Bug 1235640 - Correctly perform assignment-target detection and marking on a name (arguments, eval, or some other name) used as the target of a for-in/of loop. r=shu

--HG-- extra : rebase_source : 3071b43b19d4a35de0f8e898fa237ab3f5f8fbb3
2015-12-30 13:09:37 -06:00 · 2015-12-30 13:09:37 -06:00 · 0e7b5200f4
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@ -881,22 +881,30 @@ class FullParseHandler
        return pn->isConstant();
    }

-    PropertyName* maybeUnparenthesizedName(ParseNode* pn) {
-        if (!pn->isInParens() && pn->isKind(PNK_NAME))
-            return pn->pn_atom->asPropertyName();
-        return nullptr;
+    bool isUnparenthesizedName(ParseNode* node) {
+        return node->isKind(PNK_NAME) && !node->isInParens();
    }

-    PropertyName* maybeParenthesizedName(ParseNode* pn) {
-        if (pn->isInParens() && pn->isKind(PNK_NAME))
-            return pn->pn_atom->asPropertyName();
-        return nullptr;
+    bool isNameAnyParentheses(ParseNode* node) {
+        return node->isKind(PNK_NAME);
    }

-    PropertyName* maybeNameAnyParentheses(ParseNode* node) {
-        if (PropertyName* name = maybeUnparenthesizedName(node))
-            return name;
-        return maybeParenthesizedName(node);
+    bool nameIsEvalAnyParentheses(ParseNode* node, ExclusiveContext* cx) {
+        MOZ_ASSERT(isNameAnyParentheses(node),
+                   "must only call this function on known names");
+
+        return node->pn_atom == cx->names().eval;
+    }
+
+    const char* nameIsArgumentsEvalAnyParentheses(ParseNode* node, ExclusiveContext* cx) {
+        MOZ_ASSERT(isNameAnyParentheses(node),
+                   "must only call this function on known names");
+
+        if (nameIsEvalAnyParentheses(node, cx))
+            return js_eval_str;
+        if (node->pn_atom == cx->names().arguments)
+            return js_arguments_str;
+        return nullptr;
    }

    bool isCall(ParseNode* pn) {
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@ -4079,7 +4079,7 @@ Parser<FullParseHandler>::checkDestructuringName(BindData<FullParseHandler>* dat
    if (data) {
        // Destructuring patterns in declarations must only contain
        // unparenthesized names.
-        if (!handler.maybeUnparenthesizedName(expr)) {
+        if (!handler.isUnparenthesizedName(expr)) {
            report(ParseError, false, expr, JSMSG_NO_VARIABLE_NAME);
            return false;
        }
@ -4095,7 +4095,7 @@ Parser<FullParseHandler>::checkDestructuringName(BindData<FullParseHandler>* dat
               "function calls shouldn't be considered valid targets in "
               "destructuring patterns");

-    if (handler.maybeNameAnyParentheses(expr)) {
+    if (handler.isNameAnyParentheses(expr)) {
        // The arguments/eval identifiers are simple in non-strict mode code.
        // Warn to discourage their use nonetheless.
        if (!reportIfArgumentsEvalTarget(expr))
@ -5666,7 +5666,7 @@ Parser<ParseHandler>::validateForInOrOfLHSExpression(Node target)
    if (handler.isPropertyAccess(target))
        return true;

-    if (handler.maybeNameAnyParentheses(target)) {
+    if (handler.isNameAnyParentheses(target)) {
        // The arguments/eval identifiers are simple in non-strict mode code,
        // but warn to discourage use nonetheless.
        if (!reportIfArgumentsEvalTarget(target))
@ -5954,7 +5954,7 @@ Parser<ParseHandler>::forStatement(YieldHandling yieldHandling)

        pn3 = iteratedExpr;

-        if (handler.maybeNameAnyParentheses(pn2)) {
+        if (handler.isNameAnyParentheses(pn2)) {
            // Beware 'for (arguments in ...)' with or without a 'var'.
            handler.markAsAssigned(pn2);
        }
@ -7520,7 +7520,7 @@ Parser<ParseHandler>::checkAndMarkAsAssignmentLhs(Node target, AssignmentFlavor
    if (handler.isPropertyAccess(target))
        return true;

-    if (handler.maybeNameAnyParentheses(target)) {
+    if (handler.isNameAnyParentheses(target)) {
        // The arguments/eval identifiers are simple in non-strict mode code,
        // but warn to discourage use nonetheless.
        if (!reportIfArgumentsEvalTarget(target))
@ -7701,11 +7701,11 @@ Parser<ParseHandler>::isValidSimpleAssignmentTarget(Node node,
    // error for the various syntaxes that fail this, and warning for the
    // various syntaxes that "pass" this but should not, occurs elsewhere.

-    if (PropertyName* name = handler.maybeNameAnyParentheses(node)) {
+    if (handler.isNameAnyParentheses(node)) {
        if (!pc->sc->strict())
            return true;

-        return name != context->names().arguments && name != context->names().eval;
+        return !handler.nameIsArgumentsEvalAnyParentheses(node, context);
    }

    if (handler.isPropertyAccess(node))
@ -7723,21 +7723,16 @@ template <typename ParseHandler>
 bool
 Parser<ParseHandler>::reportIfArgumentsEvalTarget(Node nameNode)
 {
-    PropertyName* name = handler.maybeNameAnyParentheses(nameNode);
-    MOZ_ASSERT(name, "must only call this function on known names");
-
-    const char* chars = (name == context->names().arguments)
-                        ? js_arguments_str
-                        : (name == context->names().eval)
-                        ? js_eval_str
-                        : nullptr;
+    const char* chars = handler.nameIsArgumentsEvalAnyParentheses(nameNode, context);
    if (!chars)
        return true;

    if (!report(ParseStrictError, pc->sc->strict(), nameNode, JSMSG_BAD_STRICT_ASSIGN, chars))
        return false;

-    MOZ_ASSERT(!pc->sc->strict(), "in strict mode an error should have been reported");
+    MOZ_ASSERT(!pc->sc->strict(),
+               "an error should have been reported if this was strict mode "
+               "code");
    return true;
 }

@ -7751,7 +7746,7 @@ Parser<ParseHandler>::reportIfNotValidSimpleAssignmentTarget(Node target, Assign
    if (isValidSimpleAssignmentTarget(target, behavior))
        return true;

-    if (handler.maybeNameAnyParentheses(target)) {
+    if (handler.isNameAnyParentheses(target)) {
        // Use a special error if the target is arguments/eval.  This ensures
        // targeting these names is consistently a SyntaxError (which error numbers
        // below don't guarantee) while giving us a nicer error message.
@ -7802,7 +7797,7 @@ Parser<ParseHandler>::checkAndMarkAsIncOperand(Node target, AssignmentFlavor fla
        return false;

    // Mark.
-    if (handler.maybeNameAnyParentheses(target)) {
+    if (handler.isNameAnyParentheses(target)) {
        // Assignment to arguments/eval is allowed outside strict mode code,
        // but it's dodgy.  Report a strict warning (error, if werror was set).
        if (!reportIfArgumentsEvalTarget(target))
@ -7894,7 +7889,7 @@ Parser<ParseHandler>::unaryExpr(YieldHandling yieldHandling, TripledotHandling t

        // Per spec, deleting any unary expression is valid -- it simply
        // returns true -- except for one case that is illegal in strict mode.
-        if (handler.maybeNameAnyParentheses(expr)) {
+        if (handler.isNameAnyParentheses(expr)) {
            if (!report(ParseStrictError, pc->sc->strict(), expr, JSMSG_DEPRECATED_DELETE_OPERAND))
                return null();
            pc->sc->setBindingsAccessedDynamically();
@ -9090,8 +9085,8 @@ Parser<ParseHandler>::memberExpr(YieldHandling yieldHandling, TripledotHandling
                return null();

            JSOp op = JSOP_CALL;
-            if (PropertyName* name = handler.maybeNameAnyParentheses(lhs)) {
-                if (tt == TOK_LP && name == context->names().eval) {
+            if (handler.isNameAnyParentheses(lhs)) {
+                if (tt == TOK_LP && handler.nameIsEvalAnyParentheses(lhs, context)) {
                    /* Select JSOP_EVAL and flag pc as needsCallObject. */
                    op = pc->sc->strict() ? JSOP_STRICTEVAL : JSOP_EVAL;
                    pc->sc->setBindingsAccessedDynamically();
--- a/js/src/frontend/SyntaxParseHandler.h
+++ b/js/src/frontend/SyntaxParseHandler.h
@ -354,7 +354,7 @@ class SyntaxParseHandler
        // Careful: we're asking this well after the name was parsed, so the
        // value returned may not correspond to |kid|'s actual name.  But it
        // *will* be truthy iff |kid| was a name, so we're safe.
-        MOZ_ASSERT(maybeUnparenthesizedName(kid));
+        MOZ_ASSERT(isUnparenthesizedName(kid));
        return NodeGeneric;
    }

@ -551,30 +551,35 @@ class SyntaxParseHandler

    bool isConstant(Node pn) { return false; }

-    PropertyName* maybeUnparenthesizedName(Node node) {
-        if (node == NodeUnparenthesizedName ||
-            node == NodeUnparenthesizedArgumentsName ||
-            node == NodeUnparenthesizedEvalName)
-        {
-            return lastAtom->asPropertyName();
-        }
-        return nullptr;
+    bool isUnparenthesizedName(Node node) {
+        return node == NodeUnparenthesizedArgumentsName ||
+               node == NodeUnparenthesizedEvalName ||
+               node == NodeUnparenthesizedName;
    }

-    PropertyName* maybeParenthesizedName(Node node) {
-        if (node == NodeParenthesizedName ||
-            node == NodeParenthesizedArgumentsName ||
-            node == NodeParenthesizedEvalName)
-        {
-            return lastAtom->asPropertyName();
-        }
-        return nullptr;
+    bool isNameAnyParentheses(Node node) {
+        if (isUnparenthesizedName(node))
+            return true;
+        return node == NodeParenthesizedArgumentsName ||
+               node == NodeParenthesizedEvalName ||
+               node == NodeParenthesizedName;
    }

-    PropertyName* maybeNameAnyParentheses(Node node) {
-        if (PropertyName* name = maybeUnparenthesizedName(node))
-            return name;
-        return maybeParenthesizedName(node);
+    bool nameIsEvalAnyParentheses(Node node, ExclusiveContext* cx) {
+        MOZ_ASSERT(isNameAnyParentheses(node),
+                   "must only call this function on known names");
+        return node == NodeUnparenthesizedEvalName || node == NodeParenthesizedEvalName;
+    }
+
+    const char* nameIsArgumentsEvalAnyParentheses(Node node, ExclusiveContext* cx) {
+        MOZ_ASSERT(isNameAnyParentheses(node),
+                   "must only call this method on known names");
+
+        if (nameIsEvalAnyParentheses(node, cx))
+            return js_eval_str;
+        if (node == NodeUnparenthesizedArgumentsName || node == NodeParenthesizedArgumentsName)
+            return js_arguments_str;
+        return nullptr;
    }

    PropertyName* maybeDottedProperty(Node node) {
--- a/js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js
+++ b/js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js
@ -0,0 +1,43 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+//-----------------------------------------------------------------------------
+var gTestfile = "for-inof-name-iteration-expression-contains-index-string.js";
+var BUGNUMBER = 1235640;
+var summary =
+  "Don't assert parsing a for-in/of loop whose target is a name, where the " +
+  "expression being iterated over contains a string containing an index";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function f()
+{
+  var x;
+  for (x in "9")
+    continue;
+  assertEq(x, "0");
+}
+
+f();
+
+function g()
+{
+  "use strict";
+  var x = "unset";
+  for (x in arguments)
+    continue;
+  assertEq(x, "unset");
+}
+
+g();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");