WASM: Support for setting an imported function's module name

[pkedy]

I need to compile a WebAssembly program that imports functions from a host that uses a module namespace other than the standard env. I enhanced go:export to support this. For example, in addition to the default export (env module)

//go:export multiply
func multiply(x, y int) int {
    return x * y;
}

You can set the module using //go:export [module] <name>

//go:export math multiply
func multiply(x, y int) int {
    return x * y;
}

Alternatively, you can use a separate comment //go:module <name> in conjunction with go:export or go:linkname. I'm happy to make any tweaks to the argument format.

I tried to run make test but got clang linker errors. I tested using go install and wasm2wat to print the imports of the compiled wasm.

This is a first step for issue #423.

[deadprogram]

Very interesting work, @pkedy

@aykevl any thoughts on this?

[aykevl]

We definitely need this. However, I'm slightly worried by adding two different ways to set the wasm module name: I would prefer to have just one. I have a slight preference for //go:wasm-module but it could be anything.
(Note: there is currently //go:export and //export that are aliases, but I'd like to avoid adding more aliases like this).

Related: golang/go#25612

[deadprogram]

Pinging to bring this PR back to life, hopefully... :)

[pkedy]

Thanks @deadprogram.

@aykevl I agree with changing the stand alone module comment to //go:wasm-module. I just want to clarify your proposal re: having just one approach. Are you are suggesting:

1. Removing the optional argument from //go:export and using only //go:wasm-module for specifying the WASM module name; or
2. Using the optional argument in //go:export and removing //go:wasm-module?

I'm good either way. Let me know. Thanks!

[deadprogram]

I think we should use //go:wasm-module to avoid collisions. If we need to change that in the futurer for some reason, we can re-open the conversation around that.

Sounds good?

[aykevl]

I agree with @deadprogram, I have a slight preference for //go:wasm-module, so let's do that.

[pkedy]

@aykevl @deadprogram Tweaks made, rebased on dev, and tested with wasm2wat` to confirm modules is being set. Have a look.

[deadprogram]

Hi @pkedy thank you for making the requested changes. Looks good to me, now squash/merging. Thanks again for your contribution!

[aykevl]

The fork by @neelance for adding support for was appears to follow the following pattern:

//go:wasmimport localname importmodule importname

Source: golang/go@master...neelance:wasi#diff-a3fd4d4a4e3586f091ff65fe86eacb13R1527

This is of course a branch and not official, but I wonder whether we should be using the same pattern?

[neelance]

Honestly I haven't put much thought into this name yet, other than making it similar to //go:linkname.

Unfortunately the Go compiler is currently written in a way that allows flags like //go:noinline to be associated with the function below, but more complex directives like //go:linkname are position independent. This is why //go:wasmimport now also has a localname parameter.

[aykevl]

@neelance thanks for your comment!

Well, to be honest I don't really like the //go:linkname design and for simplicity I decided to ignore //go:linkname magic comments in TinyGo when the local name doesn't match the function it is a comment of. This seems to work well in practice. So if there is not a good reason to require the local name for //go:wasmimport, I'd prefer if it was left out :)

If I were to vote for any format, it would be the //export or //go:export format as in this PR description with an extra module name - but only if TinyGo is not the only compiler doing that. In any case, whatever it will be in the end, I'd like to keep it consistent between different Go compilers to avoid fragmenting the ecosystem.

[neelance]

Why use //export for a WebAssembly import?

[codefromthecrypt]

@neelance fyi I opened an issue to centralize complaints about //export for imported functions, and hopefully this can centralize a way out or a decision to not make one #3147

[aykevl]

Why use //export for a WebAssembly import?

Historical reasons, that's all.