[modules] Move MODULE variable back to Scopes, before resolution

Unlike other variable allocation logic, MODULE allocation does
not depend on resolution. So in order to give hole check elimination
(which runs during resolution) access to the information "is this
variable an import", simply allocate all modules variables prior
to resolution.

BUG=v8:1569

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

src/ast/scopes.cc

@@ -1080,6 +1080,10 @@ Declaration* Scope::CheckLexDeclarationsConflictingWith(
 }
 
 void DeclarationScope::AllocateVariables(ParseInfo* info, AnalyzeMode mode) {
+  // Module variables must be allocated before variable resolution
+  // to ensure that AccessNeedsHoleCheck() can detect import variables.
+  if (is_module_scope()) AsModuleScope()->AllocateModuleVariables();
+
   ResolveVariablesRecursively(info);
   AllocateVariablesRecursively();
 
@@ -1918,7 +1922,12 @@ void DeclarationScope::AllocateLocals() {
   }
 }
 
-void ModuleScope::AllocateModuleExports() {
+void ModuleScope::AllocateModuleVariables() {
+  for (const auto& it : module()->regular_imports()) {
+    Variable* var = LookupLocal(it.first);
+    var->AllocateTo(VariableLocation::MODULE, Variable::kModuleImportIndex);
+  }
+
   for (const auto& it : module()->regular_exports()) {
     Variable* var = LookupLocal(it.first);
     var->AllocateTo(VariableLocation::MODULE, Variable::kModuleExportIndex);
@@ -1944,9 +1953,7 @@ void Scope::AllocateVariablesRecursively() {
   // Allocate variables for this scope.
   // Parameters must be allocated first, if any.
   if (is_declaration_scope()) {
-    if (is_module_scope()) {
-      AsModuleScope()->AllocateModuleExports();
-    } else if (is_function_scope()) {
+    if (is_function_scope()) {
       AsDeclarationScope()->AllocateParameterLocals();
     }
     AsDeclarationScope()->AllocateReceiver();

src/ast/scopes.h

@@ -862,9 +862,9 @@ class ModuleScope final : public DeclarationScope {
     return module_descriptor_;
   }
 
-  // Set MODULE as VariableLocation for all variables that will live in this
-  // module's export table. Imports are allocated by the parser.
-  void AllocateModuleExports();
+  // Set MODULE as VariableLocation for all variables that will live in a
+  // module's export table.
+  void AllocateModuleVariables();
 
  private:
   ModuleDescriptor* module_descriptor_;

src/parsing/parser.cc

@@ -1154,7 +1154,8 @@ ZoneList<const Parser::NamedImport*>* Parser::ParseNamedImports(
       return nullptr;
     }
 
-    DeclareModuleImport(local_name, position(), CHECK_OK);
+    DeclareVariable(local_name, CONST, kNeedsInitialization, position(),
+                    CHECK_OK);
 
     NamedImport* import =
         new (zone()) NamedImport(import_name, local_name, location);
@@ -1204,7 +1205,8 @@ void Parser::ParseImportDeclaration(bool* ok) {
     import_default_binding =
         ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK_VOID);
     import_default_binding_loc = scanner()->location();
-    DeclareModuleImport(import_default_binding, pos, CHECK_OK_VOID);
+    DeclareVariable(import_default_binding, CONST, kNeedsInitialization, pos,
+                    CHECK_OK_VOID);
   }
 
   // Parse NameSpaceImport or NamedImports if present.
@@ -1481,21 +1483,6 @@ Declaration* Parser::DeclareVariable(const AstRawString* name,
   return declaration;
 }
 
-Declaration* Parser::DeclareModuleImport(const AstRawString* name, int pos,
-                                         bool* ok) {
-  DCHECK_EQ(MODULE_SCOPE, scope()->scope_type());
-  Declaration* decl =
-      DeclareVariable(name, CONST, kNeedsInitialization, pos, CHECK_OK);
-  // Allocate imports eagerly as hole check elimination logic in scope
-  // analisys depends on identifying imports.
-  // TODO(adamk): It's weird to allocate imports long before everything
-  // else. We should find a different way of filtering out imports
-  // during hole check elimination.
-  decl->proxy()->var()->AllocateTo(VariableLocation::MODULE,
-                                   Variable::kModuleImportIndex);
-  return decl;
-}
-
 Variable* Parser::Declare(Declaration* declaration,
                           DeclarationDescriptor::Kind declaration_kind,
                           VariableMode mode, InitializationFlag init, bool* ok,

src/parsing/parser.h

@@ -474,7 +474,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                                int pos, bool* ok);
   Declaration* DeclareVariable(const AstRawString* name, VariableMode mode,
                                InitializationFlag init, int pos, bool* ok);
-  Declaration* DeclareModuleImport(const AstRawString* name, int pos, bool* ok);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label, bool* ok);