PreParser scope analysis: sloppy block funcs.

- Generalize the sloppy block function data structures to allow
  PreParser adding and hoisting sloppy block funcs.
- This completes PreParser scope analysis.

BUG=v8:5501, v8:5516
R=verwaest@chromium.org

Review-Url: https://codereview.chromium.org/2636543002
Cr-Commit-Position: refs/heads/master@{#42368}

src/ast/ast.h

@@ -1185,22 +1185,15 @@ class SloppyBlockFunctionStatement final : public Statement {
  public:
   Statement* statement() const { return statement_; }
   void set_statement(Statement* statement) { statement_ = statement; }
-  Scope* scope() const { return scope_; }
-  SloppyBlockFunctionStatement* next() { return next_; }
-  void set_next(SloppyBlockFunctionStatement* next) { next_ = next; }
 
  private:
   friend class AstNodeFactory;
 
-  SloppyBlockFunctionStatement(Statement* statement, Scope* scope)
+  explicit SloppyBlockFunctionStatement(Statement* statement)
       : Statement(kNoSourcePosition, kSloppyBlockFunctionStatement),
-        statement_(statement),
-        scope_(scope),
-        next_(nullptr) {}
+        statement_(statement) {}
 
   Statement* statement_;
-  Scope* const scope_;
-  SloppyBlockFunctionStatement* next_;
 };
 
 
@@ -3290,9 +3283,9 @@ class AstNodeFactory final BASE_EMBEDDED {
     return new (zone_) EmptyStatement(pos);
   }
 
-  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement(Scope* scope) {
-    return new (zone_) SloppyBlockFunctionStatement(
-        NewEmptyStatement(kNoSourcePosition), scope);
+  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement() {
+    return new (zone_)
+        SloppyBlockFunctionStatement(NewEmptyStatement(kNoSourcePosition));
   }
 
   CaseClause* NewCaseClause(

src/ast/scopes.cc

@@ -20,6 +20,14 @@ namespace internal {
 
 namespace {
 void* kDummyPreParserVariable = reinterpret_cast<void*>(0x1);
+void* kDummyPreParserLexicalVariable = reinterpret_cast<void*>(0x2);
+
+bool IsLexical(Variable* variable) {
+  if (variable == kDummyPreParserLexicalVariable) return true;
+  if (variable == kDummyPreParserVariable) return false;
+  return IsLexicalVariableMode(variable->mode());
+}
+
 }  // namespace
 
 // ----------------------------------------------------------------------------
@@ -56,14 +64,16 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope,
   return reinterpret_cast<Variable*>(p->value);
 }
 
-void VariableMap::DeclareName(Zone* zone, const AstRawString* name) {
+void VariableMap::DeclareName(Zone* zone, const AstRawString* name,
+                              VariableMode mode) {
   Entry* p =
       ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
                                   ZoneAllocationPolicy(zone));
   if (p->value == nullptr) {
     // The variable has not been declared yet -> insert it.
     DCHECK_EQ(name, p->key);
-    p->value = kDummyPreParserVariable;
+    p->value =
+        mode == VAR ? kDummyPreParserVariable : kDummyPreParserLexicalVariable;
   }
 }
 
@@ -92,21 +102,27 @@ Variable* VariableMap::Lookup(const AstRawString* name) {
   return NULL;
 }
 
+void SloppyBlockFunctionMap::Delegate::set_statement(Statement* statement) {
+  if (statement_ != nullptr) {
+    statement_->set_statement(statement);
+  }
+}
+
 SloppyBlockFunctionMap::SloppyBlockFunctionMap(Zone* zone)
     : ZoneHashMap(8, ZoneAllocationPolicy(zone)) {}
 
-void SloppyBlockFunctionMap::Declare(Zone* zone, const AstRawString* name,
-                                     SloppyBlockFunctionStatement* stmt) {
+void SloppyBlockFunctionMap::Declare(
+    Zone* zone, const AstRawString* name,
+    SloppyBlockFunctionMap::Delegate* delegate) {
   // AstRawStrings are unambiguous, i.e., the same string is always represented
   // by the same AstRawString*.
   Entry* p =
       ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
                                   ZoneAllocationPolicy(zone));
-  stmt->set_next(static_cast<SloppyBlockFunctionStatement*>(p->value));
-  p->value = stmt;
+  delegate->set_next(static_cast<SloppyBlockFunctionMap::Delegate*>(p->value));
+  p->value = delegate;
 }
 
-
 // ----------------------------------------------------------------------------
 // Implementation of Scope
 
@@ -448,11 +464,21 @@ int Scope::num_parameters() const {
   return is_declaration_scope() ? AsDeclarationScope()->num_parameters() : 0;
 }
 
+void DeclarationScope::DeclareSloppyBlockFunction(
+    const AstRawString* name, Scope* scope,
+    SloppyBlockFunctionStatement* statement) {
+  auto* delegate =
+      new (zone()) SloppyBlockFunctionMap::Delegate(scope, statement);
+  sloppy_block_function_map_.Declare(zone(), name, delegate);
+}
+
 void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
   DCHECK(is_sloppy(language_mode()));
   DCHECK(is_function_scope() || is_eval_scope() || is_script_scope() ||
          (is_block_scope() && outer_scope()->is_function_scope()));
-  DCHECK(HasSimpleParameters() || is_block_scope());
+  DCHECK(HasSimpleParameters() || is_block_scope() || is_being_lazily_parsed_);
+  DCHECK_EQ(factory == nullptr, is_being_lazily_parsed_);
+
   bool has_simple_parameters = HasSimpleParameters();
   // For each variable which is used as a function declaration in a sloppy
   // block,
@@ -484,7 +510,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
     bool var_created = false;
 
     // Write in assignments to var for each block-scoped function declaration
-    auto delegates = static_cast<SloppyBlockFunctionStatement*>(p->value);
+    auto delegates = static_cast<SloppyBlockFunctionMap::Delegate*>(p->value);
 
     DeclarationScope* decl_scope = this;
     while (decl_scope->is_eval_scope()) {
@@ -492,7 +518,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
     }
     Scope* outer_scope = decl_scope->outer_scope();
 
-    for (SloppyBlockFunctionStatement* delegate = delegates;
+    for (SloppyBlockFunctionMap::Delegate* delegate = delegates;
          delegate != nullptr; delegate = delegate->next()) {
       // Check if there's a conflict with a lexical declaration
       Scope* query_scope = delegate->scope()->outer_scope();
@@ -506,7 +532,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
       // `{ let e; try {} catch (e) { function e(){} } }`
       do {
         var = query_scope->LookupLocal(name);
-        if (var != nullptr && IsLexicalVariableMode(var->mode())) {
+        if (var != nullptr && IsLexical(var)) {
           should_hoist = false;
           break;
         }
@@ -518,8 +544,10 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
       // Declare a var-style binding for the function in the outer scope
       if (!var_created) {
         var_created = true;
-        VariableProxy* proxy = factory->NewVariableProxy(name, NORMAL_VARIABLE);
-        Declaration* declaration =
+        if (factory) {
+          VariableProxy* proxy =
+              factory->NewVariableProxy(name, NORMAL_VARIABLE);
+          auto declaration =
               factory->NewVariableDeclaration(proxy, this, kNoSourcePosition);
           // Based on the preceding check, it doesn't matter what we pass as
           // allow_harmony_restrictive_generators and
@@ -529,8 +557,12 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
                           Variable::DefaultInitializationFlag(VAR), false,
                           nullptr, &ok);
           CHECK(ok);  // Based on the preceding check, this should not fail
+        } else {
+          DeclareVariableName(name, VAR);
+        }
       }
 
+      if (factory) {
         Expression* assignment = factory->NewAssignment(
             Token::ASSIGN, NewUnresolved(factory, name),
             delegate->scope()->NewUnresolved(factory, name), kNoSourcePosition);
@@ -539,6 +571,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
         delegate->set_statement(statement);
       }
     }
+  }
 }
 
 void DeclarationScope::Analyze(ParseInfo* info, AnalyzeMode mode) {
@@ -1048,7 +1081,7 @@ void Scope::DeclareVariableName(const AstRawString* name, VariableMode mode) {
   DCHECK(scope_info_.is_null());
 
   // Declare the variable in the declaration scope.
-  variables_.DeclareName(zone(), name);
+  variables_.DeclareName(zone(), name, mode);
 }
 
 VariableProxy* Scope::NewUnresolved(AstNodeFactory* factory,
@@ -1647,7 +1680,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
   if (var == nullptr) return var;
 
   // TODO(marja): Separate LookupRecursive for preparsed scopes better.
-  if (var == kDummyPreParserVariable) {
+  if (var == kDummyPreParserVariable || var == kDummyPreParserLexicalVariable) {
     DCHECK(GetDeclarationScope()->is_being_lazily_parsed());
     DCHECK(FLAG_lazy_inner_functions);
     return var;
@@ -1845,7 +1878,8 @@ VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
     if (var == nullptr) {
       proxy->set_next_unresolved(stack);
       stack = proxy;
-    } else if (var != kDummyPreParserVariable) {
+    } else if (var != kDummyPreParserVariable &&
+               var != kDummyPreParserLexicalVariable) {
       if (info != nullptr) {
         // In this case we need to leave scopes in a way that they can be
         // allocated. If we resolved variables from lazy parsed scopes, we need

src/ast/scopes.h

@@ -21,6 +21,7 @@ class AstRawString;
 class Declaration;
 class ParseInfo;
 class SloppyBlockFunctionStatement;
+class Statement;
 class StringSet;
 class VariableProxy;
 
@@ -37,7 +38,7 @@ class VariableMap: public ZoneHashMap {
 
   // Records that "name" exists (if not recorded yet) but doesn't create a
   // Variable. Useful for preparsing.
-  void DeclareName(Zone* zone, const AstRawString* name);
+  void DeclareName(Zone* zone, const AstRawString* name, VariableMode mode);
 
   Variable* Lookup(const AstRawString* name);
   void Remove(Variable* var);
@@ -48,9 +49,24 @@ class VariableMap: public ZoneHashMap {
 // Sloppy block-scoped function declarations to var-bind
 class SloppyBlockFunctionMap : public ZoneHashMap {
  public:
+  class Delegate : public ZoneObject {
+   public:
+    explicit Delegate(Scope* scope,
+                      SloppyBlockFunctionStatement* statement = nullptr)
+        : scope_(scope), statement_(statement), next_(nullptr) {}
+    void set_statement(Statement* statement);
+    void set_next(Delegate* next) { next_ = next; }
+    Delegate* next() const { return next_; }
+    Scope* scope() const { return scope_; }
+
+   private:
+    Scope* scope_;
+    SloppyBlockFunctionStatement* statement_;
+    Delegate* next_;
+  };
+
   explicit SloppyBlockFunctionMap(Zone* zone);
-  void Declare(Zone* zone, const AstRawString* name,
-               SloppyBlockFunctionStatement* statement);
+  void Declare(Zone* zone, const AstRawString* name, Delegate* delegate);
 };
 
 enum class AnalyzeMode { kRegular, kDebugger };
@@ -747,10 +763,9 @@ class DeclarationScope : public Scope {
   // initializers.
   void AddLocal(Variable* var);
 
-  void DeclareSloppyBlockFunction(const AstRawString* name,
-                                  SloppyBlockFunctionStatement* statement) {
-    sloppy_block_function_map_.Declare(zone(), name, statement);
-  }
+  void DeclareSloppyBlockFunction(
+      const AstRawString* name, Scope* scope,
+      SloppyBlockFunctionStatement* statement = nullptr);
 
   // Go through sloppy_block_function_map_ and hoist those (into this scope)
   // which should be hoisted.

src/parsing/parser-base.h

@@ -3780,8 +3780,23 @@ ParserBase<Impl>::ParseHoistableDeclaration(
       pos, FunctionLiteral::kDeclaration, language_mode(),
       CHECK_OK_CUSTOM(NullStatement));
 
-  return impl()->DeclareFunction(variable_name, function, pos, is_generator,
-                                 is_async, names, ok);
+  // In ES6, a function behaves as a lexical binding, except in
+  // a script scope, or the initial scope of eval or another function.
+  VariableMode mode =
+      (!scope()->is_declaration_scope() || scope()->is_module_scope()) ? LET
+                                                                       : VAR;
+  // 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.
+  // 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() &&
+      !is_async && !(allow_harmony_restrictive_generators() && is_generator);
+
+  return impl()->DeclareFunction(variable_name, function, mode, pos,
+                                 is_generator, is_async,
+                                 is_sloppy_block_function, names, ok);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -1472,15 +1472,11 @@ void Parser::DeclareAndInitializeVariables(
 }
 
 Statement* Parser::DeclareFunction(const AstRawString* variable_name,
-                                   FunctionLiteral* function, int pos,
-                                   bool is_generator, bool is_async,
+                                   FunctionLiteral* function, VariableMode mode,
+                                   int pos, bool is_generator, bool is_async,
+                                   bool is_sloppy_block_function,
                                    ZoneList<const AstRawString*>* names,
                                    bool* ok) {
-  // In ES6, a function behaves as a lexical binding, except in
-  // a script scope, or the initial scope of eval or another function.
-  VariableMode mode =
-      (!scope()->is_declaration_scope() || scope()->is_module_scope()) ? LET
-                                                                       : VAR;
   VariableProxy* proxy =
       factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE);
   Declaration* declaration =
@@ -1488,18 +1484,12 @@ Statement* Parser::DeclareFunction(const AstRawString* variable_name,
   Declare(declaration, DeclarationDescriptor::NORMAL, mode, kCreatedInitialized,
           CHECK_OK);
   if (names) names->Add(variable_name, zone());
-  // 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.
-  // Generators are also supposed to be prohibited; currently doing this behind
-  // a flag and UseCounting violations to assess web compatibility.
-  if (is_sloppy(language_mode()) && !scope()->is_declaration_scope() &&
-      !is_async && !(allow_harmony_restrictive_generators() && is_generator)) {
-    SloppyBlockFunctionStatement* delegate =
-        factory()->NewSloppyBlockFunctionStatement(scope());
+  if (is_sloppy_block_function) {
+    SloppyBlockFunctionStatement* statement =
+        factory()->NewSloppyBlockFunctionStatement();
     DeclarationScope* target_scope = GetDeclarationScope();
-    target_scope->DeclareSloppyBlockFunction(variable_name, delegate);
-    return delegate;
+    target_scope->DeclareSloppyBlockFunction(variable_name, scope(), statement);
+    return statement;
   }
   return factory()->NewEmptyStatement(kNoSourcePosition);
 }

src/parsing/parser.h

@@ -341,8 +341,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                                  const CatchInfo& catch_info, int pos);
 
   Statement* DeclareFunction(const AstRawString* variable_name,
-                             FunctionLiteral* function, int pos,
-                             bool is_generator, bool is_async,
+                             FunctionLiteral* function, VariableMode mode,
+                             int pos, bool is_generator, bool is_async,
+                             bool is_sloppy_block_function,
                              ZoneList<const AstRawString*>* names, bool* ok);
   V8_INLINE Statement* DeclareClass(const AstRawString* variable_name,
                                     Expression* value,

src/parsing/preparser.cc

@@ -144,6 +144,10 @@ PreParser::PreParseResult PreParser::PreParseFunction(
   LazyParsingResult result = ParseStatementListAndLogFunction(
       &formals, has_duplicate_parameters, may_abort, ok);
 
+  if (is_sloppy(function_scope->language_mode())) {
+    function_scope->HoistSloppyBlockFunctions(nullptr);
+  }
+
   use_counts_ = nullptr;
   track_unresolved_variables_ = false;
 
@@ -232,6 +236,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   // Parsing the body may change the language mode in our scope.
   language_mode = function_scope->language_mode();
 
+  if (is_sloppy(language_mode)) {
+    function_scope->HoistSloppyBlockFunctions(nullptr);
+  }
+
   // Validate name and parameter names. We can do this only after parsing the
   // function, since the function can declare itself strict.
   CheckFunctionName(language_mode, function_name, function_name_validity,

src/parsing/preparser.h

@@ -886,6 +886,9 @@ class PreParser : public ParserBase<PreParser> {
                                  bool is_module = false) {
     DCHECK_NULL(scope_state_);
     DeclarationScope* scope = NewScriptScope();
+#ifdef DEBUG
+    scope->set_is_being_lazily_parsed(true);
+#endif
 
     // ModuleDeclarationInstantiation for Source Text Module Records creates a
     // new Module Environment Record whose outer lexical environment record is
@@ -1089,9 +1092,19 @@ class PreParser : public ParserBase<PreParser> {
   }
 
   V8_INLINE PreParserStatement DeclareFunction(
-      PreParserIdentifier variable_name, PreParserExpression function, int pos,
-      bool is_generator, bool is_async, ZoneList<const AstRawString*>* names,
+      PreParserIdentifier variable_name, PreParserExpression function,
+      VariableMode mode, int pos, bool is_generator, bool is_async,
+      bool is_sloppy_block_function, ZoneList<const AstRawString*>* names,
       bool* ok) {
+    DCHECK_NULL(names);
+    if (variable_name.string_ != nullptr) {
+      DCHECK(track_unresolved_variables_);
+      scope()->DeclareVariableName(variable_name.string_, mode);
+      if (is_sloppy_block_function) {
+        GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name.string_,
+                                                          scope());
+      }
+    }
     return Statement::Default();
   }
 
@@ -1616,8 +1629,8 @@ PreParserStatementList PreParser::ParseEagerFunctionBody(
     FunctionLiteral::FunctionType function_type, bool* ok) {
   PreParserStatementList result;
 
-  Scope* inner_scope = scope();
-  if (!parameters.is_simple) inner_scope = NewScope(BLOCK_SCOPE);
+  DeclarationScope* inner_scope = scope()->AsDeclarationScope();
+  if (!parameters.is_simple) inner_scope = NewVarblockScope();
 
   {
     BlockState block_state(&scope_state_, inner_scope);
@@ -1626,6 +1639,10 @@ PreParserStatementList PreParser::ParseEagerFunctionBody(
   }
 
   Expect(Token::RBRACE, ok);
+
+  if (is_sloppy(inner_scope->language_mode())) {
+    inner_scope->HoistSloppyBlockFunctions(nullptr);
+  }
   return result;
 }
 

test/cctest/test-parsing.cc

@@ -8615,6 +8615,21 @@ TEST(NoPessimisticContextAllocation) {
       {"function inner() { for (let my_var = 0; my_var < 1; ++my_var) { } "
        "my_var }",
        true},
+      {"function inner() { 'use strict'; if (true) { function my_var() {} }  "
+       "my_var; }",
+       true},
+      {"function inner() { 'use strict'; function inner2() { if (true) { "
+       "function my_var() {} }  my_var; } }",
+       true},
+      {"function inner() { function inner2() { 'use strict'; if (true) { "
+       "function my_var() {} }  my_var; } }",
+       true},
+      {"function inner() { () => { 'use strict'; if (true) { function my_var() "
+       "{} }  my_var; } }",
+       true},
+      {"function inner() { if (true) { let my_var; if (true) { function "
+       "my_var() {} } }  my_var; }",
+       true},
       // No pessimistic context allocation:
       {"function inner() { var my_var; my_var; }", false},
       {"function inner() { var my_var; }", false},
@@ -8806,10 +8821,18 @@ TEST(NoPessimisticContextAllocation) {
        "my_var } }",
        false},
       {"function inner() { class my_var {}; my_var }", false},
-      // In the following cases we still context allocate pessimistically:
-      {"function inner() { function my_var() {} my_var; }", true},
+      {"function inner() { function my_var() {} my_var; }", false},
       {"function inner() { if (true) { function my_var() {} }  my_var; }",
-       true},
+       false},
+      {"function inner() { function inner2() { if (true) { function my_var() "
+       "{} }  my_var; } }",
+       false},
+      {"function inner() { () => { if (true) { function my_var() {} }  my_var; "
+       "} }",
+       false},
+      {"function inner() { if (true) { var my_var; if (true) { function "
+       "my_var() {} } }  my_var; }",
+       false},
   };
 
   for (unsigned i = 0; i < arraysize(inners); ++i) {