Are zero-length import groups allowed in text format?

[chacailline]

Hi,

I can see that import groups with empty lists are allowed in the binary format.

To me, it looks like the text format grammar, allows empty lists. I'm not a SpecTec expert, but looking at the source, I don't see any rules that would disallow them.

However, in #17, it seems like it's not intended for the text format to recognize empty groups.

Currently, the reference interpreter rejects empty lists in compact encoding 1, but not in compact encoding 2.

❯ ./interpreter/wasm -e '(module (import "mod"))'
string:1.22-1.23: syntax error: unexpected token
❯

❯ ./interpreter/wasm -e '(module (import "mod" (func)))'
❯ 

[bvisness]

I guess as the text format is currently specified, both of your examples should be considered valid but zero-length. I would consider this either a bug in the reference interpreter, or an undesirable outcome of the current text grammar. I am strongly leaning toward the latter, particularly because I know I have forgotten that imports have two names, and have definitely typed your latter example by mistake in the past. It should be an error instead of a successful parse that results in no import.

Thanks for catching that; I will update the spec to require at least one (item) in each case.

[guybedford]

Since JavaScript imports have side effects, in the ESM Integration being able to import a module without requiring a binding is very much a use case.

It could be worth having some discussion further on that before outright considering this a bug!

[bvisness]

The way compact imports are specified, compact imports are indistinguishable from non-compact imports. The proposal makes no changes to the module AST, which is what drives the "get the imports" step in the JS API spec. We considered the alternative (and I even prototyped it) but the ramifications of changing the AST just seemed like too much. You can see a bit about this in the overview and we discussed it briefly in this meeting.

I'd be curious to know what use case you have for importing a module without requiring a binding...generally, while we do care about the behavior of "get the imports" being predictable for Proxy reasons, I don't think we've actually wanted to precisely dispatch property lookups to the imports object in that way.

[guybedford]

From a pure WebAssembly validation perspective, that makes sense - core Wasm has no semantics for missing bindings. But in the ESM Integration there are various uses for it:

1. Fixing dependency ordering: Execution order in JS is determined by the first-visited post-order pass. Being able to hint orderings without explicit usage can allow important but subtle import ordering control between dependencies.
2. Network Preloading: By including transitive dependencies (dependencies of dependencies) as explicit imports, preloaders are able to fetch those dependencies with better parallelization. This doesn't affect post-ordering because by definition these hints are early second-visits.
3. Global polyfills and side-effects: The DOM is shared global state. Being able to guarantee global mutations prior to Wasm instantiation can be beneficial (say a polyfill system)
4. Dependency injection: A dependency module does not export a binding, but rather imports a cyclic binding back from the importing module, and calls a function on it / mutates it.

While dummy import bindings can always work around the problem, the point isn't that the use case is critical in any way, but rather that it is a natural degenerate case not worth just ruling out by default.

[tlively]

rather that it is a natural degenerate case not worth just ruling out by default.

I agree with that as a general principle, and we have generally followed this principle (see for example the existence of (ref none) or empty (rec) groups). So I would say it's working as intended for the text format to support empty compact imports just like the binary format does, even if I also agree it's a bit of a footgun.

[bvisness]

General principle notwithstanding, nothing we do here will have an impact on the AST, and therefore the behavior of the JS API, unless we feel strongly enough about it to go through (some of?) the phase process again to discuss the impact of an AST change. So I think the only relevant concerns here are the ergonomics of the text format and symmetry with the binary format.

In #17 @rossberg said:

The text format will not be able to (directly) express zero-length groups, just like it already cannot express zero-length sections or zero-length local groups. That's fine I think.

While he is right about zero-length sections as far as I know, I think he is actually wrong about locals, which have '(' 'local' valtype* ')' as their abbreviation, implying that (local) alone is valid. In fact, wasm-tools successfully parses (module (func (local))), so in fact the precedent there is to allow the empty thing.

My main concern with allowing empty forms for imports is the ergonomics of the empty-2 case I mentioned earlier, where (import "foo" (func (param i32))) would be accepted. If a beginner does this, then this will be valid but mysteriously add zero imports to the module. If they try to use an identifier, like (import "foo" (func $foo (param i32))), presumably the parser will complain about the invalid identifier instead. If they succeed in validating a module and try to naively instantiate it with { foo: () => ... }, then...I guess nothing happens, but they clearly never called the function anyway, so perhaps it's irrelevant.

[guybedford]

Perhaps we could avoid the problematic case of (import "foo" (func (param i32)) with no items in the compact encoding 2 form, by just requiring that production to have one or more items, while still allowing the untyped empty case in the compact encoding 1 form.

[tlively]

I would prefer not to add special cases to the text format for ergonomic reasons, but I guess I don't feel strongly. In fact, it looks like my implementation in the Binaryen text parser already rejects the cases under discussion, even though I am arguing that they should be accepted :)

[bvisness]

After sleeping on it, my ergonomic concerns are mostly resolved by the fact that my hypothetical beginner can't even call the imported function until they fix their import statement. At this point I don't really see a strong reason to forbid groups with zero items. Someday perhaps we'll have a really nice WAT linter, and this would be a nice case for such a thing to call out.

So, I will go ahead and fix up the reference interpreter.

[bvisness]

The reference interpreter now accepts empty groups in both forms. One interesting quirk to be aware of:

(module
  (import "foo" (func (param i32)))   ;; this still implicitly declares (type (func (param i32)))
  (func (type 0))                     ;; so, this is valid
)