[modules] Introduce new VariableLocation for module imports/exports.

src/ast/scopes.cc

Index: src/ast/scopes.cc
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index dfa66609b15c5340299dbc507c00f398e4c27c0e..a97725a4bf583232e597d5f0d99d40817c7c4be9 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -767,6 +767,7 @@ bool Scope::AllocateVariables(ParseInfo* info, AstNodeFactory* factory) {
   if (!ResolveVariablesRecursively(info, factory)) return false;
 
   // 3) Allocate variables.
+  if (info->is_module()) AllocateModuleVariables();
   AllocateVariablesRecursively(info->ast_value_factory());
 
   return true;
@@ -857,6 +858,12 @@ Scope* Scope::ClosureScope() {
   return scope;
 }
 
+Scope* Scope::ModuleScope() {
+  Scope* scope = this;
+  while (!scope->is_script_scope()) scope = scope->outer_scope();
+  scope = scope->inner_scope();

[adamk, 2016/08/02 16:34:31]

Why not just walk up till you hit the module scope (or not)? They should never
be nested...

[neis, 2016/08/03 09:16:30]

Because "this" may already be the script scope. See what Analyze does before it
calls AllocateVariables:

  // Traverse the scope tree up to the first unresolved scope or the global
  // scope and start scope resolution and variable allocation from that scope.
  while (!top->is_script_scope() &&
         !top->outer_scope()->already_resolved()) {
    top = top->outer_scope();
  }

[adamk, 2016/08/03 17:51:26]

But we'll walk _down_ the scope tree as part of analysis.

Maybe my question is, why isn't AllocateModuleVariables called from
AllocateVariablesRecursively when is_module_scope() is true, like we do with
parameters for function scopes?

+  return (scope == nullptr || !scope->is_module_scope()) ? nullptr : scope;
+}
 
 Scope* Scope::ReceiverScope() {
   Scope* scope = this;
@@ -979,6 +986,9 @@ static void PrintLocation(Variable* var) {
     case VariableLocation::LOOKUP:
       PrintF("lookup");
       break;
+    case VariableLocation::MODULE:
+      PrintF("module");
+      break;
   }
 }
 
@@ -1588,6 +1598,23 @@ void Scope::AllocateNonParameterLocalsAndDeclaredGlobals(
   }
 }
 
+void Scope::AllocateModuleVariables() {
+  Scope* module_scope = ModuleScope();

[adamk, 2016/08/02 16:34:31]

Does this need to walk up to the ModuleScope? I would have thought it would only
make sense to call it on the module's scope itself.

[neis, 2016/08/03 09:16:30]

I'm calling AllocateModuleVariables inside AllocateVariables, which may be
called on some quasi-random scope (see above).  But I'm open for suggestions.

[adamk, 2016/08/03 17:51:26]

See above, I was imagining that we'd check is_module_scope() in
AllocateVariablesRecursively and call this method in that case.

+  ModuleDescriptor* mod = module_scope->module();
+  for (auto entry : mod->imports()) {
+    if (entry->local_name == nullptr) continue;
+    if (entry->import_name == nullptr) continue;  // Namespace import.
+    Variable* var = module_scope->LookupLocal(entry->local_name);
+    // TODO(neis): Use a meaningful index.
+    var->AllocateTo(VariableLocation::MODULE, 42);

[adamk, 2016/08/02 16:34:31]

For imports, I think we want to check Scope::MustAllocate() (and remove the
is_module_scope() logic from that method) before allocating. If an import is
unreferenced, we shouldn't need space for it (but I admit that's kind of a weird
case).

[neis, 2016/08/03 09:16:30]

Not sure I understand. Setting the VariableLocation to MODULE doesn't by itself
reserve any space for the variable. It just tells the compiler where to look it
up. Space will be have to be reserved by the exporting module, not by the module
we're analyzing.

I will have a look at MustAllocate and the module logic you mentioned, though.

[adamk, 2016/08/03 17:51:26]

My understanding may be wrong, but I thought that once we set a variable as
"allocated" it's going to get space somewhere; that is, deciding to allocate
means deciding to reserve space "somewhere". As it happens we've yet to write
any code for actually allocating slots in some heap object for MODULE variables,
though, so I guess that can wait? A TODO would be appropriate if we want to
leave this allocated-always.

+  }
+  for (auto entry : mod->exports()) {
+    if (entry->local_name == nullptr) continue;
+    Variable* var = module_scope->LookupLocal(entry->local_name);
+    var->AllocateTo(VariableLocation::MODULE, 42);
+  }
+}
+
 void Scope::AllocateVariablesRecursively(AstValueFactory* ast_value_factory) {
   if (!already_resolved()) {
     num_stack_slots_ = 0;