Merge pull request #1654 from fitzgen/no-import-shims

Skip generating JS import shims when unnecessary

crates/backend/src/ast.rs

@@ -122,6 +122,7 @@ pub struct ImportFunction {
     pub catch: bool,
     pub variadic: bool,
     pub structural: bool,
+    pub assert_no_shim: bool,
     pub kind: ImportFunctionKind,
     pub shim: Ident,
     pub doc_comment: Option<String>,

crates/backend/src/encode.rs

@@ -272,6 +272,7 @@ fn shared_import_function<'a>(
         shim: intern.intern(&i.shim),
         catch: i.catch,
         method,
+        assert_no_shim: i.assert_no_shim,
         structural: i.structural,
         function: shared_function(&i.function, intern),
         variadic: i.variadic,

crates/cli-support/Cargo.toml

@@ -23,4 +23,4 @@ wasm-bindgen-anyref-xform = { path = '../anyref-xform', version = '=0.2.48' }
 wasm-bindgen-shared = { path = "../shared", version = '=0.2.48' }
 wasm-bindgen-threads-xform = { path = '../threads-xform', version = '=0.2.48' }
 wasm-bindgen-wasm-interpreter = { path = "../wasm-interpreter", version = '=0.2.48' }
-wasm-webidl-bindings = "0.1.0"
+wasm-webidl-bindings = "0.1.2"

crates/cli-support/src/js/mod.rs

@@ -1,5 +1,6 @@
 use crate::descriptor::VectorKind;
 use crate::intrinsic::Intrinsic;
+use crate::webidl;
 use crate::webidl::{AuxEnum, AuxExport, AuxExportKind, AuxImport, AuxStruct};
 use crate::webidl::{AuxValue, Binding};
 use crate::webidl::{JsImport, JsImportName, NonstandardWebidlSection, WasmBindgenAux};
@@ -284,7 +285,10 @@ impl<'a> Context<'a> {
             | OutputMode::Node {
                 experimental_modules: true,
             } => {
-                imports.push_str(&format!("import * as wasm from './{}_bg.wasm';\n", module_name));
+                imports.push_str(&format!(
+                    "import * as wasm from './{}_bg.wasm';\n",
+                    module_name
+                ));
                 for (id, js) in sorted_iter(&self.wasm_import_definitions) {
                     let import = self.module.imports.get_mut(*id);
                     import.module = format!("./{}.js", module_name);
@@ -723,6 +727,15 @@ impl<'a> Context<'a> {
         self.global("function getObject(idx) { return heap[idx]; }");
     }
 
+    fn expose_not_defined(&mut self) {
+        if !self.should_write_global("not_defined") {
+            return;
+        }
+        self.global(
+            "function notDefined(what) { return () => { throw new Error(`${what} is not defined`); }; }"
+        );
+    }
+
     fn expose_assert_num(&mut self) {
         if !self.should_write_global("assert_num") {
             return;
@@ -1826,7 +1839,8 @@ impl<'a> Context<'a> {
         for (id, import) in sorted_iter(&aux.import_map) {
             let variadic = aux.imports_with_variadic.contains(&id);
             let catch = aux.imports_with_catch.contains(&id);
-            self.generate_import(*id, import, bindings, variadic, catch)
+            let assert_no_shim = aux.imports_with_assert_no_shim.contains(&id);
+            self.generate_import(*id, import, bindings, variadic, catch, assert_no_shim)
                 .with_context(|_| {
                     format!("failed to generate bindings for import `{:?}`", import,)
                 })?;
@@ -1917,7 +1931,7 @@ impl<'a> Context<'a> {
         let js_doc = builder.js_doc_comments();
         let docs = format_doc_comments(&export.comments, Some(js_doc));
 
-        // Once we've got all the JS then put it in the right location dependin
+        // Once we've got all the JS then put it in the right location depending
         // on what's being exported.
         match &export.kind {
             AuxExportKind::Function(name) => {
@@ -1965,22 +1979,77 @@ impl<'a> Context<'a> {
         bindings: &NonstandardWebidlSection,
         variadic: bool,
         catch: bool,
+        assert_no_shim: bool,
     ) -> Result<(), Error> {
         let binding = &bindings.imports[&id];
         let webidl = bindings
             .types
             .get::<ast::WebidlFunction>(binding.webidl_ty)
             .unwrap();
-        let mut builder = binding::Builder::new(self);
-        builder.catch(catch)?;
-        let js = builder.process(&binding, &webidl, false, &None, &mut |cx, prelude, args| {
-            cx.invoke_import(&binding, import, bindings, args, variadic, prelude)
-        })?;
-        let js = format!("function{}", js);
+        let js = 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,
+                )
+            }
+            _ => {
+                if assert_no_shim {
+                    panic!(
+                        "imported function was annotated with `#[wasm_bindgen(assert_no_shim)]` \
+                         but we need to generate a JS shim for it:\n\n\
+                         \timport = {:?}\n\n\
+                         \tbinding = {:?}\n\n\
+                         \twebidl = {:?}",
+                        import, binding, webidl,
+                    );
+                }
+
+                let mut builder = binding::Builder::new(self);
+                builder.catch(catch)?;
+                let js = builder.process(
+                    &binding,
+                    &webidl,
+                    false,
+                    &None,
+                    &mut |cx, prelude, args| {
+                        cx.invoke_import(&binding, import, bindings, args, variadic, prelude)
+                    },
+                )?;
+                format!("function{}", js)
+            }
+        };
         self.wasm_import_definitions.insert(id, js);
         Ok(())
     }
 
+    fn import_does_not_require_glue(
+        &self,
+        binding: &Binding,
+        webidl: &ast::WebidlFunction,
+    ) -> bool {
+        if !self.config.anyref && binding.contains_anyref(self.module) {
+            return false;
+        }
+
+        let wasm_ty = self.module.types.get(binding.wasm_ty);
+        webidl.kind == ast::WebidlFunctionKind::Static
+            && webidl::outgoing_do_not_require_glue(
+                &binding.outgoing,
+                wasm_ty.params(),
+                &webidl.params,
+            )
+            && webidl::incoming_do_not_require_glue(
+                &binding.incoming,
+                &webidl.result.into_iter().collect::<Vec<_>>(),
+                wasm_ty.results(),
+            )
+    }
+
     /// Generates a JS snippet appropriate for invoking `import`.
     ///
     /// This is generating code for `binding` where `bindings` has more type
@@ -2060,7 +2129,7 @@ impl<'a> Context<'a> {
                 ast::WebidlFunctionKind::Static => {
                     let js = match val {
                         AuxValue::Bare(js) => self.import_name(js)?,
-                        _ => bail!("invalid import set for constructor"),
+                        _ => bail!("invalid import set for free function"),
                     };
                     Ok(format!("{}({})", js, variadic_args(&args)?))
                 }

crates/cli-support/src/webidl/mod.rs

@@ -120,6 +120,17 @@ pub struct Binding {
     pub return_via_outptr: Option<Vec<walrus::ValType>>,
 }
 
+impl Binding {
+    /// Does this binding's wasm function signature have any `anyref`s?
+    pub fn contains_anyref(&self, module: &walrus::Module) -> bool {
+        let ty = module.types.get(self.wasm_ty);
+        ty.params()
+            .iter()
+            .chain(ty.results())
+            .any(|ty| *ty == walrus::ValType::Anyref)
+    }
+}
+
 /// A synthetic custom section which is not standardized, never will be, and
 /// cannot be serialized or parsed. This is synthesized from all of the
 /// compiler-emitted wasm-bindgen sections and then immediately removed to be
@@ -152,6 +163,7 @@ pub struct WasmBindgenAux {
     /// Small bits of metadata about imports.
     pub imports_with_catch: HashSet<ImportId>,
     pub imports_with_variadic: HashSet<ImportId>,
+    pub imports_with_assert_no_shim: HashSet<ImportId>,
 
     /// Auxiliary information to go into JS/TypeScript bindings describing the
     /// exported enums from Rust.
@@ -782,6 +794,7 @@ impl<'a> Context<'a> {
             method,
             structural,
             function,
+            assert_no_shim,
         } = function;
         let (import_id, _id) = match self.function_imports.get(*shim) {
             Some(pair) => *pair,
@@ -800,6 +813,9 @@ impl<'a> Context<'a> {
         if *catch {
             self.aux.imports_with_catch.insert(import_id);
         }
+        if *assert_no_shim {
+            self.aux.imports_with_assert_no_shim.insert(import_id);
+        }
 
         // Perform two functions here. First we're saving off our WebIDL
         // bindings signature, indicating what we think our import is going to
@@ -1428,3 +1444,49 @@ fn concatenate_comments(comments: &[&str]) -> String {
         .collect::<Vec<_>>()
         .join("\n")
 }
+
+/// Do we need to generate JS glue shims for these incoming bindings?
+pub fn incoming_do_not_require_glue(
+    exprs: &[NonstandardIncoming],
+    from_webidl_tys: &[ast::WebidlTypeRef],
+    to_wasm_tys: &[walrus::ValType],
+) -> bool {
+    exprs.len() == from_webidl_tys.len()
+        && exprs.len() == to_wasm_tys.len()
+        && exprs
+            .iter()
+            .zip(from_webidl_tys)
+            .zip(to_wasm_tys)
+            .enumerate()
+            .all(|(i, ((expr, from_webidl_ty), to_wasm_ty))| match expr {
+                NonstandardIncoming::Standard(e) => e.is_expressible_in_js_without_webidl_bindings(
+                    *from_webidl_ty,
+                    *to_wasm_ty,
+                    i as u32,
+                ),
+                _ => false,
+            })
+}
+
+/// Do we need to generate JS glue shims for these outgoing bindings?
+pub fn outgoing_do_not_require_glue(
+    exprs: &[NonstandardOutgoing],
+    from_wasm_tys: &[walrus::ValType],
+    to_webidl_tys: &[ast::WebidlTypeRef],
+) -> bool {
+    exprs.len() == from_wasm_tys.len()
+        && exprs.len() == to_webidl_tys.len()
+        && exprs
+            .iter()
+            .zip(from_wasm_tys)
+            .zip(to_webidl_tys)
+            .enumerate()
+            .all(|(i, ((expr, from_wasm_ty), to_webidl_ty))| match expr {
+                NonstandardOutgoing::Standard(e) => e.is_expressible_in_js_without_webidl_bindings(
+                    *from_wasm_ty,
+                    *to_webidl_ty,
+                    i as u32,
+                ),
+                _ => false,
+            })
+}

crates/cli-support/src/webidl/outgoing.rs

@@ -200,8 +200,12 @@ impl OutgoingBuilder<'_> {
             Descriptor::U16 => self.standard_as(ValType::I32, ast::WebidlScalarType::UnsignedShort),
             Descriptor::I32 => self.standard_as(ValType::I32, ast::WebidlScalarType::Long),
             Descriptor::U32 => self.standard_as(ValType::I32, ast::WebidlScalarType::UnsignedLong),
-            Descriptor::F32 => self.standard_as(ValType::F32, ast::WebidlScalarType::Float),
-            Descriptor::F64 => self.standard_as(ValType::F64, ast::WebidlScalarType::Double),
+            Descriptor::F32 => {
+                self.standard_as(ValType::F32, ast::WebidlScalarType::UnrestrictedFloat)
+            }
+            Descriptor::F64 => {
+                self.standard_as(ValType::F64, ast::WebidlScalarType::UnrestrictedDouble)
+            }
             Descriptor::Enum { .. } => self.standard_as(ValType::I32, ast::WebidlScalarType::Long),
 
             Descriptor::Char => {

crates/macro-support/src/parser.rs

@@ -52,6 +52,9 @@ macro_rules! attrgen {
             (typescript_custom_section, TypescriptCustomSection(Span)),
             (start, Start(Span)),
             (skip, Skip(Span)),
+
+            // For testing purposes only.
+            (assert_no_shim, AssertNoShim(Span)),
         }
     };
 }
@@ -495,8 +498,10 @@ impl<'a> ConvertToAst<(BindgenAttrs, &'a ast::ImportModule)> for syn::ForeignIte
                 return Err(Diagnostic::span_error(*span, msg));
             }
         }
+        let assert_no_shim = opts.assert_no_shim().is_some();
         let ret = ast::ImportKind::Function(ast::ImportFunction {
             function: wasm,
+            assert_no_shim,
             kind,
             js_ret,
             catch,

crates/shared/src/lib.rs

@@ -44,6 +44,7 @@ macro_rules! shared_api {
             shim: &'a str,
             catch: bool,
             variadic: bool,
+            assert_no_shim: bool,
             method: Option<MethodData<'a>>,
             structural: bool,
             function: Function<'a>,

crates/webidl/src/util.rs

@@ -314,6 +314,7 @@ impl<'src> FirstPassRecord<'src> {
             variadic,
             catch,
             structural,
+            assert_no_shim: false,
             shim: {
                 let ns = match kind {
                     ast::ImportFunctionKind::Normal => "",

tests/wasm/main.rs

@@ -29,6 +29,7 @@ pub mod imports;
 pub mod js_objects;
 pub mod jscast;
 pub mod math;
+pub mod no_shims;
 pub mod node;
 pub mod option;
 pub mod optional_primitives;

tests/wasm/math.rs

@@ -10,15 +10,15 @@ extern "C" {
     // return value is always `f64` to faithfully capture what was sent to JS
     // (what we're interested in) because all JS numbers fit in `f64` anyway.
     // This is testing what happens when we pass numbers to JS and what it sees.
-    #[wasm_bindgen(js_name = roundtrip)]
+    #[wasm_bindgen(assert_no_shim, js_name = roundtrip)]
     fn roundtrip_i8(a: i8) -> f64;
-    #[wasm_bindgen(js_name = roundtrip)]
+    #[wasm_bindgen(assert_no_shim, js_name = roundtrip)]
     fn roundtrip_i16(a: i16) -> f64;
-    #[wasm_bindgen(js_name = roundtrip)]
+    #[wasm_bindgen(assert_no_shim, js_name = roundtrip)]
     fn roundtrip_i32(a: i32) -> f64;
-    #[wasm_bindgen(js_name = roundtrip)]
+    #[wasm_bindgen(assert_no_shim, js_name = roundtrip)]
     fn roundtrip_u8(a: u8) -> f64;
-    #[wasm_bindgen(js_name = roundtrip)]
+    #[wasm_bindgen(assert_no_shim, js_name = roundtrip)]
     fn roundtrip_u16(a: u16) -> f64;
     #[wasm_bindgen(js_name = roundtrip)]
     fn roundtrip_u32(a: u32) -> f64;

tests/wasm/no_shims.rs

@@ -0,0 +1,152 @@
+//! A collection of tests to exercise imports where we don't need to generate a
+//! JS shim to convert arguments/returns even when Web IDL bindings is not
+//! implemented.
+
+use wasm_bindgen::prelude::*;
+use wasm_bindgen_test::*;
+
+#[wasm_bindgen(inline_js = "
+    function assert_eq(a, b) {
+        if (a !== b) {
+            throw new Error(`assert_eq failed: ${a} != ${b}`);
+        }
+    }
+
+    module.exports.trivial = function () {};
+
+    module.exports.incoming_bool = function () { return true; };
+    module.exports.incoming_u8 = function () { return 255; };
+    module.exports.incoming_i8 = function () { return -127; };
+    module.exports.incoming_u16 = function () { return 65535; };
+    module.exports.incoming_i16 = function () { return 32767; };
+    module.exports.incoming_u32 = function () { return 4294967295; };
+    module.exports.incoming_i32 = function () { return 0; };
+    module.exports.incoming_f32 = function () { return 1.5; };
+    module.exports.incoming_f64 = function () { return 13.37; };
+
+    module.exports.outgoing_u8 = function (k) { assert_eq(k, 255); };
+    module.exports.outgoing_i8 = function (i) { assert_eq(i, -127); };
+    module.exports.outgoing_u16 = function (l) { assert_eq(l, 65535); };
+    module.exports.outgoing_i16 = function (j) { assert_eq(j, 32767); };
+    module.exports.outgoing_i32 = function (x) { assert_eq(x, 0); };
+    module.exports.outgoing_f32 = function (y) { assert_eq(y, 1.5); };
+    module.exports.outgoing_f64 = function (z) { assert_eq(z, 13.37); };
+
+    module.exports.many = function (x, y, z) {
+        assert_eq(x, 0);
+        assert_eq(y, 1.5);
+        assert_eq(z, 13.37);
+        return 42;
+    };
+
+    module.exports.works_when_anyref_support_is_enabled = function (v) {
+        assert_eq(v, 'hello');
+        return v;
+    };
+
+    module.exports.MyNamespace = {};
+    module.exports.MyNamespace.incoming_namespaced = function () { return 3.14; };
+    module.exports.MyNamespace.outgoing_namespaced = function (pi) { assert_eq(3.14, pi); };
+")]
+extern "C" {
+    #[wasm_bindgen(assert_no_shim)]
+    fn trivial();
+
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_bool() -> bool;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_u8() -> u8;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_i8() -> i8;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_u16() -> u16;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_i16() -> i16;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_u32() -> u32;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_i32() -> i32;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_f32() -> f32;
+    #[wasm_bindgen(assert_no_shim)]
+    fn incoming_f64() -> f64;
+
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_u8(k: u8);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_i8(i: i8);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_u16(l: u16);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_i16(j: i16);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_i32(x: i32);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_f32(y: f32);
+    #[wasm_bindgen(assert_no_shim)]
+    fn outgoing_f64(z: f64);
+
+    #[wasm_bindgen(assert_no_shim)]
+    fn many(x: i32, y: f32, z: f64) -> i32;
+
+    #[wasm_bindgen(assert_no_shim, js_namespace = MyNamespace)]
+    fn incoming_namespaced() -> f64;
+    #[wasm_bindgen(assert_no_shim, js_namespace = MyNamespace)]
+    fn outgoing_namespaced(x: f64);
+
+    // Note that this should only skip the JS shim if we have anyref support
+    // enabled.
+    //
+    // #[wasm_bindgen(assert_no_shim)]
+    fn works_when_anyref_support_is_enabled(v: JsValue) -> JsValue;
+}
+
+#[wasm_bindgen_test]
+fn no_shims() {
+    trivial();
+
+    let k = incoming_u8();
+    assert_eq!(k, 255);
+    outgoing_u8(k);
+
+    let l = incoming_u16();
+    assert_eq!(l, 65535);
+    outgoing_u16(l);
+
+    let m = incoming_u32();
+    assert_eq!(m, 4294967295);
+
+    let i = incoming_i8();
+    assert_eq!(i, -127);
+    outgoing_i8(i);
+
+    let j = incoming_i16();
+    assert_eq!(j, 32767);
+    outgoing_i16(j);
+
+    let x = incoming_i32();
+    assert_eq!(x, 0);
+    outgoing_i32(x);
+
+    let y = incoming_f32();
+    assert_eq!(y, 1.5);
+    outgoing_f32(y);
+
+    let z = incoming_f64();
+    assert_eq!(z, 13.37);
+    outgoing_f64(z);
+
+    let w = many(x, y, z);
+    assert_eq!(w, 42);
+
+    let pi = incoming_namespaced();
+    assert_eq!(pi, 3.14);
+    outgoing_namespaced(pi);
+
+    let b = incoming_bool();
+    assert!(b);
+
+    let v = JsValue::from("hello");
+    let vv = works_when_anyref_support_is_enabled(v.clone());
+    assert_eq!(v, vv);
+}