Unable to import modules

[kaadmy]

I'm using the latest Git version of Wren (744147f), and when I try to import a custom module Wren fails with the message Error at 'TestClass': Module variable is already defined.

I can reliably reproduce with this code:

#include <stdio.h>
#include <string.h>

#include "vm/wren.h"

static const char *moduleString = "\n"
  "class TestClass {\n"
  "  static testFunction() {\n"
  "    System.print(\"Test function!\")\n"
  "  }\n"
  "}\n"
  "\n";

static const char *sourceString = "\n"
  "import \"io\" for Stdin\n"
  "import \"mytest\" for TestClass\n"
  "TestClass.testFunction()"
  "\n";

static void wrenWriteFn(WrenVM *vm, const char *text) {
  printf("%s", text);
}

static void wrenErrorFn(WrenVM *vm, WrenErrorType type, const char *module, int line, const char *message) {
  printf("Error from %s:%d: %s\n", module, line, message);
}

static char *wrenLoadModuleFn(WrenVM *vm, const char *name) {
  if(!strcmp(name, "mytest")) { return strdup(moduleString); }
  return strdup("");
}

int main(int argc, char **argv) {
  WrenConfiguration config;
  wrenInitConfiguration(&config);
  config.loadModuleFn = wrenLoadModuleFn;
  config.writeFn = wrenWriteFn;
  config.errorFn = wrenErrorFn;

  WrenVM *vm = wrenNewVM(&config);
  wrenInterpret(vm, NULL, sourceString);
}

The line import "io" for Stdin does not fail, so this appears to be an issue related to custom modules.

A previous version (at least a few weeks ago, probably 1-2 months) did not have this issue, so this seems to be a fairly recent regression.

Any help would be appreciated.

[ruby0x1]

This thread #479 has some details closer to the end, which is a recent change.
I'd check commits for a more concise description of the change however (I'd look but am occupied atm).

[kaadmy]

Ah you appear to be correct, module imports seem to have changed, so therefore is failure to RTFM and not a bug. Will close this to avoid any further confusion.

[ruby0x1]

I don't think the docs have been updated just yet but I could be wrong!

[kaadmy]

Yeah the website docs appear out of date currently.

[kaadmy]

This issue might be a bit relevant actually, I added a module resolve function

static const char *wrenResolveModuleFn(WrenVM *vm, const char *importer, const char *name) {
  int length = strlen(name);

  char *newname = malloc(length + 2);
  newname[0] = '!'; // This can be anything as long as there's at least one character
  newname[1] = '\0';

  strcat(newname, name);

  return newname;
}

Unless I add an extra character at the start of the module name (to make it unique) Wren refuses to load it. Is this intentional or a bug?

[ruby0x1]

I'd suggest the test folder would have all relevant cases showing the usage, and maybe @munificent is around to comment on the nature of it in master.

[kaadmy]

All the import tests in test/ seem to use the relative form (import "./foo" for Foo) for single modules, and the only uses of the short form (import "foo" for Foo) seem to be only inside subdirectories (import "foo/bar" for Foo).

Also the importer argument for the resolve function is a pointer to 0x20 for some reason (with the statement being import "mytest" for TestClass), this seemed strange so I'm not sure if it's a special value or just uninitialized.

[mhermier]

Always assume that pointers with address < 0xffff is a NULL pointer offset at first read. And there are not much special values in the VM, so I can say for sure it is a bug/null pointer.

[kaadmy]

Huh, I just tested a bit more, and my 2nd latest comment seems invnalid now.

This code

static const char *moduleString = "\n"
  "class TestClass {\n"
  "  static testFunction() {\n"
  "    System.print(\"Test function!\")\n"
  "  }\n"
  "}\n"
  "\n";

static const char *sourceString = "\n"
  "import \"io\" for Stdin\n"
  "import \"mytest\" for TestClass\n"
  "TestClass.testFunction()"
  "\n";

static const char *wrenResolveModuleFn(WrenVM *vm, const char *importer, const char *name) {
  int length = strlen(name);

  char *newname = malloc(length + 2);
  newname[0] = '!'; // This can be anything as long as there's at least one character                                                                                                                                                                                            
  newname[1] = '\0';

  strcat(newname, name);

  return newname;
}

static char *wrenLoadModuleFn(WrenVM *vm, const char *name) {
  if(!strcmp(name, "!mytest")) { return strdup(moduleString); }

  return strdup("");
}

still does not work, it seems that as long as the loadModuleFn returns a non-empty string, Wren fails to import the module for the same reason (Error at 'TestClass': Module variable is already defined.)

[mhermier]

Le dim. 9 sept. 2018 à 20:45, KaadmY <notifications@github.com> a écrit :

Huh, I just tested a bit more, and my 2nd latest comment seems invnalid now. This code static const char *moduleString = "\n" "class TestClass {\n" " static testFunction() {\n" " System.print(\"Test function!\")\n" " }\n" "}\n" "\n"; static const char *sourceString = "\n" "import \"io\" for Stdin\n" "import \"mytest\" for TestClass\n" "TestClass.testFunction()" "\n"; static const char *wrenResolveModuleFn(WrenVM *vm, const char *importer, const char *name) { int length = strlen(name); char *newname = malloc(length + 2); newname[0] = '!'; // This can be anything as long as there's at least one character newname[1] = '\0'; strcat(newname, name); return newname; } static char *wrenLoadModuleFn(WrenVM *vm, const char *name) { if(!strcmp(name, "!mytest")) { return strdup(moduleString); } Why the '!' here? return strdup(""); Why returning "" and not NULL? I wonder if this has no side effects trying

to load core.

…

} still does not work, it seems that as long as the loadModuleFn returns a non-empty string, Wren fails to import the module for the same reason (Error at 'TestClass': Module variable is already defined.) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#600 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AY7blZNUShbUT0Ddy3e9A3xb6dsEb2cXks5uZWG4gaJpZM4WgHJa> .

[kaadmy]

Yeah sorry, after posting I realized that code had some problems with it

The ! character was just a testing thing I did, but after that I realized it was moot so I think you can ignore that.

I'm not sure why I returned an empty string instead of NULL, I probably added that and forgot it was supposed to be NULL.

If I return NULL instead of an empty string, the io module can no longer be found, but I'm not sure how the io module works so I'm ignoring that for now.

Updated code:

// moduleString is unchanged
// sourceString is unchanged except the line `import "io" for Stdin` is removed

static const char *wrenResolveModuleFn(WrenVM *vm, const char *importer, const char *name) {
  printf("Resolve: %s\n", name);
  return name;
}

static char *wrenLoadModuleFn(WrenVM *vm, const char *name) {
  printf("Load: %s\n", name);
  if(!strcmp(name, "mytest")) { return strdup(moduleString); }
  return NULL;
}

And upon running I still get this:

Resolve: mytest
Load: mytest
[mytest line 2] Error at 'TestClass': Module variable is already defined.
[(null) line -1] Could not compile module 'mytest'.

[ruby0x1]

Related to the io part:

io is a module in the CLI code. The VM and the CLI are distinct parts, the CLI uses the VM in the same way that you're embedding (you're embedding the VM part, not the CLI). So, that io module doesn't exist because you told wren vm that it doesn't (by returning null). The optional modules (random and meta) would get found if they were enabled and you return null, nothing else exists to the vm in embedding.

You can of course bring over the bindings from the cli etc if you wanted them, though. It'd be a bit of extra work, depending on your goals!

[kaadmy]

Ah okay, so the io module working properly was a bug I caused by returning an empty string and not NULL.

[kaadmy]

Did some digging, it works fine with commit b2b2b8a but fails with the next commit (fe7e56d)

Unfortunately the failing commit is a rather large merge, so it may take time to find a specific way to fix this.

[kaadmy]

So by changing this line in my main source file from

wrenInterpret(vm, NULL, sourceString);

to

wrenInterpret(vm, "main", sourceString);

all the import issues seem to no longer exist. I just checked the existing tests and this seems to be the proper method, and I was using wrenInterpret incorrectly.

[ruby0x1]

glad you figured that out, we should probably add some assertions or validations about what that parameter being null means.