[parser] Use Token::INIT for hoisted sloppy block functions when possible

Change-Id: I83dc3bed644361be1b94063daefd890b10ba50cd
Reviewed-on: https://chromium-review.googlesource.com/c/1433772
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59095}

src/ast/ast.h

@@ -491,26 +491,14 @@ inline NestedVariableDeclaration* VariableDeclaration::AsNested() {
 class FunctionDeclaration final : public Declaration {
  public:
   FunctionLiteral* fun() const { return fun_; }
-  bool declares_sloppy_block_function() const {
-    return DeclaresSloppyBlockFunction::decode(bit_field_);
-  }
 
  private:
   friend class AstNodeFactory;
 
-  class DeclaresSloppyBlockFunction
-      : public BitField<bool, Declaration::kNextBitFieldIndex, 1> {};
-
-  FunctionDeclaration(FunctionLiteral* fun, bool declares_sloppy_block_function,
-                      int pos)
-      : Declaration(pos, kFunctionDeclaration), fun_(fun) {
-    bit_field_ = DeclaresSloppyBlockFunction::update(
-        bit_field_, declares_sloppy_block_function);
-  }
+  FunctionDeclaration(FunctionLiteral* fun, int pos)
+      : Declaration(pos, kFunctionDeclaration), fun_(fun) {}
 
   FunctionLiteral* fun_;
-
-  static const uint8_t kNextBitFieldIndex = DeclaresSloppyBlockFunction::kNext;
 };
 
 
@@ -994,14 +982,30 @@ class SloppyBlockFunctionStatement final : public Statement {
  public:
   Statement* statement() const { return statement_; }
   void set_statement(Statement* statement) { statement_ = statement; }
+  Scope* scope() const { return var_->scope(); }
+  Variable* var() const { return var_; }
+  Token::Value init() const { return TokenField::decode(bit_field_); }
+  const AstRawString* name() const { return var_->raw_name(); }
+  SloppyBlockFunctionStatement** next() { return &next_; }
 
  private:
   friend class AstNodeFactory;
 
-  SloppyBlockFunctionStatement(int pos, Statement* statement)
-      : Statement(pos, kSloppyBlockFunctionStatement), statement_(statement) {}
+  class TokenField
+      : public BitField<Token::Value, Statement::kNextBitFieldIndex, 8> {};
+
+  SloppyBlockFunctionStatement(int pos, Variable* var, Token::Value init,
+                               Statement* statement)
+      : Statement(pos, kSloppyBlockFunctionStatement),
+        var_(var),
+        statement_(statement),
+        next_(nullptr) {
+    bit_field_ = TokenField::update(bit_field_, init);
+  }
 
+  Variable* var_;
   Statement* statement_;
+  SloppyBlockFunctionStatement* next_;
 };
 
 
@@ -2812,10 +2816,8 @@ class AstNodeFactory final {
     return new (zone_) NestedVariableDeclaration(scope, pos);
   }
 
-  FunctionDeclaration* NewFunctionDeclaration(FunctionLiteral* fun,
-                                              bool is_sloppy_block_function,
-                                              int pos) {
-    return new (zone_) FunctionDeclaration(fun, is_sloppy_block_function, pos);
+  FunctionDeclaration* NewFunctionDeclaration(FunctionLiteral* fun, int pos) {
+    return new (zone_) FunctionDeclaration(fun, pos);
   }
 
   Block* NewBlock(int capacity, bool ignore_completion_value) {
@@ -2958,8 +2960,10 @@ class AstNodeFactory final {
     return failure_expression_;
   }
 
-  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement(int pos) {
-    return new (zone_) SloppyBlockFunctionStatement(pos, EmptyStatement());
+  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement(
+      int pos, Variable* var, Token::Value init) {
+    return new (zone_)
+        SloppyBlockFunctionStatement(pos, var, init, EmptyStatement());
   }
 
   CaseClause* NewCaseClause(Expression* label,

src/ast/scopes.h

@@ -43,37 +43,6 @@ class VariableMap: public ZoneHashMap {
   void Add(Zone* zone, Variable* var);
 };
 
-
-// Sloppy block-scoped function declarations to var-bind
-class SloppyBlockFunctionMap : public ZoneHashMap {
- public:
-  class Delegate : public ZoneObject {
-   public:
-    Delegate(Scope* scope, SloppyBlockFunctionStatement* statement, int index)
-        : scope_(scope), statement_(statement), next_(nullptr), index_(index) {}
-    void set_statement(Statement* statement);
-
-    void set_next(Delegate* next) { next_ = next; }
-    Delegate* next() const { return next_; }
-    Scope* scope() const { return scope_; }
-    int index() const { return index_; }
-    int position() const { return statement_->position(); }
-
-   private:
-    Scope* scope_;
-    SloppyBlockFunctionStatement* statement_;
-    Delegate* next_;
-    int index_;
-  };
-
-  explicit SloppyBlockFunctionMap(Zone* zone);
-  void Declare(Zone* zone, const AstRawString* name, Scope* scope,
-               SloppyBlockFunctionStatement* statement);
-
- private:
-  int count_;
-};
-
 class Scope;
 
 template <>
@@ -940,17 +909,12 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   void AddLocal(Variable* var);
 
   void DeclareSloppyBlockFunction(
-      const AstRawString* name, Scope* scope,
-      SloppyBlockFunctionStatement* statement = nullptr);
+      SloppyBlockFunctionStatement* sloppy_block_function);
 
-  // Go through sloppy_block_function_map_ and hoist those (into this scope)
+  // Go through sloppy_block_functions_ and hoist those (into this scope)
   // which should be hoisted.
   void HoistSloppyBlockFunctions(AstNodeFactory* factory);
 
-  SloppyBlockFunctionMap* sloppy_block_function_map() {
-    return sloppy_block_function_map_;
-  }
-
   // Compute top scope and allocate variables. For lazy compilation the top
   // scope only contains the single lazily compiled function, so this
   // doesn't re-allocate variables repeatedly.
@@ -1069,7 +1033,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // Parameter list in source order.
   ZonePtrList<Variable> params_;
   // Map of function names to lists of functions defined in sloppy blocks
-  SloppyBlockFunctionMap* sloppy_block_function_map_;
+  base::ThreadedList<SloppyBlockFunctionStatement> sloppy_block_functions_;
   // Convenience variable.
   Variable* receiver_;
   // Function variable, if any; function scopes only.

src/ast/variables.h

@@ -137,6 +137,9 @@ class Variable final : public ZoneObject {
   }
 
   bool is_parameter() const { return kind() == PARAMETER_VARIABLE; }
+  bool is_sloppy_block_function() {
+    return kind() == SLOPPY_BLOCK_FUNCTION_VARIABLE;
+  }
 
   Variable* local_if_not_shadowed() const {
     DCHECK(mode() == VariableMode::kDynamicLocal &&
@@ -207,7 +210,7 @@ class Variable final : public ZoneObject {
 
   class VariableModeField : public BitField16<VariableMode, 0, 3> {};
   class VariableKindField
-      : public BitField16<VariableKind, VariableModeField::kNext, 2> {};
+      : public BitField16<VariableKind, VariableModeField::kNext, 3> {};
   class LocationField
       : public BitField16<VariableLocation, VariableKindField::kNext, 3> {};
   class ForceContextAllocationField

src/globals.h

@@ -1099,6 +1099,7 @@ enum VariableKind : uint8_t {
   NORMAL_VARIABLE,
   PARAMETER_VARIABLE,
   THIS_VARIABLE,
+  SLOPPY_BLOCK_FUNCTION_VARIABLE,
   SLOPPY_FUNCTION_NAME_VARIABLE
 };
 

src/parsing/parser-base.h

@@ -3648,10 +3648,10 @@ ParserBase<Impl>::ParseHoistableDeclaration(
   FuncNameInferrerState fni_state(&fni_);
   impl()->PushEnclosingName(name);
 
-  FunctionKind kind = FunctionKindFor(flags);
+  FunctionKind function_kind = FunctionKindFor(flags);
 
   FunctionLiteralT function = impl()->ParseFunctionLiteral(
-      name, scanner()->location(), name_validity, kind, pos,
+      name, scanner()->location(), name_validity, function_kind, pos,
       FunctionLiteral::kDeclaration, language_mode(), nullptr);
 
   // In ES6, a function behaves as a lexical binding, except in
@@ -3662,16 +3662,17 @@ ParserBase<Impl>::ParseHoistableDeclaration(
           : VariableMode::kVar;
   // Async functions don't undergo sloppy mode block scoped hoisting, and don't
   // allow duplicates in a block. Both are represented by the
-  // sloppy_block_function_map. Don't add them to the map for async functions.
+  // sloppy_block_functions_. Don't add them to the map for async functions.
   // Generators are also supposed to be prohibited; currently doing this behind
   // a flag and UseCounting violations to assess web compatibility.
-  bool is_sloppy_block_function = is_sloppy(language_mode()) &&
-                                  !scope()->is_declaration_scope() &&
-                                  flags == ParseFunctionFlag::kIsNormal;
-
-  return impl()->DeclareFunction(variable_name, function, mode, pos,
-                                 end_position(), is_sloppy_block_function,
-                                 names);
+  VariableKind kind = is_sloppy(language_mode()) &&
+                              !scope()->is_declaration_scope() &&
+                              flags == ParseFunctionFlag::kIsNormal
+                          ? SLOPPY_BLOCK_FUNCTION_VARIABLE
+                          : NORMAL_VARIABLE;
+
+  return impl()->DeclareFunction(variable_name, function, mode, kind, pos,
+                                 end_position(), names);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -1438,20 +1438,20 @@ Statement* Parser::BuildInitializationBlock(
 
 Statement* Parser::DeclareFunction(const AstRawString* variable_name,
                                    FunctionLiteral* function, VariableMode mode,
-                                   int beg_pos, int end_pos,
-                                   bool is_sloppy_block_function,
+                                   VariableKind kind, int beg_pos, int end_pos,
                                    ZonePtrList<const AstRawString>* names) {
-  Declaration* declaration = factory()->NewFunctionDeclaration(
-      function, is_sloppy_block_function, beg_pos);
+  Declaration* declaration =
+      factory()->NewFunctionDeclaration(function, beg_pos);
   bool was_added;
-  Declare(declaration, variable_name, NORMAL_VARIABLE, mode,
-          kCreatedInitialized, scope(), &was_added, beg_pos);
+  Declare(declaration, variable_name, kind, mode, kCreatedInitialized, scope(),
+          &was_added, beg_pos);
   if (names) names->Add(variable_name, zone());
-  if (is_sloppy_block_function) {
+  if (kind == SLOPPY_BLOCK_FUNCTION_VARIABLE) {
+    Token::Value init = loop_nesting_depth() > 0 ? Token::ASSIGN : Token::INIT;
     SloppyBlockFunctionStatement* statement =
-        factory()->NewSloppyBlockFunctionStatement(end_pos);
-    GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name, scope(),
-                                                      statement);
+        factory()->NewSloppyBlockFunctionStatement(end_pos, declaration->var(),
+                                                   init);
+    GetDeclarationScope()->DeclareSloppyBlockFunction(statement);
     return statement;
   }
   return factory()->EmptyStatement();

src/parsing/parser.h

@@ -341,8 +341,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Statement* DeclareFunction(const AstRawString* variable_name,
                              FunctionLiteral* function, VariableMode mode,
-                             int beg_pos, int end_pos,
-                             bool is_sloppy_block_function,
+                             VariableKind kind, int beg_pos, int end_pos,
                              ZonePtrList<const AstRawString>* names);
   Variable* CreateSyntheticContextVariable(const AstRawString* synthetic_name);
   FunctionLiteral* CreateInitializerFunction(

src/parsing/preparser.h

@@ -1101,16 +1101,14 @@ class PreParser : public ParserBase<PreParser> {
     // Don't bother actually binding the proxy.
   }
 
-  void DeclareVariableName(const AstRawString* name, VariableMode mode,
-                           Scope* scope, bool* was_added,
-                           int position = kNoSourcePosition,
-                           VariableKind kind = NORMAL_VARIABLE) {
+  Variable* DeclareVariableName(const AstRawString* name, VariableMode mode,
+                                Scope* scope, bool* was_added,
+                                int position = kNoSourcePosition,
+                                VariableKind kind = NORMAL_VARIABLE) {
     Variable* var = scope->DeclareVariableName(name, mode, was_added, kind);
     if (var == nullptr) {
       ReportUnidentifiableError();
-      return;
-    }
-    if (var->scope() != scope) {
+    } else if (var->scope() != scope) {
       DCHECK_NE(kNoSourcePosition, position);
       DCHECK_EQ(VariableMode::kVar, mode);
       Declaration* nested_declaration =
@@ -1119,6 +1117,7 @@ class PreParser : public ParserBase<PreParser> {
       nested_declaration->set_var(var);
       var->scope()->declarations()->Add(nested_declaration);
     }
+    return var;
   }
 
   V8_INLINE PreParserBlock RewriteCatchPattern(CatchInfo* catch_info) {
@@ -1183,22 +1182,22 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Default();
   }
 
-  V8_INLINE PreParserStatement
-  DeclareFunction(const PreParserIdentifier& variable_name,
-                  const PreParserExpression& function, VariableMode mode,
-                  int beg_pos, int end_pos, bool is_sloppy_block_function,
-                  ZonePtrList<const AstRawString>* names) {
+  V8_INLINE PreParserStatement DeclareFunction(
+      const PreParserIdentifier& variable_name,
+      const PreParserExpression& function, VariableMode mode, VariableKind kind,
+      int beg_pos, int end_pos, ZonePtrList<const AstRawString>* names) {
     DCHECK_NULL(names);
     if (variable_name.string_ != nullptr) {
       bool was_added;
-      if (is_strict(language_mode())) {
-        DeclareVariableName(variable_name.string_, mode, scope(), &was_added);
-      } else {
-        scope()->DeclareVariableName(variable_name.string_, mode, &was_added);
-      }
-      if (is_sloppy_block_function) {
-        GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name.string_,
-                                                          scope());
+      Variable* var = DeclareVariableName(variable_name.string_, mode, scope(),
+                                          &was_added, beg_pos, kind);
+      if (kind == SLOPPY_BLOCK_FUNCTION_VARIABLE) {
+        Token::Value init =
+            loop_nesting_depth() > 0 ? Token::ASSIGN : Token::INIT;
+        SloppyBlockFunctionStatement* statement =
+            factory()->ast_node_factory()->NewSloppyBlockFunctionStatement(
+                end_pos, var, init);
+        GetDeclarationScope()->DeclareSloppyBlockFunction(statement);
       }
     }
     return Statement::Default();

test/mjsunit/regress/regress-917755.js

@@ -2,11 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+assertThrows(`
 {
     function a() {}
 }
 
 {
+    // Duplicate lexical declarations are only allowed if they are both sloppy
+    // block functions (see bug 4693). In this case the sloppy block function
+    // conflicts with the lexical variable declaration, causing a syntax error.
     let a;
     function a() {};
 }
+`, SyntaxError)