Bug 1288460 - Allow escape sequences in the keyword-like but non-reserved 'let' Identifier (in non-strict code). r=arai
authorJeff Walden <jwalden@mit.edu>
Tue, 30 Aug 2016 09:37:26 -0700
changeset 313598 869a5b6dae7834e2d33e92486b5fd9629f6c3000
parent 313597 c978746cc2cbb2a2c662e854095686ba2105ba5a
child 313599 240e3c1ab62217ac892c3d58c719750e20f5bcc4
push id20525
push usercbook@mozilla.com
push dateTue, 13 Sep 2016 10:28:20 +0000
treeherderfx-team@f5d043ce6d36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1288460
milestone51.0a1
Bug 1288460 - Allow escape sequences in the keyword-like but non-reserved 'let' Identifier (in non-strict code). r=arai
CLOBBER
js/src/frontend/Parser.cpp
js/src/frontend/TokenKind.h
js/src/frontend/TokenStream.cpp
js/src/frontend/TokenStream.h
js/src/tests/ecma_6/Syntax/escaped-let-static-identifier.js
js/src/tests/ecma_6/Syntax/let-as-label.js
js/src/vm/Keywords.h
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Bug 1287426 - Clobber required because of Linux Chromium sandbox file moves.
+Bug 1288460 requires a clobber due to bug 1298779.
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -840,17 +840,21 @@ Parser<ParseHandler>::reportBadReturn(No
  */
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::checkStrictBinding(PropertyName* name, TokenPos pos)
 {
     if (!pc->sc()->needStrictChecks())
         return true;
 
-    if (name == context->names().eval || name == context->names().arguments || IsKeyword(name)) {
+    if (name == context->names().eval ||
+        name == context->names().arguments ||
+        name == context->names().let ||
+        IsKeyword(name))
+    {
         JSAutoByteString bytes;
         if (!AtomToPrintableString(context, name, &bytes))
             return false;
         return reportWithOffset(ParseStrictError, pc->sc()->strict(), pos.begin,
                                 JSMSG_BAD_BINDING, bytes.ptr());
     }
 
     return true;
@@ -4854,25 +4858,38 @@ Parser<FullParseHandler>::exportDeclarat
         ParseNode* node = handler.newExportDefaultDeclaration(kid, nameNode,
                                                               TokenPos(begin, pos().end));
         if (!node || !pc->sc()->asModuleContext()->builder.processExport(node))
             return null();
 
         return node;
       }
 
-      case TOK_LET:
       case TOK_CONST:
-        kid = lexicalDeclaration(YieldIsName, tt == TOK_CONST);
+        kid = lexicalDeclaration(YieldIsName, /* isConst = */ true);
         if (!kid)
             return null();
         if (!checkExportedNamesForDeclaration(kid))
             return null();
         break;
 
+      case TOK_NAME:
+        if (tokenStream.currentName() == context->names().let) {
+            if (!checkUnescapedName())
+                return null();
+
+            kid = lexicalDeclaration(YieldIsName, /* isConst = */ false);
+            if (!kid)
+                return null();
+            if (!checkExportedNamesForDeclaration(kid))
+                return null();
+            break;
+        }
+        MOZ_FALLTHROUGH;
+
       default:
         report(ParseError, false, null(), JSMSG_DECLARATION_AFTER_EXPORT);
         return null();
     }
 
     ParseNode* node = handler.newExportDeclaration(kid, TokenPos(begin, pos().end));
     if (!node || !pc->sc()->asModuleContext()->builder.processExport(node))
         return null();
@@ -5108,26 +5125,25 @@ Parser<ParseHandler>::forHeadStart(Yield
         return *forInitialPart != null();
     }
 
     // Otherwise we have a lexical declaration or an expression.
 
     // For-in loop backwards compatibility requires that |let| starting a
     // for-loop that's not a (new to ES6) for-of loop, in non-strict mode code,
     // parse as an identifier.  (|let| in for-of is always a declaration.)
-    // Thus we must can't just sniff out TOK_CONST/TOK_LET here.  :-(
     bool parsingLexicalDeclaration = false;
     bool letIsIdentifier = false;
-    if (tt == TOK_LET || tt == TOK_CONST) {
+    if (tt == TOK_CONST) {
         parsingLexicalDeclaration = true;
         tokenStream.consumeKnownToken(tt, TokenStream::Operand);
-    } else if (tt == TOK_NAME && tokenStream.nextName() == context->names().let) {
-        MOZ_ASSERT(!pc->sc()->strict(),
-                   "should parse |let| as TOK_LET in strict mode code");
-
+    } else if (tt == TOK_NAME &&
+               tokenStream.nextName() == context->names().let &&
+               !tokenStream.nextNameContainsEscape())
+    {
         // We could have a {For,Lexical}Declaration, or we could have a
         // LeftHandSideExpression with lookahead restrictions so it's not
         // ambiguous with the former.  Check for a continuation of the former
         // to decide which we have.
         tokenStream.consumeKnownToken(TOK_NAME, TokenStream::Operand);
 
         TokenKind next;
         if (!tokenStream.peekToken(&next))
@@ -6415,20 +6431,19 @@ Parser<ParseHandler>::classDefinition(Yi
 
     return handler.newClass(nameNode, classHeritage, methodsOrBlock);
 }
 
 template <class ParseHandler>
 bool
 Parser<ParseHandler>::nextTokenContinuesLetDeclaration(TokenKind next, YieldHandling yieldHandling)
 {
-    MOZ_ASSERT(tokenStream.isCurrentTokenType(TOK_NAME),
-               "TOK_LET should have been summarily considered a "
-               "LexicalDeclaration");
+    MOZ_ASSERT(tokenStream.isCurrentTokenType(TOK_NAME));
     MOZ_ASSERT(tokenStream.currentName() == context->names().let);
+    MOZ_ASSERT(!tokenStream.currentToken().nameContainsEscape());
 
 #ifdef DEBUG
     TokenKind verify;
     MOZ_ALWAYS_TRUE(tokenStream.peekToken(&verify));
     MOZ_ASSERT(next == verify);
 #endif
 
     // Destructuring is (for once) the easy case.
@@ -6526,35 +6541,31 @@ Parser<ParseHandler>::statement(YieldHan
         return expressionStatement(yieldHandling);
       }
 
       case TOK_NAME: {
         TokenKind next;
         if (!tokenStream.peekToken(&next))
             return null();
 
-#ifdef DEBUG
-        if (tokenStream.currentName() == context->names().let) {
-            MOZ_ASSERT(!pc->sc()->strict(),
-                       "observing |let| as TOK_NAME and not TOK_LET implies "
-                       "non-strict code (and the edge case of 'use strict' "
-                       "immediately followed by |let| on a new line only "
-                       "applies to StatementListItems, not to Statements)");
-        }
-#endif
-
-        // Statement context forbids LexicalDeclaration.
-        if ((next == TOK_LB || next == TOK_LC || next == TOK_NAME) &&
+        // |let| here can only be an Identifier, not a declaration.  Give nicer
+        // errors for declaration-looking typos.
+        if (!tokenStream.currentToken().nameContainsEscape() &&
             tokenStream.currentName() == context->names().let)
         {
             bool forbiddenLetDeclaration = false;
-            if (next == TOK_LB) {
-                // ExpressionStatement has a 'let [' lookahead restriction.
+
+            if (pc->sc()->strict() || versionNumber() >= JSVERSION_1_7) {
+                // |let| can't be an Identifier in strict mode code.  Ditto for
+                // non-standard JavaScript 1.7+.
                 forbiddenLetDeclaration = true;
-            } else {
+            } else if (next == TOK_LB) {
+                // Enforce ExpressionStatement's 'let [' lookahead restriction.
+                forbiddenLetDeclaration = true;
+            } else if (next == TOK_LC || next == TOK_NAME) {
                 // 'let {' and 'let foo' aren't completely forbidden, if ASI
                 // causes 'let' to be the entire Statement.  But if they're
                 // same-line, we can aggressively give a better error message.
                 //
                 // Note that this ignores 'yield' as TOK_YIELD: we'll handle it
                 // correctly but with a worse error message.
                 TokenKind nextSameLine;
                 if (!tokenStream.peekTokenSameLine(&nextSameLine))
@@ -6672,24 +6683,16 @@ Parser<ParseHandler>::statement(YieldHan
       case TOK_CATCH:
         report(ParseError, false, null(), JSMSG_CATCH_WITHOUT_TRY);
         return null();
 
       case TOK_FINALLY:
         report(ParseError, false, null(), JSMSG_FINALLY_WITHOUT_TRY);
         return null();
 
-      // TOK_LET implies we're in strict mode code where static semantics
-      // forbid IdentifierName to be "let": a stronger restriction than
-      // Statement's lookahead restriction on |let [|.  Provide a better error
-      // message here than the default case would.
-      case TOK_LET:
-        report(ParseError, false, null(), JSMSG_FORBIDDEN_AS_STATEMENT, "let declarations");
-        return null();
-
       // NOTE: default case handled in the ExpressionStatement section.
     }
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::statementListItem(YieldHandling yieldHandling,
                                         bool canHaveDirectives /* = false */)
@@ -6747,33 +6750,21 @@ Parser<ParseHandler>::statementListItem(
         return expressionStatement(yieldHandling);
       }
 
       case TOK_NAME: {
         TokenKind next;
         if (!tokenStream.peekToken(&next))
             return null();
 
-        if (tokenStream.currentName() == context->names().let) {
-            if (nextTokenContinuesLetDeclaration(next, yieldHandling))
-                return lexicalDeclaration(yieldHandling, /* isConst = */ false);
-
-            // IdentifierName can't be "let" in strict mode code.  |let| in
-            // strict mode code is usually TOK_LET, but in this one weird case
-            // in global code it's TOK_NAME:
-            //
-            //   "use strict"              // ExpressionStatement ended by ASI
-            //   let <...whatever else...> // a fresh StatementListItem
-            //
-            // Carefully reject strict mode |let| non-declarations.
-            if (pc->sc()->strict()) {
-                report(ParseError, false, null(), JSMSG_UNEXPECTED_TOKEN,
-                       "declaration pattern", TokenKindToDesc(next));
-                return null();
-            }
+        if (!tokenStream.currentToken().nameContainsEscape() &&
+            tokenStream.currentName() == context->names().let &&
+            nextTokenContinuesLetDeclaration(next, yieldHandling))
+        {
+            return lexicalDeclaration(yieldHandling, /* isConst = */ false);
         }
 
         if (next == TOK_COLON)
             return labeledStatement(yieldHandling);
 
         return expressionStatement(yieldHandling);
       }
 
@@ -6849,21 +6840,20 @@ Parser<ParseHandler>::statementListItem(
         return functionStmt(yieldHandling, NameRequired);
 
       //   ClassDeclaration[?Yield, ~Default]
       case TOK_CLASS:
         return classDefinition(yieldHandling, ClassStatement, NameRequired);
 
       //   LexicalDeclaration[In, ?Yield]
       //     LetOrConst BindingList[?In, ?Yield]
-      case TOK_LET:
       case TOK_CONST:
         // [In] is the default behavior, because for-loops specially parse
         // their heads to handle |in| in this situation.
-        return lexicalDeclaration(yieldHandling, /* isConst = */ tt == TOK_CONST);
+        return lexicalDeclaration(yieldHandling, /* isConst = */ true);
 
       // ImportDeclaration (only inside modules)
       case TOK_IMPORT:
         return importDeclaration();
 
       // ExportDeclaration (only inside modules)
       case TOK_EXPORT:
         return exportDeclaration();
@@ -8249,16 +8239,26 @@ PropertyName*
 Parser<ParseHandler>::labelOrIdentifierReference(YieldHandling yieldHandling)
 {
     PropertyName* ident;
     const Token& tok = tokenStream.currentToken();
     if (tok.type == TOK_NAME) {
         ident = tok.name();
         MOZ_ASSERT(ident != context->names().yield,
                    "tokenizer should have treated 'yield' as TOK_YIELD");
+
+        if (pc->sc()->strict()) {
+            const char* badName = ident == context->names().let
+                                  ? "let"
+                                  : nullptr;
+            if (badName) {
+                report(ParseError, false, null(), JSMSG_RESERVED_ID, badName);
+                return nullptr;
+            }
+        }
     } else {
         MOZ_ASSERT(tok.type == TOK_YIELD);
 
         if (yieldHandling == YieldIsKeyword ||
             pc->sc()->strict() ||
             pc->isStarGenerator() ||
             versionNumber() >= JSVERSION_1_7)
         {
@@ -8288,16 +8288,24 @@ Parser<ParseHandler>::bindingIdentifier(
                                   ? "arguments"
                                   : ident == context->names().eval
                                   ? "eval"
                                   : nullptr;
             if (badName) {
                 report(ParseError, false, null(), JSMSG_BAD_STRICT_ASSIGN, badName);
                 return nullptr;
             }
+
+            badName = ident == context->names().let
+                      ? "let"
+                      : nullptr;
+            if (badName) {
+                report(ParseError, false, null(), JSMSG_RESERVED_ID, badName);
+                return nullptr;
+            }
         }
     } else {
         MOZ_ASSERT(tok.type == TOK_YIELD);
 
         if (yieldHandling == YieldIsKeyword ||
             pc->sc()->strict() ||
             pc->isStarGenerator() ||
             versionNumber() >= JSVERSION_1_7)
--- a/js/src/frontend/TokenKind.h
+++ b/js/src/frontend/TokenKind.h
@@ -102,17 +102,16 @@
     macro(NEW,          "keyword 'new'") \
     macro(DELETE,       "keyword 'delete'") \
     macro(TRY,          "keyword 'try'") \
     macro(CATCH,        "keyword 'catch'") \
     macro(FINALLY,      "keyword 'finally'") \
     macro(THROW,        "keyword 'throw'") \
     macro(DEBUGGER,     "keyword 'debugger'") \
     macro(YIELD,        "keyword 'yield'") \
-    macro(LET,          "keyword 'let'") \
     macro(EXPORT,       "keyword 'export'") \
     macro(IMPORT,       "keyword 'import'") \
     macro(CLASS,        "keyword 'class'") \
     macro(EXTENDS,      "keyword 'extends'") \
     macro(SUPER,        "keyword 'super'") \
     macro(RESERVED,     "reserved keyword") \
     /* reserved keywords in strict mode */ \
     macro(STRICT_RESERVED, "reserved keyword") \
@@ -234,18 +233,12 @@ TokenKindIsShift(TokenKind tt)
 }
 
 inline bool
 TokenKindIsAssignment(TokenKind tt)
 {
     return TOK_ASSIGNMENT_START <= tt && tt <= TOK_ASSIGNMENT_LAST;
 }
 
-inline bool
-TokenKindIsDecl(TokenKind tt)
-{
-    return tt == TOK_VAR || tt == TOK_LET;
-}
-
 } // namespace frontend
 } // namespace js
 
 #endif /* frontend_TokenKind_h */
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -976,21 +976,16 @@ bool
 TokenStream::checkForKeyword(const KeywordInfo* kw, TokenKind* ttp)
 {
     if (kw->tokentype == TOK_RESERVED)
         return reportError(JSMSG_RESERVED_ID, kw->chars);
 
     if (kw->tokentype == TOK_STRICT_RESERVED)
         return reportStrictModeError(JSMSG_RESERVED_ID, kw->chars);
 
-    // Treat 'let' as an identifier and contextually a keyword in sloppy mode.
-    // It is always a keyword in strict mode.
-    if (kw->tokentype == TOK_LET && !strictMode())
-        return true;
-
     // Working keyword.
     if (ttp) {
         *ttp = kw->tokentype;
         return true;
     }
 
     return reportError(JSMSG_RESERVED_ID, kw->chars);
 }
--- a/js/src/frontend/TokenStream.h
+++ b/js/src/frontend/TokenStream.h
@@ -346,16 +346,23 @@ class MOZ_STACK_CLASS TokenStream
 
     PropertyName* nextName() const {
         if (nextToken().type == TOK_YIELD)
             return cx->names().yield;
         MOZ_ASSERT(nextToken().type == TOK_NAME);
         return nextToken().name();
     }
 
+    bool nextNameContainsEscape() const {
+        if (nextToken().type == TOK_YIELD)
+            return false;
+        MOZ_ASSERT(nextToken().type == TOK_NAME);
+        return nextToken().nameContainsEscape();
+    }
+
     bool isCurrentTokenAssignment() const {
         return TokenKindIsAssignment(currentToken().type);
     }
 
     // Flag methods.
     bool isEOF() const { return flags.isEOF; }
     bool sawOctalEscape() const { return flags.sawOctalEscape; }
     bool hadError() const { return flags.hadError; }
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Syntax/escaped-let-static-identifier.js
@@ -0,0 +1,52 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1288460;
+var summary =
+  "|let| and |static| are forbidden as Identifier only in strict mode code, " +
+  "and it's permissible to use them as Identifier (with or without " +
+  "containing escapes) in non-strict code";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function t(code)
+{
+  var strictSemi = " 'use strict'; " + code;
+  var strictASI = " 'use strict' \n " + code;
+
+  var creationFunctions = [Function];
+  if (typeof evaluate === "function")
+    creationFunctions.push(evaluate);
+  if (typeof parseModule === "function")
+    creationFunctions.push(parseModule);
+
+  for (var func of creationFunctions)
+  {
+    if (typeof parseModule === "function" && func === parseModule)
+      assertThrowsInstanceOf(() => func(code), SyntaxError);
+    else
+      func(code);
+
+    assertThrowsInstanceOf(() => func(strictSemi), SyntaxError);
+    assertThrowsInstanceOf(() => func(strictASI), SyntaxError);
+  }
+}
+
+t("l\\u0065t: 42;");
+t("if (1) l\\u0065t: 42;");
+t("l\\u0065t = 42;");
+t("if (1) l\\u0065t = 42;");
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/tests/ecma_6/Syntax/let-as-label.js
+++ b/js/src/tests/ecma_6/Syntax/let-as-label.js
@@ -8,22 +8,28 @@ var BUGNUMBER = 1288459;
 var summary = "let can't be used as a label in strict mode code";
 
 print(BUGNUMBER + ": " + summary);
 
 /**************
  * BEGIN TEST *
  **************/
 
-Function("let: 42")
+Function("let: 42");
+Function("l\\u0065t: 42");
 assertThrowsInstanceOf(() => Function(" 'use strict'; let: 42"), SyntaxError);
 assertThrowsInstanceOf(() => Function(" 'use strict' \n let: 42"), SyntaxError);
+assertThrowsInstanceOf(() => Function(" 'use strict'; l\\u0065t: 42"), SyntaxError);
+assertThrowsInstanceOf(() => Function(" 'use strict' \n l\\u0065t: 42"), SyntaxError);
 
-eval("let: 42")
+eval("let: 42");
+eval("l\\u0065t: 42");
 assertThrowsInstanceOf(() => eval(" 'use strict'; let: 42"), SyntaxError);
 assertThrowsInstanceOf(() => eval(" 'use strict' \n let: 42;"), SyntaxError);
+assertThrowsInstanceOf(() => eval(" 'use strict'; l\\u0065t: 42"), SyntaxError);
+assertThrowsInstanceOf(() => eval(" 'use strict' \n l\\u0065t: 42;"), SyntaxError);
 
 /******************************************************************************/
 
 if (typeof reportCompare === "function")
   reportCompare(true, true);
 
 print("Tests complete");
--- a/js/src/vm/Keywords.h
+++ b/js/src/vm/Keywords.h
@@ -56,12 +56,11 @@
     macro(protected, protected_, TOK_STRICT_RESERVED) \
     macro(public, public_, TOK_STRICT_RESERVED) \
     macro(static, static_, TOK_STRICT_RESERVED) \
     /* \
      * Yield is a token inside function*.  Outside of a function*, it is a \
      * future reserved keyword in strict mode, but a keyword in JS1.7 even \
      * when strict.  Punt logic to parser. \
      */ \
-    macro(yield, yield, TOK_YIELD) \
-    macro(let, let, TOK_LET)
+    macro(yield, yield, TOK_YIELD)
 
 #endif /* vm_Keywords_h */