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
+host should return the source code for that module in a `WrenLoadModuleResult` struct.
-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.
+<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

@@ -81,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:
+module in a `WrenLoadModuleResult`. When you embed Wren in your app, you can handle
-reach out to the file system, look inside resources bundled into your app,
+this however you want: reach out to the file system, look inside resources bundled
-whatever.
+into your app, whatever.
 
-You can return `NULL` from this function to indicate that a module couldn't be
+You can return the source field as `NULL` from this function to indicate that a module
-found. When you do this, Wren will report it as a runtime error.
+couldn't be found. When you do this, Wren will report it as a runtime error.
 
 ### The command-line loader
 

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_vm.c

@@ -701,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
+  // Now that we're done, give the result back in case there's cleanup to do.
-  // ownership given to the VM, which will free it. The built in optional
+  if(result.onComplete) result.onComplete(vm, AS_CSTRING(name), result);
-  // modules are constant strings which don't need to be freed.
-  if (allocatedSource) DEALLOCATE(vm, (char*)source);
   
   if (moduleClosure == NULL)
   {

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/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
+    //source may or may not be null
-    return source;
+    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);