Support polyfill fallbacks within builtin modules #46
Conversation
|
It could also be worthwhile getting together a web platform test for this behaviour to ensure implementation compliance if per-function fallbacks are desired. |
|
In the spirit of #40 this makes sense to me. This was actually what I thought I had originally specified. @jakobkummerow or @dtig How would you feel with making this change? It would loosen the validation of builtin imports so that if an individual function is not found in a builtin collection then the engine falls back to looking up the import in the importsObj. This would help people polyfill future extensions to existing builtin collections. It's not technically a breaking change, but it is a change in a proposal that we've shipped. |
|
It has always been my understanding that per-function fallback was the behavior we wanted (considering that the whole proposal is designed around polyfillability), so it's also what V8 has always implemented. I wasn't aware that the spec text said otherwise. So, +1 to fixing the spec text to be what it always should have been. To illustrate with an example, the following works just fine in V8 today: d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
var builder = new WasmModuleBuilder();
var sig_i_i = builder.addType(kSig_i_i);
var foo = builder.addImport("wasm:js-string", "foo", sig_i_i);
// Enable this to get a CompileError due to wrong signature:
// builder.addImport("wasm:js-string", "concat", sig_i_i);
builder.addFunction("main", sig_i_i).exportFunc().addBody([
kExprLocalGet, 0,
kExprCallFunction, foo,
]);
let instance = builder.instantiate({
"wasm:js-string": {
foo: (x) => 42,
}
}, { builtins: ["js-string"] });
console.log(instance.exports.main());For convenience, here's same thing as a standalone HTML file; I can see that it indeed fails with an error in Firefox: <html>
<body>
<script>
var wirebytes = new Uint8Array([
0, 97, 115, 109, 1, 0, 0, 0, 1, 6, 1, 96, 1, 127,
1, 127, 2, 22, 1, 14, 119, 97, 115, 109, 58, 106, 115, 45,
115, 116, 114, 105, 110, 103, 3, 102, 111, 111, 0, 0, 3, 2,
1, 0, 7, 8, 1, 4, 109, 97, 105, 110, 0, 1, 10, 8,
1, 6, 0, 32, 0, 16, 0, 11, 0, 14, 4, 110, 97, 109,
101, 1, 7, 1, 1, 4, 109, 97, 105, 110
]);
(async () => {
let result = await WebAssembly.instantiate(wirebytes, {
"wasm:js-string": {
foo: (x) => 42,
}
}, { builtins: ["js-string"] });
document.write(result.instance.exports.main());
})();
</script>
</body>
</html> |
|
Oh, that's good to hear. I will make the change to the spec and update Firefox. |
Currently my reading of the spec as written is that when string builtins are enabled, the read the imports algorithm will fail if a function is imported from
wasm:js-stringsthat does not exist, instead of falling back to theimportsObjfor that specific function only.This updates the algorithm to instead allow falling back to the imports object on a per-named-import basis.