Prohibit let in lexical bindings

This patch prohibits lexical bindings from being called 'let', even in
sloppy mode, following the ES2015 specification. The change affects
multiple cases of lexical bindings, including simple let/const declarations
and both kinds of for loops. var and legacy const bindings still permit
the name to be let, including in destructuring cases. Tests are added to
verify, though some cases are commented out since they led to (pre-existing)
crashes.

BUG=v8:4403
R=adamk
LOG=Y

Review URL: https://codereview.chromium.org/1371263003

Cr-Commit-Position: refs/heads/master@{#31115}

src/expression-classifier.h

@@ -35,11 +35,12 @@ class ExpressionClassifier {
     StrictModeFormalParametersProduction = 1 << 5,
     StrongModeFormalParametersProduction = 1 << 6,
     ArrowFormalParametersProduction = 1 << 7,
+    LetPatternProduction = 1 << 8,
 
     ExpressionProductions =
         (ExpressionProduction | FormalParameterInitializerProduction),
-    PatternProductions =
-        (BindingPatternProduction | AssignmentPatternProduction),
+    PatternProductions = (BindingPatternProduction |
+                          AssignmentPatternProduction | LetPatternProduction),
     FormalParametersProductions = (DistinctFormalParametersProduction |
                                    StrictModeFormalParametersProduction |
                                    StrongModeFormalParametersProduction),
@@ -100,6 +101,8 @@ class ExpressionClassifier {
     return is_valid(StrongModeFormalParametersProduction);
   }
 
+  bool is_valid_let_pattern() const { return is_valid(LetPatternProduction); }
+
   const Error& expression_error() const { return expression_error_; }
 
   const Error& formal_parameter_initializer_error() const {
@@ -128,6 +131,8 @@ class ExpressionClassifier {
     return strong_mode_formal_parameter_error_;
   }
 
+  const Error& let_pattern_error() const { return let_pattern_error_; }
+
   bool is_simple_parameter_list() const {
     return !(function_properties_ & NonSimpleParameter);
   }
@@ -217,6 +222,16 @@ class ExpressionClassifier {
     strong_mode_formal_parameter_error_.arg = arg;
   }
 
+  void RecordLetPatternError(const Scanner::Location& loc,
+                             MessageTemplate::Template message,
+                             const char* arg = nullptr) {
+    if (!is_valid_let_pattern()) return;
+    invalid_productions_ |= LetPatternProduction;
+    let_pattern_error_.location = loc;
+    let_pattern_error_.message = message;
+    let_pattern_error_.arg = arg;
+  }
+
   void Accumulate(const ExpressionClassifier& inner,
                   unsigned productions = StandardProductions) {
     // Propagate errors from inner, but don't overwrite already recorded
@@ -277,6 +292,7 @@ class ExpressionClassifier {
   Error duplicate_formal_parameter_error_;
   Error strict_mode_formal_parameter_error_;
   Error strong_mode_formal_parameter_error_;
+  Error let_pattern_error_;
   DuplicateFinder* duplicate_finder_;
 };
 

src/messages.h

@@ -268,6 +268,7 @@ class CallSite {
   T(InvalidTypedArrayAlignment, "% of % should be a multiple of %")            \
   T(InvalidTypedArrayLength, "Invalid typed array length")                     \
   T(InvalidTypedArrayOffset, "Start offset is too large:")                     \
+  T(LetInLexicalBinding, "let is disallowed as a lexically bound name")        \
   T(LocaleMatcher, "Illegal value for localeMatcher:%")                        \
   T(NormalizationForm, "The normalization form should be one of %.")           \
   T(NumberFormatRange, "% argument must be between 0 and 20")                  \

src/parser.cc

@@ -2535,6 +2535,10 @@ void Parser::ParseVariableDeclarations(VariableDeclarationContext var_context,
       if (!*ok) return;
       ValidateBindingPattern(&pattern_classifier, ok);
       if (!*ok) return;
+      if (IsLexicalVariableMode(parsing_result->descriptor.mode)) {
+        ValidateLetPattern(&pattern_classifier, ok);
+        if (!*ok) return;
+      }
       if (!allow_harmony_destructuring() && !pattern->IsVariableProxy()) {
         ReportUnexpectedToken(next);
         *ok = false;

src/preparser.cc

@@ -534,6 +534,7 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
   //   BindingPattern '=' AssignmentExpression
   bool require_initializer = false;
   bool is_strict_const = false;
+  bool lexical = false;
   if (peek() == Token::VAR) {
     if (is_strong(language_mode())) {
       Scanner::Location location = scanner()->peek_location();
@@ -559,10 +560,12 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
       DCHECK(var_context != kStatement);
       is_strict_const = true;
       require_initializer = var_context != kForStatement;
+      lexical = true;
     }
   } else if (peek() == Token::LET && allow_let()) {
     Consume(Token::LET);
     DCHECK(var_context != kStatement);
+    lexical = true;
   } else {
     *ok = false;
     return Statement::Default();
@@ -583,6 +586,9 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
       PreParserExpression pattern =
           ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
       ValidateBindingPattern(&pattern_classifier, CHECK_OK);
+      if (lexical) {
+        ValidateLetPattern(&pattern_classifier, CHECK_OK);
+      }
 
       if (!allow_harmony_destructuring() && !pattern.IsIdentifier()) {
         ReportUnexpectedToken(next);

src/preparser.h

@@ -618,6 +618,13 @@ class ParserBase : public Traits {
     }
   }
 
+  void ValidateLetPattern(const ExpressionClassifier* classifier, bool* ok) {
+    if (!classifier->is_valid_let_pattern()) {
+      ReportClassifierError(classifier->let_pattern_error());
+      *ok = false;
+    }
+  }
+
   void ExpressionUnexpectedToken(ExpressionClassifier* classifier) {
     MessageTemplate::Template message = MessageTemplate::kUnexpectedToken;
     const char* arg;
@@ -2092,6 +2099,10 @@ ParserBase<Traits>::ParseAndClassifyIdentifier(ExpressionClassifier* classifier,
               (next == Token::YIELD && !is_generator()))) {
     classifier->RecordStrictModeFormalParameterError(
         scanner()->location(), MessageTemplate::kUnexpectedStrictReserved);
+    if (next == Token::LET) {
+      classifier->RecordLetPatternError(scanner()->location(),
+                                        MessageTemplate::kLetInLexicalBinding);
+    }
     return this->GetSymbol(scanner());
   } else {
     this->ReportUnexpectedToken(next);

test/cctest/test-parsing.cc

@@ -7165,25 +7165,66 @@ TEST(LetSloppyOnly) {
   const char* context_data[][2] = {
     {"", ""},
     {"{", "}"},
+    {"(function() {", "})()"},
     {NULL, NULL}
   };
 
   const char* data[] = {
-    "let let",
     "let",
-    "let let = 1",
     "let = 1",
-    "for (let let = 1; let < 1; let++) {}",
     "for (let = 1; let < 1; let++) {}",
-    "for (let let in {}) {}",
-    "for (let let of []) {}",
     "for (let in {}) {}",
+    "for (var let = 1; let < 1; let++) {}",
+    "for (var let in {}) {}",
+    "for (var [let] = 1; let < 1; let++) {}",
+    "for (var [let] in {}) {}",
+    "var let",
+    // Unrelated parser crash BUG(v8:4462)
+    // "var [let]",
+    "for (const let = 1; let < 1; let++) {}",
+    "for (const let in {}) {}",
+    "for (const [let] = 1; let < 1; let++) {}",
+    // Unrelated parser crash BUG(v8:4461)
+    // "for (const [let] in {}) {}",
+    "const let",
+    "const [let]",
     NULL
   };
   // clang-format on
 
-  static const ParserFlag always_flags[] = {kAllowHarmonySloppy,
-                                            kAllowHarmonySloppyLet};
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonySloppy, kAllowHarmonySloppyLet, kAllowHarmonyDestructuring};
   RunParserSyncTest(context_data, data, kSuccess, NULL, 0, always_flags,
                     arraysize(always_flags));
+
+  // Some things should be rejected even in sloppy mode
+  // This addresses BUG(v8:4403).
+
+  // clang-format off
+  const char* fail_data[] = {
+    "let let = 1",
+    "for (let let = 1; let < 1; let++) {}",
+    "for (let let in {}) {}",
+    "for (let let of []) {}",
+    "const let = 1",
+    "for (const let = 1; let < 1; let++) {}",
+    "for (const let in {}) {}",
+    "for (const let of []) {}",
+    "let [let] = 1",
+    "for (let [let] = 1; let < 1; let++) {}",
+    "for (let [let] in {}) {}",
+    "for (let [let] of []) {}",
+    "const [let] = 1",
+    "for (const [let] = 1; let < 1; let++) {}",
+    "for (const [let] in {}) {}",
+    "for (const [let] of []) {}",
+    NULL
+  };
+  // clang-format on
+
+  static const ParserFlag fail_flags[] = {
+      kAllowHarmonySloppy, kAllowHarmonySloppyLet, kNoLegacyConst,
+      kAllowHarmonyDestructuring};
+  RunParserSyncTest(context_data, fail_data, kError, NULL, 0, fail_flags,
+                    arraysize(fail_flags));
 }

test/message/let-lexical-name-prohibited.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --harmony-sloppy --harmony-sloppy-let
+
+let let;

test/message/let-lexical-name-prohibited.out

+*%(basename)s:7: SyntaxError: let is disallowed as a lexically bound name
+let let;
+    ^^^
+SyntaxError: let is disallowed as a lexically bound name
+

test/mjsunit/harmony/block-let-contextual-sloppy.js

@@ -4,17 +4,19 @@
 
 // Flags: --harmony-sloppy --harmony-sloppy-let --harmony-destructuring
 
+// let is usable as a variable with var or legacy const, not let or ES6 const
 
-{
-  assertThrows(function() { return let; }, ReferenceError);
-  let let;
+(function (){
+  assertEquals(undefined, let);
+
+  var let;
 
   let = 5;
   assertEquals(5, let);
 
-  { let let = 1; assertEquals(1, let); }
+  (function() { var let = 1; assertEquals(1, let); })();
   assertEquals(5, let);
-}
+})();
 
 assertThrows(function() { return let; }, ReferenceError);
 
@@ -23,36 +25,36 @@ assertThrows(function() { return let; }, ReferenceError);
    for (let in [1, 2, 3, 4]) sum += Number(let);
    assertEquals(6, sum);
 
-   for (let let of [4, 5]) sum += let;
+   (function() { for (var let of [4, 5]) sum += let; })();
    assertEquals(15, sum);
 
-   for (let let in [6]) sum += Number([6][let]);
+   (function() { for (var let in [6]) sum += Number([6][let]); })();
    assertEquals(21, sum);
 
    for (let = 7; let < 8; let++) sum += let;
    assertEquals(28, sum);
    assertEquals(8, let);
 
-   for (let let = 8; let < 9; let++) sum += let;
+   (function() { for (var let = 8; let < 9; let++) sum += let; })();
    assertEquals(36, sum);
    assertEquals(8, let);
-})()
+})();
 
 assertThrows(function() { return let; }, ReferenceError);
 
-{
+(function () {
   let obj = {};
-  let {let} = {let() { return obj; }};
+  var {let} = {let() { return obj; }};
   let().x = 1;
   assertEquals(1, obj.x);
-}
+})();
 
-{
+(function () {
   let obj = {};
-  let [let] = [function() { return obj; }];
+  const [let] = [function() { return obj; }];
   let().x = 1;
   assertEquals(1, obj.x);
-}
+})();
 
 (function() {
   function let() {

test/test262/test262.status

@@ -129,9 +129,6 @@
   # This times out in sloppy mode because sloppy const assignment does not throw.
   'language/statements/const/syntax/const-invalid-assignment-next-expression-for': [PASS, FAIL, TIMEOUT],
 
-  # https://code.google.com/p/v8/issues/detail?id=4403
-  'language/statements/let/syntax/identifier-let-disallowed-as-boundname': [PASS, FAIL_SLOPPY],
-
   # Number/Boolean.prototype is a plain object in ES6
   # https://code.google.com/p/v8/issues/detail?id=4001
   'built-ins/Boolean/prototype/S15.6.3.1_A1': [FAIL],