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 id30694
push usercbook@mozilla.com
push dateTue, 13 Sep 2016 09:58:08 +0000
treeherdermozilla-central@f5d043ce6d36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1288460
milestone51.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 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 */