From e0d96eb817922e41039aa2e6ad89cd55273acad3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 31 Jul 2019 11:35:36 -0700 Subject: [PATCH] Support emitting direct imports in wasm files Support was previously (re-)added in #1654 for importing direct JS values into a WebAssembly module by completely skipping JS shim generation. This commit takes that PR one step further by *also* embedding a direct import in the wasm file, where supported. The wasm file currently largely just imports from the JS shim file that we generate, but this allows it to directly improt from ES modules where supported and where possible. Note that like #1654 this only happens when the function signature doesn't actually require any conversions to happen in JS (such as handling closures). For imports from ES modules, local snippets, or inline JS they'll all have their import directives directly embedded into the final WebAssembly binary without any shims necessary to hook it all up. For imports from the global namespace or possibly vendor-prefixed items these still unconditionally require an import shim to be generated because there's no way to describe that import in an ES-friendly way (yet). There's a few consequences of this commit which are also worth noting: * The logic in `wasm-bindgen` where it gracefully handles (to some degree) not-defined items now only is guaranteed to be applied to the global namespace. If you import from a module, it'll be an instantiation time error rather than today's runtime error when the import is called. * Handling imports in the wasm module not registered with `#[wasm_bindgen]` has become more strict. Previously these imports were basically ignored, leaving them up for interpretation depending on the output format. The changes for each output target are: * `bundler` - not much has changed here. Previously these ignored imports would have been treated as ES module imports, and after this commit there might just be some more of these imports for bundlers to resolve. * `web` - previously the ignored imports would likely cause instantiation failures because the import object never actually included a binding for other imports. After this commit though the JS glue which instantiates the module now interprets all unrecognized wasm module imports as ES module imports, emitting an `import` directive. This matches what we want for the direct import functionality, and is also largely what we want for modules in general. * `nodejs` - previously ignored imports were handled in the translation shim for Node to generate `require` statements, so they were actually "correctly handled" sort of with module imports. The handling of this hasn't changed, and reflects what we want for direct imports of values where loading a wasm module in Node ends up translating the module field of each import to a `require`. * `no-modules` - this is very similar to the `web` target where previously this didn't really work one way or the other because we'd never fill in more fields of the import object when instantiating the module. After this PR though this is a hard-error to have unrecognized imports from `#[wasm_bindgen]` with the `no-modules` output type, because we don't know how to handle the imports. Note that this touches on #1584 and will likely break the current use case being mentioned there. I think though that this tightening up of how we handle imports is what we'll want in the long run where everything is interpreted as modules, and we'll need to figure out best how wasi fits into this. This commit is unlikely to have any real major immediate effects. The goal here is to continue to inch us towards a world where there's less and less JS glue necessary and `wasm-bindgen` is just a polyfill for web standards that otherwise all already exist. Also note that there's no explicitly added tests for this since this is largely just a refactoring of an internal implementation detail of `wasm-bindgen`, but the main `wasm` test suite has many instances of this path being taken, for example having imports like: (import "tests/wasm/duplicates_a.js" "foo" (func $__wbg_foo_969c253238f136f0 (type 1))) (import "tests/wasm/duplicates_b.js" "foo" (func $__wbg_foo_027958cb2e320a94 (type 0))) (import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "trivial" (func $__wbg_trivial_75e27c84882af23b (type 1))) (import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "incoming_bool" (func $__wbg_incomingbool_0f2d9f55f73a256f (type 0))) --- crates/cli-support/src/js/mod.rs | 126 +++++++++++++++++++++++++------ crates/cli-support/src/lib.rs | 15 ++++ 2 files changed, 118 insertions(+), 23 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index f4c0dfd4b65..f6e49b8009f 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -6,7 +6,7 @@ use crate::webidl::{AuxValue, Binding}; use crate::webidl::{JsImport, JsImportName, NonstandardWebidlSection, WasmBindgenAux}; use crate::{Bindgen, EncodeInto, OutputMode}; use failure::{bail, Error, ResultExt}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fs; use std::path::{Path, PathBuf}; use walrus::{ExportId, ImportId, MemoryId, Module}; @@ -248,7 +248,7 @@ impl<'a> Context<'a> { OutputMode::NoModules { global } => { js.push_str("const __exports = {};\n"); js.push_str("let wasm;\n"); - init = self.gen_init(needs_manual_start); + init = self.gen_init(needs_manual_start, None)?; footer.push_str(&format!( "self.{} = Object.assign(init, __exports);\n", global @@ -309,7 +309,7 @@ impl<'a> Context<'a> { // as the default export of the module. OutputMode::Web => { self.imports_post.push_str("let wasm;\n"); - init = self.gen_init(needs_manual_start); + init = self.gen_init(needs_manual_start, Some(&mut imports))?; footer.push_str("export default init;\n"); } } @@ -435,7 +435,11 @@ impl<'a> Context<'a> { ) } - fn gen_init(&mut self, needs_manual_start: bool) -> (String, String) { + fn gen_init( + &mut self, + needs_manual_start: bool, + mut imports: Option<&mut String>, + ) -> Result<(String, String), Error> { let module_name = "wbg"; let mem = self.module.memories.get(self.memory); let (init_memory1, init_memory2) = if let Some(id) = mem.import { @@ -495,6 +499,22 @@ impl<'a> Context<'a> { imports_init.push_str(";\n"); } + let extra_modules = self + .module + .imports + .iter() + .filter(|i| !self.wasm_import_definitions.contains_key(&i.id())) + .map(|i| &i.module) + .collect::>(); + for (i, extra) in extra_modules.iter().enumerate() { + let imports = match &mut imports { + Some(list) => list, + None => bail!("cannot import from modules with `--no-modules`"), + }; + imports.push_str(&format!("import * as __wbg_star{} from '{}';\n", i, extra)); + imports_init.push_str(&format!("imports['{}'] = __wbg_star{};\n", extra, i)); + } + let js = format!( "\ function init(module{init_memory_arg}) {{ @@ -553,7 +573,7 @@ impl<'a> Context<'a> { imports_init = imports_init, ); - (js, ts) + Ok((js, ts)) } fn write_classes(&mut self) -> Result<(), Error> { @@ -1104,7 +1124,8 @@ impl<'a> Context<'a> { // // If `ptr` and `len` are both `0` then that means it's `None`, in that case we rely upon // the fact that `getObject(0)` is guaranteed to be `undefined`. - self.global(" + self.global( + " function getCachedStringFromWasm(ptr, len) { if (ptr === 0) { return getObject(len); @@ -1112,7 +1133,8 @@ impl<'a> Context<'a> { return getStringFromWasm(ptr, len); } } - "); + ", + ); Ok(()) } @@ -1730,7 +1752,8 @@ impl<'a> Context<'a> { JsImportName::LocalModule { module, name } => { let unique_name = generate_identifier(name, &mut self.defined_identifiers); - add_module_import(format!("./snippets/{}", module), name, &unique_name); + let module = self.config.local_module_name(module); + add_module_import(module, name, &unique_name); unique_name } @@ -1739,11 +1762,10 @@ impl<'a> Context<'a> { snippet_idx_in_crate, name, } => { + let module = self + .config + .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); let unique_name = generate_identifier(name, &mut self.defined_identifiers); - let module = format!( - "./snippets/{}/inline{}.js", - unique_crate_identifier, snippet_idx_in_crate, - ); add_module_import(module, name, &unique_name); unique_name } @@ -2014,16 +2036,11 @@ impl<'a> Context<'a> { .types .get::(binding.webidl_ty) .unwrap(); - let js = match import { + match import { AuxImport::Value(AuxValue::Bare(js)) if !variadic && !catch && self.import_does_not_require_glue(binding, webidl) => { - self.expose_not_defined(); - let name = self.import_name(js)?; - format!( - "typeof {name} == 'function' ? {name} : notDefined('{name}')", - name = name, - ) + self.direct_import(id, js) } _ => { if assert_no_shim { @@ -2048,11 +2065,11 @@ impl<'a> Context<'a> { cx.invoke_import(&binding, import, bindings, args, variadic, prelude) }, )?; - format!("function{}", js) + self.wasm_import_definitions + .insert(id, format!("function{}", js)); + Ok(()) } - }; - self.wasm_import_definitions.insert(id, js); - Ok(()) + } } fn import_does_not_require_glue( @@ -2078,6 +2095,69 @@ impl<'a> Context<'a> { ) } + /// Emit a direct import directive that hooks up the `js` value specified to + /// the wasm import `id`. + fn direct_import(&mut self, id: ImportId, js: &JsImport) -> Result<(), Error> { + // If there's no field projection happening here and this is a direct + // import from an ES-looking module, then we can actually just hook this + // up directly in the wasm file itself. Note that this is covered in the + // various output formats as well: + // + // * `bundler` - they think wasm is an ES module anyway + // * `web` - we're sure to emit more `import` directives during + // `gen_init` and we update the import object accordingly. + // * `nodejs` - the polyfill we have for requiring a wasm file as a node + // module will naturally emit `require` directives for the module + // listed on each wasm import. + // * `no-modules` - imports aren't allowed here anyway from other + // modules and an error is generated. + if js.fields.len() == 0 { + match &js.name { + JsImportName::Module { module, name } => { + let import = self.module.imports.get_mut(id); + import.module = module.clone(); + import.name = name.clone(); + return Ok(()); + } + JsImportName::LocalModule { module, name } => { + let module = self.config.local_module_name(module); + let import = self.module.imports.get_mut(id); + import.module = module; + import.name = name.clone(); + return Ok(()); + } + JsImportName::InlineJs { + unique_crate_identifier, + snippet_idx_in_crate, + name, + } => { + let module = self + .config + .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); + let import = self.module.imports.get_mut(id); + import.module = module; + import.name = name.clone(); + return Ok(()); + } + + // Fall through below to requiring a JS shim to create an item + // that we can import. These are plucked from the global + // environment so there's no way right now to describe these + // imports in an ES module-like fashion. + JsImportName::Global { .. } | JsImportName::VendorPrefixed { .. } => {} + } + } + + self.expose_not_defined(); + let name = self.import_name(js)?; + let js = format!( + "typeof {name} == 'function' ? {name} : notDefined('{name}')", + name = name, + ); + self.wasm_import_definitions.insert(id, js); + Ok(()) + } + /// Generates a JS snippet appropriate for invoking `import`. /// /// This is generating code for `binding` where `bindings` has more type diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index 05026b10f6b..0f332f10615 100755 --- a/crates/cli-support/src/lib.rs +++ b/crates/cli-support/src/lib.rs @@ -360,6 +360,21 @@ impl Bindgen { typescript: self.typescript, }) } + + fn local_module_name(&self, module: &str) -> String { + format!("./snippets/{}", module) + } + + fn inline_js_module_name( + &self, + unique_crate_identifier: &str, + snippet_idx_in_crate: usize, + ) -> String { + format!( + "./snippets/{}/inline{}.js", + unique_crate_identifier, snippet_idx_in_crate, + ) + } } fn reset_indentation(s: &str) -> String {