Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

Why are the core module exports completely inaccessible? #75

Open
surma opened this issue Oct 8, 2019 · 6 comments
Open

Why are the core module exports completely inaccessible? #75

surma opened this issue Oct 8, 2019 · 6 comments

Comments

@surma
Copy link

surma commented Oct 8, 2019

I am wondering why the explainer currently says that the core module exports are completely inaccessible unless the host doesn't have support for interface types or exports are explicitely re-exported. I can't follow why this restrictive approach is desirable. Can someone fill me in on the reasoning?

I admit that I don't have strong use-cases right now, but I feel like there might be situations where skipping the adapter code could be desirable for performance or monkey-patching purposes.

I'd love to somehow allow developers to access the core module exports if necessary, maybe via something like instance.coreExports or similar.

(cc @RReverser)

@devsnek
Copy link
Member

devsnek commented Oct 8, 2019

if a module uses interface types, the api it wants to expose is not the core exports, so we shouldn't automatically expose them.

@surma
Copy link
Author

surma commented Oct 8, 2019

I understand that, and I think that's why the adapted functions are exactly right in exports. I am just wondering if there should be an escape hatch to circumvent/disable adapter code if it's erroneous or something.

@RReverser
Copy link
Member

so we shouldn't automatically expose them

They're already explicitly marked with (export ...) so it seems rather odd and backwards incompatible not to expose them when engine supports interface types.

@jgravelle-google
Copy link
Contributor

We explicitly want to avoid exporting things the module author doesn't want public. The use case for the core/interface export distinction is because many adapter functions will want to interact with a module's memory, but a module may not want to allow arbitrary reads+writes to its memory. If we require memory to be exported, that makes it impossible to truly share nothing when linking, because an external module could always do totally unmodeled things to one's state.

It should be possible to re-export a core export as a no-op, but only if the module author does so. In my polyfill I model this as forwarding a core export.

The reason this is backwards-compatible is because of modeling the core module as wrapped by an external module. In any polyfill, we create a new JS object around the exports that the wasm module generates, and that doesn't need to forward every export. We then don't expose any reference to the original exports object, and it essentially becomes a (older JS style) private variable. Which is exactly what we want for our encapsulation purposes.

And it's semantically incorrect to call an interface-having wasm module without either codegen support in the engine, or a polyfill. Calling module.exports.foo("some string") is the ABI in those cases, but the core wasm ABI uses (say) i32 and can't work backwards-compatibly anyway.

@surma
Copy link
Author

surma commented Oct 8, 2019

Hm, I do understand the notion behind this. I guess I find it a bit strange that a browser without support for Interface Types (ITs) gets access to something that a browser with support for ITs cannot access.

For ITs, I predict a pattern like this will get used:

let {instance} = await WebAssembly.instantiate(/*...*/);
if(!hasSupportForInterfaceTypes) {
	const {wrap} = await import("./glue.js");
	instance = wrap(instance);
}
instance.export.highLevelFunction();

This is something that I think a developer should be able to develop and test without having to download and old version of a browser. WDYT?

(Alternatively: What about giving compile() et al a flag to disable ITs?)

@jgravelle-google
Copy link
Contributor

I was predicting a pattern more like

const { wrapIfNeedBe } = await import("./glue.js");
const { instance } = await wrapIfNeedBe(WebAssembly.instantiate(/*...*/));
instance.export.highLevelFunction();

but it's likely that we'll see both in the wild. Your version defers the glue import, which is beneficial if the browser supports it, so is probably more production-ready and will be more common.

I think having an option to pass a flag to compile would break the encapsulation way too easily. I think the best way for a developer to test their JS is to strip the IT section (nothing currently depends on the section existing, so this is always legal) and have a custom hasSupportForInterfaceTypes debug check.
In general I predict most of the testing will happen in developing glue.js, which should be a common library so most end-users shouldn't need to test the IT-polyfilling aspect. I'm perfectly ok making the tradeoff in favor of stronger encapsulation guarantees at the cost of glue.js being unconventional to write.
(I can predict changing my mind here in two ways, 1: that the work involved with stripping that section of the binary is more work than I imagine, the complexity there is likely in the setting up of a testing workflow. Or 2: that more developers prefer to write their own polyfills than I'm expecting, where now I'm expecting most people will npm install wasm-it-polyfill and the awkwardness will be amortized over the ecosystem)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants