Don't use module import string when loading imported variables.

This is an interim step towards supporting relative imports. Previously,
the IMPORT_VARIABLE instruction had a constant string operand for the
import string of the module to import the variable from. However, with
relative imports, the import string needs to be resolved by the host
all into a canonical import string. At that point, the original import
string in the source is no longer useful.

This changes that to have IMPORT_VARIABLE access the imported ObjModule
directly. It works in two pieces:

1. When a module is compiled, it ends with an END_MODULE instruction.
   That instruction stores the current ObjModule in vm->lastModule.

2. The IMPORT_VARIABLE instruction uses vm->lastModule as the module to
   load the variable from. Since no interesting code can execute between
   when a module body completes and the subsequent IMPORT_VARIABLE
   statements, we know vm->lastModule will be the one we imported.

src/optional/wren_opt_meta.c

@@ -29,7 +29,8 @@ void metaCompile(WrenVM* vm)
   }
 }
 
-void metaGetModuleVariables(WrenVM* vm) {
+void metaGetModuleVariables(WrenVM* vm)
+{
   wrenEnsureSlots(vm, 3);
   
   Value moduleValue = wrenMapGet(vm->modules, vm->apiStack[1]);

src/vm/wren_compiler.c

@@ -1609,9 +1609,7 @@ static void patchJump(Compiler* compiler, int offset)
 static bool finishBlock(Compiler* compiler)
 {
   // Empty blocks do nothing.
-  if (match(compiler, TOKEN_RIGHT_BRACE)) {
+  if (match(compiler, TOKEN_RIGHT_BRACE)) return false;
-    return false;
-  }
 
   // If there's no line after the "{", it's a single-expression body.
   if (!matchLine(compiler))
@@ -1622,9 +1620,7 @@ static bool finishBlock(Compiler* compiler)
   }
 
   // Empty blocks (with just a newline inside) do nothing.
-  if (match(compiler, TOKEN_RIGHT_BRACE)) {
+  if (match(compiler, TOKEN_RIGHT_BRACE)) return false;
-    return false;
-  }
 
   // Compile the definition list.
   do
@@ -2719,6 +2715,7 @@ static int getNumArguments(const uint8_t* bytecode, const Value* constants,
     case CODE_CONSTRUCT:
     case CODE_FOREIGN_CONSTRUCT:
     case CODE_FOREIGN_CLASS:
+    case CODE_END_MODULE:
       return 0;
 
     case CODE_LOAD_LOCAL:
@@ -3321,9 +3318,13 @@ static void classDefinition(Compiler* compiler, bool isForeign)
 //     import "foo" for Bar, Baz
 //
 // We compile a single IMPORT_MODULE "foo" instruction to load the module
-// itself. Then, for each imported name, we declare a variable and them emit a
+// itself. When that finishes executing the imported module, it leaves the
-// IMPORT_VARIABLE instruction to load the variable from the other module and
+// ObjModule in vm->lastModule. Then, for Bar and Baz, we:
-// assign it to the new variable in this one.
+//
+// * Declare a variable in the current scope with that name.
+// * Emit an IMPORT_VARIABLE instruction to load the variable's value from the
+//   other module.
+// * Compile the code to store that value in the variable in this scope.
 static void import(Compiler* compiler)
 {
   ignoreNewlines(compiler);
@@ -3333,9 +3334,9 @@ static void import(Compiler* compiler)
   // Load the module.
   emitShortArg(compiler, CODE_IMPORT_MODULE, moduleConstant);
 
-  // Discard the unused result value from calling the module's fiber.
+  // Discard the unused result value from calling the module body's closure.
   emitOp(compiler, CODE_POP);
-
+  
   // The for clause is optional.
   if (!match(compiler, TOKEN_FOR)) return;
 
@@ -3344,16 +3345,15 @@ static void import(Compiler* compiler)
   {
     ignoreNewlines(compiler);
     int slot = declareNamedVariable(compiler);
-
+    
     // Define a string constant for the variable name.
     int variableConstant = addConstant(compiler,
         wrenNewStringLength(compiler->parser->vm,
                             compiler->parser->previous.start,
                             compiler->parser->previous.length));
-
+    
     // Load the variable from the other module.
-    emitShortArg(compiler, CODE_IMPORT_VARIABLE, moduleConstant);
+    emitShortArg(compiler, CODE_IMPORT_VARIABLE, variableConstant);
-    emitShort(compiler, variableConstant);
     
     // Store the result in the variable here.
     defineVariable(compiler, slot);
@@ -3466,7 +3466,7 @@ ObjFn* wrenCompile(WrenVM* vm, ObjModule* module, const char* source,
       }
     }
     
-    emitOp(&compiler, CODE_NULL);
+    emitOp(&compiler, CODE_END_MODULE);
   }
   
   emitOp(&compiler, CODE_RETURN);

src/vm/wren_debug.c

@@ -313,6 +313,10 @@ static int dumpInstruction(WrenVM* vm, ObjFn* fn, int i, int* lastLine)
       break;
     }
       
+    case CODE_END_MODULE:
+      printf("END_MODULE\n");
+      break;
+      
     case CODE_IMPORT_MODULE:
     {
       int name = READ_SHORT();
@@ -324,16 +328,13 @@ static int dumpInstruction(WrenVM* vm, ObjFn* fn, int i, int* lastLine)
       
     case CODE_IMPORT_VARIABLE:
     {
-      int module = READ_SHORT();
       int variable = READ_SHORT();
-      printf("%-16s %5d '", "IMPORT_VARIABLE", module);
+      printf("%-16s %5d '", "IMPORT_VARIABLE", variable);
-      wrenDumpValue(fn->constants.data[module]);
-      printf("' %5d '", variable);
       wrenDumpValue(fn->constants.data[variable]);
       printf("'\n");
       break;
     }
-
+      
     case CODE_END:
       printf("END\n");
       break;

src/vm/wren_opcodes.h

@@ -190,6 +190,11 @@ OPCODE(METHOD_INSTANCE, -2)
 // closure.
 OPCODE(METHOD_STATIC, -2)
 
+// This is executed at the end of the module's body. Pushes NULL onto the stack
+// as the "return value" of the import statement and stores the module as the
+// most recently imported one.
+OPCODE(END_MODULE, 1)
+
 // Import a module whose name is the string stored at [arg] in the constant
 // table.
 //
@@ -198,9 +203,9 @@ OPCODE(METHOD_STATIC, -2)
 // value when resuming a caller.)
 OPCODE(IMPORT_MODULE, 1)
 
-// Import a variable from a previously-imported module. The module's name is at
+// Import a variable from the most recently imported module. The name of the
-// [arg1] in the constant table and the variable name is at [arg2]. Pushes the
+// variable to import is at [arg] in the constant table. Pushes the loaded
-// loaded variable onto the stack.
+// variable's value.
 OPCODE(IMPORT_VARIABLE, 1)
 
 // This pseudo-instruction indicates the end of the bytecode. It should

src/vm/wren_primitive.c

@@ -80,7 +80,8 @@ uint32_t calculateRange(WrenVM* vm, ObjRange* range, uint32_t* length,
   // list[0..-1] and list[0...list.count] can be used to copy a list even when
   // empty.
   if (range->from == *length &&
-      range->to == (range->isInclusive ? -1.0 : (double)*length)) {
+      range->to == (range->isInclusive ? -1.0 : (double)*length))
+  {
     *length = 0;
     return 0;
   }

src/vm/wren_vm.c

@@ -696,7 +696,8 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign)
 Value wrenImportModule(WrenVM* vm, Value name)
 {
   // If the module is already loaded, we don't need to do anything.
-  if (!IS_UNDEFINED(wrenMapGet(vm->modules, name))) return NULL_VAL;
+  Value existing = wrenMapGet(vm->modules, name);
+  if (!IS_UNDEFINED(existing)) return existing;
   
   const char* source = NULL;
   bool allocatedSource = true;
@@ -747,6 +748,26 @@ Value wrenImportModule(WrenVM* vm, Value name)
   return OBJ_VAL(moduleClosure);
 }
 
+static Value getModuleVariable(WrenVM* vm, ObjModule* module,
+                               Value variableName)
+{
+  ObjString* variable = AS_STRING(variableName);
+  uint32_t variableEntry = wrenSymbolTableFind(&module->variableNames,
+                                               variable->value,
+                                               variable->length);
+  
+  // It's a runtime error if the imported variable does not exist.
+  if (variableEntry != UINT32_MAX)
+  {
+    return module->variables.data[variableEntry];
+  }
+  
+  vm->fiber->error = wrenStringFormat(vm,
+      "Could not find a variable named '@' in module '@'.",
+      variableName, OBJ_VAL(module->name));
+  return NULL_VAL;
+}
+
 // The main bytecode interpreter loop. This is where the magic happens. It is
 // also, as you can imagine, highly performance critical. Returns `true` if the
 // fiber completed without error.
@@ -1244,17 +1265,24 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
       DISPATCH();
     }
     
+    CASE_CODE(END_MODULE):
+    {
+      vm->lastModule = fn->module;
+      PUSH(NULL_VAL);
+      DISPATCH();
+    }
+    
     CASE_CODE(IMPORT_MODULE):
     {
       Value name = fn->constants.data[READ_SHORT()];
 
-      // Make a slot on the stack for the module's fiber to place the return
-      // value. It will be popped after this fiber is resumed.
-      PUSH(NULL_VAL);
-      
       Value result = wrenImportModule(vm, name);
       if (!IS_NULL(fiber->error)) RUNTIME_ERROR();
 
+      // Make a slot on the stack for the module's closure to place the return
+      // value. It will be popped after the module body code returns.
+      PUSH(NULL_VAL);
+      
       // If we get a closure, call it to execute the module body.
       if (IS_CLOSURE(result))
       {
@@ -1263,16 +1291,21 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
         callFunction(vm, fiber, closure, 1);
         LOAD_FRAME();
       }
+      else
+      {
+        // The module has already been loaded. Remember it so we can import
+        // variables from it if needed.
+        vm->lastModule = AS_MODULE(result);
+      }
 
       DISPATCH();
     }
     
     CASE_CODE(IMPORT_VARIABLE):
     {
-      Value module = fn->constants.data[READ_SHORT()];
       Value variable = fn->constants.data[READ_SHORT()];
-      
+      ASSERT(vm->lastModule != NULL, "Should have already imported module.");
-      Value result = wrenGetModuleVariable(vm, module, variable);
+      Value result = getModuleVariable(vm, vm->lastModule, variable);
       if (!IS_NULL(fiber->error)) RUNTIME_ERROR();
 
       PUSH(result);
@@ -1448,21 +1481,7 @@ Value wrenGetModuleVariable(WrenVM* vm, Value moduleName, Value variableName)
     return NULL_VAL;
   }
   
-  ObjString* variable = AS_STRING(variableName);
+  return getModuleVariable(vm, module, variableName);
-  uint32_t variableEntry = wrenSymbolTableFind(&module->variableNames,
-                                               variable->value,
-                                               variable->length);
-  
-  // It's a runtime error if the imported variable does not exist.
-  if (variableEntry != UINT32_MAX)
-  {
-    return module->variables.data[variableEntry];
-  }
-  
-  vm->fiber->error = wrenStringFormat(vm,
-      "Could not find a variable named '@' in module '@'.",
-      variableName, moduleName);
-  return NULL_VAL;
 }
 
 Value wrenFindVariable(WrenVM* vm, ObjModule* module, const char* name)

src/vm/wren_vm.h

@@ -49,6 +49,12 @@ struct WrenVM
   // whose key is null) for the module's name and the value is the ObjModule
   // for the module.
   ObjMap* modules;
+  
+  // The most recently imported module. More specifically, the module whose
+  // code has most recently finished executing.
+  //
+  // Not treated like a GC root since the module is already in [modules].
+  ObjModule* lastModule;
 
   // Memory management data:
 

test/api/main.c

@@ -91,7 +91,8 @@ static WrenForeignClassMethods bindForeignClass(
   return methods;
 }
 
-static void afterLoad(WrenVM* vm) {
+static void afterLoad(WrenVM* vm)
+{
   if (strstr(testName, "/call.wren") != NULL)
   {
     callRunTests(vm);