additional documentation updates

The original attempt at handling the returns from loadModuleFn wasn't ideal. 889cae5ff1

Instead of making the host go via the VM allocation and need to understand it semantically, we can instead solve the problem of the unfreed return result directly.

This also opens up the option of providing a length parameter or other information needed later (length is optional, and not used as of right now, but exists to show intent).

doc/site/embedding/configuring-the-vm.markdown

@@ -47,22 +47,53 @@ for a module.
 The signature of this function is:
 
 <pre class="snippet" data-lang="c">
-char* loadModule(WrenVM* vm, const char* name)
+WrenLoadModuleResult loadModule(WrenVM* vm, const char* name)
 </pre>
 
 When a module is imported, Wren calls this and passes in the module's name. The
-host should return the source code for that module. Memory for the source should
-be allocated using the same allocator that the VM uses for other allocation (see
-below). Wren will take ownership of the returned string and free it later.
+host should return the source code for that module in a `WrenLoadModuleResult` struct.
+
+<pre class="snippet" data-lang="c">
+WrenLoadModuleResult myLoadModule(WrenVM* vm, const char* name) {
+  WrenLoadModuleResult result = {0};
+    result.source = getSourceForModule(name);
+  return result;
+}
+</pre>
 
 The module loader is only be called once for any given module name. Wren caches
 the result internally so subsequent imports of the same module use the
 previously loaded code.
 
 If your host application isn't able to load a module with some name, it should
-return `NULL` and Wren will report that as a runtime error.
+make sure the `source` value is `NULL` when returned. Wren will then report that as a runtime error.
 
-If you don't use any `import` statements, you can leave this `NULL`.
+If you don't use any `import` statements, you can leave the `loadModuleFn` field in
+the configuration set to `NULL` (the default).
+
+Additionally, the `WrenLoadModuleResult` allows us to add a callback for when Wren is 
+done with the `source`, so we can free the memory if needed.
+
+<pre class="snippet" data-lang="c">
+
+static void loadModuleComplete(WrenVM* vm, 
+                               const char* module,
+                               WrenLoadModuleResult result) 
+{
+  if(result.source) {
+    //for example, if we used malloc to allocate
+    our source string, we use free to release it.
+    free((void*)result.source);
+  }
+}
+
+WrenLoadModuleResult myLoadModule(WrenVM* vm, const char* name) {
+  WrenLoadModuleResult result = {0};
+    result.onComplete = loadModuleComplete;
+    result.source = getSourceForModule(name);
+  return result;
+}
+</pre>
 
 ### **`bindForeignMethodFn`**
 

doc/site/modularity.markdown

@@ -42,17 +42,6 @@ if (thirsty) {
 }
 </pre>
 
-If you need to import a variable under a different name, you can use 
-`import "..." for Name as OtherName`. This looks up the top-level variable
-`Name` in *that* module, but declares a variable called `OtherName` in *this* module
-with its value.
-
-<pre class="snippet">
-import "liquids" for Water //Water is now taken
-import "beverages" for Coffee, Water as H2O, Tea
-// var water = H2O.new()
-</pre>
-
 If you want to load a module, but not bind any variables from it, you can omit
 the `for` clause:
 
@@ -92,17 +81,17 @@ WrenVM* vm = wrenNewVM(&config);
 That function has this signature:
 
 <pre class="snippet" data-lang="c">
-char* WrenLoadModuleFn(WrenVM* vm, const char* name);
+WrenLoadModuleResult WrenLoadModuleFn(WrenVM* vm, const char* name);
 </pre>
 
 Whenever a module is imported, the VM calls this and passes it the name of the
 module. The embedder is expected to return the source code contents of the
-module. When you embed Wren in your app, you can handle this however you want:
-reach out to the file system, look inside resources bundled into your app,
-whatever.
+module in a `WrenLoadModuleResult`. When you embed Wren in your app, you can handle
+this however you want: reach out to the file system, look inside resources bundled
+into your app, whatever.
 
-You can return `NULL` from this function to indicate that a module couldn't be
-found. When you do this, Wren will report it as a runtime error.
+You can return the source field as `NULL` from this function to indicate that a module
+couldn't be found. When you do this, Wren will report it as a runtime error.
 
 ### The command-line loader
 

doc/site/syntax.markdown

@@ -43,7 +43,7 @@ One way to get a quick feel for a language's style is to see what words it
 reserves. Here's what Wren has:
 
 <pre class="snippet">
-as break class construct else false for foreign if import
+break class construct else false for foreign if import
 in is null return static super this true var while
 </pre>
 

src/include/wren.h

@@ -65,8 +65,26 @@ typedef void (*WrenFinalizerFn)(void* data);
 typedef const char* (*WrenResolveModuleFn)(WrenVM* vm,
     const char* importer, const char* name);
 
+// Forward declare
+typedef struct WrenLoadModuleResult WrenLoadModuleResult;
+
+// Called after loadModuleFn is called for module [name]. The original returned result
+// is handed back to you in this callback, so that you can free memory if appropriate.
+typedef void (*WrenLoadModuleCompleteFn)(WrenVM* vm, const char* name, WrenLoadModuleResult result);
+
+// The result of a loadModuleFn call. 
+// [length] is optional, a value of 0 means length is ignored.
+// [source] is the source code for the module, or NULL if the module is not found.
+// [onComplete] an optional callback that will be called once Wren is done with the result.
+typedef struct WrenLoadModuleResult
+{
+  const char* source;
+  WrenLoadModuleCompleteFn onComplete;
+  void* userData;
+} WrenLoadModuleResult;
+
 // Loads and returns the source code for the module [name].
-typedef char* (*WrenLoadModuleFn)(WrenVM* vm, const char* name);
+typedef WrenLoadModuleResult (*WrenLoadModuleFn)(WrenVM* vm, const char* name);
 
 // Returns a pointer to a foreign method on [className] in [module] with
 // [signature].

src/vm/wren_compiler.c

@@ -95,7 +95,6 @@ typedef enum
   TOKEN_FOREIGN,
   TOKEN_IF,
   TOKEN_IMPORT,
-  TOKEN_AS,
   TOKEN_IN,
   TOKEN_IS,
   TOKEN_NULL,
@@ -576,7 +575,6 @@ static Keyword keywords[] =
   {"foreign",   7, TOKEN_FOREIGN},
   {"if",        2, TOKEN_IF},
   {"import",    6, TOKEN_IMPORT},
-  {"as",        2, TOKEN_AS},
   {"in",        2, TOKEN_IN},
   {"is",        2, TOKEN_IS},
   {"null",      4, TOKEN_NULL},
@@ -2631,7 +2629,6 @@ GrammarRule rules[] =
   /* TOKEN_FOREIGN       */ UNUSED,
   /* TOKEN_IF            */ UNUSED,
   /* TOKEN_IMPORT        */ UNUSED,
-  /* TOKEN_AS            */ UNUSED,
   /* TOKEN_IN            */ UNUSED,
   /* TOKEN_IS            */ INFIX_OPERATOR(PREC_IS, "is"),
   /* TOKEN_NULL          */ PREFIX(null),
@@ -3362,38 +3359,17 @@ static void import(Compiler* compiler)
   do
   {
     ignoreNewlines(compiler);
+    int slot = declareNamedVariable(compiler);
     
-    consume(compiler, TOKEN_NAME, "Expect variable name.");
+    // Define a string constant for the variable name.
+    int variableConstant = addConstant(compiler,
+        wrenNewStringLength(compiler->parser->vm,
+                            compiler->parser->previous.start,
+                            compiler->parser->previous.length));
     
-    // We need to hold onto the source variable, 
-    // in order to reference it in the import later
-    Token sourceVariableToken = compiler->parser->previous;
-
-    // Define a string constant for the original variable name.
-    int sourceVariableConstant = addConstant(compiler,
-          wrenNewStringLength(compiler->parser->vm,
-                        sourceVariableToken.start,
-                        sourceVariableToken.length));
-
-    // Store the symbol we care about for the variable
-    int slot = -1;
-    if(match(compiler, TOKEN_AS))
-    {
-      //import "module" for Source as Dest
-      //Use 'Dest' as the name by declaring a new variable for it.
-      //This parses a name after the 'as' and defines it.
-      slot = declareNamedVariable(compiler);
-    }
-    else
-    {
-      //import "module" for Source
-      //Uses 'Source' as the name directly
-      slot = declareVariable(compiler, &sourceVariableToken);
-    }
-
     // Load the variable from the other module.
-    emitShortArg(compiler, CODE_IMPORT_VARIABLE, sourceVariableConstant);
-
+    emitShortArg(compiler, CODE_IMPORT_VARIABLE, variableConstant);
+    
     // Store the result in the variable here.
     defineVariable(compiler, slot);
   } while (match(compiler, TOKEN_COMMA));

src/vm/wren_vm.c

@@ -446,20 +446,16 @@ static ObjClosure* compileInModule(WrenVM* vm, Value name, const char* source,
   {
     module = wrenNewModule(vm, AS_STRING(name));
 
-    // It's possible for the map set call below to resize the modules map,
-    // and trigger a GC while doing so. When this happens and there are enough
-    // objects and modules, it will collect the module we've just created.
-    // wrenPushRoot isn't an option when there are enough modules to exhaust
-    // WREN_MAX_TEMP_ROOTS, however handles are also not collected by GC.
-    // Note that this only affects the module until it's in the map, because
-    // then it has a reference to it, and won't be collected.
-    WrenHandle* moduleHandle = wrenMakeHandle(vm, OBJ_VAL(module));
+    // It's possible for the wrenMapSet below to resize the modules map,
+    // and trigger a GC while doing so. When this happens it will collect
+    // the module we've just created. Once in the map it is safe.
+    wrenPushRoot(vm, (Obj*)module);
 
     // Store it in the VM's module registry so we don't load the same module
     // multiple times.
     wrenMapSet(vm, vm->modules, name, OBJ_VAL(module));
 
-    wrenReleaseHandle(vm, moduleHandle);
+    wrenPopRoot(vm);
 
     // Implicitly import the core module.
     ObjModule* coreModule = getModule(vm, NULL_VAL);
@@ -705,44 +701,39 @@ static Value importModule(WrenVM* vm, Value name)
 
   wrenPushRoot(vm, AS_OBJ(name));
 
+  WrenLoadModuleResult result = {0};
   const char* source = NULL;
-  bool allocatedSource = true;
   
   // Let the host try to provide the module.
   if (vm->config.loadModuleFn != NULL)
   {
-    source = vm->config.loadModuleFn(vm, AS_CSTRING(name));
+    result = vm->config.loadModuleFn(vm, AS_CSTRING(name));
   }
   
   // If the host didn't provide it, see if it's a built in optional module.
-  if (source == NULL)
+  if (result.source == NULL)
   {
+    result.onComplete = NULL;
     ObjString* nameString = AS_STRING(name);
 #if WREN_OPT_META
-    if (strcmp(nameString->value, "meta") == 0) source = wrenMetaSource();
+    if (strcmp(nameString->value, "meta") == 0) result.source = wrenMetaSource();
 #endif
 #if WREN_OPT_RANDOM
-    if (strcmp(nameString->value, "random") == 0) source = wrenRandomSource();
+    if (strcmp(nameString->value, "random") == 0) result.source = wrenRandomSource();
 #endif
-    
-    // TODO: Should we give the host the ability to provide strings that don't
-    // need to be freed?
-    allocatedSource = false;
   }
   
-  if (source == NULL)
+  if (result.source == NULL)
   {
     vm->fiber->error = wrenStringFormat(vm, "Could not load module '@'.", name);
     wrenPopRoot(vm); // name.
     return NULL_VAL;
   }
   
-  ObjClosure* moduleClosure = compileInModule(vm, name, source, false, true);
+  ObjClosure* moduleClosure = compileInModule(vm, name, result.source, false, true);
   
-  // Modules loaded by the host are expected to be dynamically allocated with
-  // ownership given to the VM, which will free it. The built in optional
-  // modules are constant strings which don't need to be freed.
-  if (allocatedSource) DEALLOCATE(vm, (char*)source);
+  // Now that we're done, give the result back in case there's cleanup to do.
+  if(result.onComplete) result.onComplete(vm, AS_CSTRING(name), result);
   
   if (moduleClosure == NULL)
   {

src/vm/wren_vm.h

@@ -8,7 +8,7 @@
 
 // The maximum number of temporary objects that can be made visible to the GC
 // at one time.
-#define WREN_MAX_TEMP_ROOTS 5
+#define WREN_MAX_TEMP_ROOTS 8
 
 typedef enum
 {

test/api/resolution.c

@@ -14,7 +14,12 @@ static void reportError(WrenVM* vm, WrenErrorType type,
   if (type == WREN_ERROR_RUNTIME) printf("%s\n", message);
 }
 
-static char* loadModule(WrenVM* vm, const char* module)
+static void loadModuleComplete(WrenVM* vm, const char* module, WrenLoadModuleResult result)
+{
+  free((void*)result.source);
+}
+
+static WrenLoadModuleResult loadModule(WrenVM* vm, const char* module)
 {
   printf("loading %s\n", module);
 
@@ -27,10 +32,14 @@ static char* loadModule(WrenVM* vm, const char* module)
   {
     source = "System.print(\"ok\")";
   }
-
+   
   char* string = (char*)malloc(strlen(source) + 1);
   strcpy(string, source);
-  return string;
+
+  WrenLoadModuleResult result = {0};
+    result.onComplete = loadModuleComplete;
+    result.source = string;
+  return result;
 }
 
 static void runTestVM(WrenVM* vm, WrenConfiguration* configuration,

test/language/module/import_as/import_as.wren

@@ -1,11 +0,0 @@
-var Module = "from here"
-var ValueC = "value C"
-import "./module" for ValueA, Module as Another, ValueB // expect: ran module
-import "./module" for ValueC as OtherC
-
-System.print(Module)  // expect: from here
-System.print(Another) // expect: from module
-System.print(ValueA)  // expect: module A
-System.print(ValueB)  // expect: module B
-System.print(ValueC)  // expect: value C
-System.print(OtherC)  // expect: module C

test/language/module/import_as/module.wren

@@ -1,7 +0,0 @@
-// nontest
-var Module = "from module"
-System.print("ran module")
-
-var ValueA = "module A"
-var ValueB = "module B"
-var ValueC = "module C"

test/test.c

@@ -364,7 +364,15 @@
     }
   }
 
-  char* readModule(WrenVM* vm, const char* module) 
+  void readModuleComplete(WrenVM* vm, const char* module, WrenLoadModuleResult result)
+  {
+    if (result.source) {
+      free((void*)result.source);
+      result.source = NULL;
+    }
+  }
+
+  WrenLoadModuleResult readModule(WrenVM* vm, const char* module) 
   {
 
     Path* filePath = pathNew(module);
@@ -375,8 +383,11 @@
     char* source = readFile(filePath->chars);
     pathFree(filePath);
 
-    //may or may not be null
-    return source;
+    //source may or may not be null
+    WrenLoadModuleResult result;
+      result.source = source;
+      result.onComplete = readModuleComplete;
+    return result;
 
   }
 

test/test.h

@@ -59,7 +59,7 @@ typedef struct
   PathType pathType(const char* path);
 //file helpers
   char* readFile(const char* path);
-  char* readModule(WrenVM* vm, const char* module);
+  WrenLoadModuleResult readModule(WrenVM* vm, const char* module);
 //vm helpers
   void vm_write(WrenVM* vm, const char* text);
   void reportError(WrenVM* vm, WrenErrorType type, const char* module, int line, const char* message);