Use bytecode to import variable, instead of builtin function.

src/wren_compiler.c

@@ -2484,6 +2484,9 @@ static int getNumArguments(const uint8_t* bytecode, const Value* constants,
     case CODE_LOAD_MODULE:
       return 2;
 
+    case CODE_IMPORT_VARIABLE:
+      return 4;
+
     case CODE_CLOSURE:
     {
       int constant = (bytecode[ip + 1] << 8) | bytecode[ip + 2];
@@ -2917,10 +2920,9 @@ static void import(Compiler* compiler)
   // Discard the unused result value from calling the module's fiber.
   emit(compiler, CODE_POP);
 
-  // Call "module".import_("variable")
-  emitShortArg(compiler, CODE_CONSTANT, moduleConstant);
-  emitShortArg(compiler, CODE_CONSTANT, variableConstant);
-  emitShortArg(compiler, CODE_CALL_1, methodSymbol(compiler, "import_ ", 8));
+  // Load the variable from the other module.
+  emitShortArg(compiler, CODE_IMPORT_VARIABLE, moduleConstant);
+  emitShort(compiler, variableConstant);
 
   // Store the result in the variable here.
   defineVariable(compiler, slot);

src/wren_core.c

@@ -1294,46 +1294,6 @@ DEF_NATIVE(string_subscript)
   RETURN_OBJ(result);
 }
 
-DEF_NATIVE(string_import)
-{
-  uint32_t moduleEntry = wrenMapFind(vm->modules, args[0]);
-  ASSERT(moduleEntry != UINT32_MAX, "Should only look up loaded modules.");
-
-  ObjModule* module = AS_MODULE(vm->modules->entries[moduleEntry].value);
-
-  ObjString* variableName = AS_STRING(args[1]);
-  uint32_t variable = wrenSymbolTableFind(&module->variableNames,
-                                          variableName->value,
-                                          variableName->length);
-
-  // It's a runtime error if the imported variable does not exist.
-  if (variable == UINT32_MAX)
-  {
-    // TODO: This is pretty verbose. Do something cleaner?
-    ObjString* moduleName = AS_STRING(args[0]);
-    int length = 48 + variableName->length + moduleName->length;
-    ObjString* error = AS_STRING(wrenNewUninitializedString(vm, length));
-
-    char* start = error->value;
-    memcpy(start, "Could not find a variable named '", 33);
-    start += 33;
-    memcpy(start, variableName->value, variableName->length);
-    start += variableName->length;
-    memcpy(start, "' in module '", 13);
-    start += 13;
-    memcpy(start, moduleName->value, moduleName->length);
-    start += moduleName->length;
-    memcpy(start, "'.", 2);
-    start += 2;
-    *start = '\0';
-
-    args[0] = OBJ_VAL(error);
-    return PRIM_ERROR;
-  }
-
-  RETURN_VAL(module->variables.data[variable]);
-}
-
 static ObjClass* defineSingleClass(WrenVM* vm, const char* name)
 {
   size_t length = strlen(name);
@@ -1535,18 +1495,14 @@ void wrenInitializeCore(WrenVM* vm)
   NATIVE(vm->rangeClass, "iteratorValue ", range_iteratorValue);
   NATIVE(vm->rangeClass, "toString", range_toString);
 
-  // TODO: Putting these on String is pretty strange. Find a better home for
-  // them.
-  NATIVE(vm->stringClass, "import_ ", string_import);
-
   // While bootstrapping the core types and running the core library, a number
-  // string objects have been created, many of which were instantiated before
-  // stringClass was stored in the VM. Some of them *must* be created first:
-  // the ObjClass for string itself has a reference to the ObjString for its
-  // name.
+  // of string objects have been created, many of which were instantiated
+  // before stringClass was stored in the VM. Some of them *must* be created
+  // first -- the ObjClass for string itself has a reference to the ObjString
+  // for its name.
   //
-  // These all currently a NULL classObj pointer, so go back and assign them
-  // now that the string class is known.
+  // These all currently have a NULL classObj pointer, so go back and assign
+  // them now that the string class is known.
   for (Obj* obj = vm->first; obj != NULL; obj = obj->next)
   {
     if (obj->type == OBJ_STRING) obj->classObj = vm->stringClass;

src/wren_debug.c

@@ -251,6 +251,18 @@ static int debugPrintInstruction(WrenVM* vm, ObjFn* fn, int i, int* lastLine)
       break;
     }
 
+    case CODE_IMPORT_VARIABLE:
+    {
+      int module = READ_SHORT();
+      int variable = READ_SHORT();
+      printf("%-16s %5d '", "IMPORT_VARIABLE", module);
+      wrenPrintValue(fn->constants[module]);
+      printf("' '");
+      wrenPrintValue(fn->constants[variable]);
+      printf("'\n");
+      break;
+    }
+
     case CODE_END:
       printf("CODE_END\n");
       break;

src/wren_vm.c

@@ -434,6 +434,50 @@ static Value importModule(WrenVM* vm, Value name)
   return OBJ_VAL(moduleFiber);
 }
 
+
+static bool importVariable(WrenVM* vm, Value moduleName, Value variableName,
+                            Value* result)
+{
+  uint32_t moduleEntry = wrenMapFind(vm->modules, moduleName);
+  ASSERT(moduleEntry != UINT32_MAX, "Should only look up loaded modules.");
+
+  ObjModule* module = AS_MODULE(vm->modules->entries[moduleEntry].value);
+
+  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)
+  {
+    *result = module->variables.data[variableEntry];
+    return true;
+  }
+
+  // TODO: This is pretty verbose. Do something cleaner?
+  ObjString* moduleString = AS_STRING(moduleName);
+  ObjString* variableString = AS_STRING(variableName);
+  int length = 48 + variableString->length + moduleString->length;
+  ObjString* error = AS_STRING(wrenNewUninitializedString(vm, length));
+
+  char* start = error->value;
+  memcpy(start, "Could not find a variable named '", 33);
+  start += 33;
+  memcpy(start, variableString->value, variableString->length);
+  start += variableString->length;
+  memcpy(start, "' in module '", 13);
+  start += 13;
+  memcpy(start, moduleString->value, moduleString->length);
+  start += moduleString->length;
+  memcpy(start, "'.", 2);
+  start += 2;
+  *start = '\0';
+
+  *result = OBJ_VAL(error);
+  return false;
+}
+
 // 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.
@@ -566,6 +610,7 @@ static bool runInterpreter(WrenVM* vm)
     &&code_METHOD_INSTANCE,
     &&code_METHOD_STATIC,
     &&code_LOAD_MODULE,
+    &&code_IMPORT_VARIABLE,
     &&code_END
   };
 
@@ -1116,6 +1161,22 @@ static bool runInterpreter(WrenVM* vm)
       DISPATCH();
     }
 
+    CASE_CODE(IMPORT_VARIABLE):
+    {
+      Value module = fn->constants[READ_SHORT()];
+      Value variable = fn->constants[READ_SHORT()];
+      Value result;
+      if (importVariable(vm, module, variable, &result))
+      {
+        PUSH(result);
+      }
+      else
+      {
+        RUNTIME_ERROR(AS_STRING(result));
+      }
+      DISPATCH();
+    }
+
     CASE_CODE(END):
       // A CODE_END should always be preceded by a CODE_RETURN. If we get here,
       // the compiler generated wrong code.

src/wren_vm.h

@@ -178,6 +178,12 @@ typedef enum
   // to that. When that fiber is done, the current one is resumed.
   CODE_LOAD_MODULE,
 
+  // Reads a top-level variable from another module. [arg1] is a string
+  // constant for the name of the module, and [arg2] is a string constant for
+  // the variable name. Pushes the variable if found, or generates a runtime
+  // error otherwise.
+  CODE_IMPORT_VARIABLE,
+
   // This pseudo-instruction indicates the end of the bytecode. It should
   // always be preceded by a `CODE_RETURN`, so is never actually executed.
   CODE_END