Optimize shim generation for structural items

This commit removes shims, where possible, for `structural` items.
Instead of generating code that looks like:

    const target = function() { this.foo(); };
    exports.__wbg_thing = function(a) { target.call(getObject(a)); };

we now instead generate:

    exports.__wbg_thing = function(a) { getObject(a).foo(); };

Note that this only applies to `structural` bindings, all default
bindings (as of this commit) are still using imported targets to ensure
that their binding can't change after instantiation.

This change was [detailed in RFC #5][link] as an important optimization
for `structural` bindings to ensure they've got performance parity with
today's non-`structural` default bindings.

[link]: https://rustwasm.github.io/rfcs/005-structural-and-deref.html#why-is-it-ok-to-make-structural-the-default

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

@@ -1,5 +1,4 @@
 use std::collections::{HashMap, HashSet};
-use std::fmt::Write;
 use std::mem;
 
 use decode;
@@ -65,6 +64,18 @@ pub struct SubContext<'a, 'b: 'a> {
     pub vendor_prefixes: HashMap<&'b str, Vec<&'b str>>,
 }
 
+pub enum ImportTarget {
+    Function(String),
+    Method(String),
+    Constructor(String),
+    StructuralMethod(String),
+    StructuralGetter(Option<String>, String),
+    StructuralSetter(Option<String>, String),
+    StructuralIndexingGetter(Option<String>),
+    StructuralIndexingSetter(Option<String>),
+    StructuralIndexingDeleter(Option<String>),
+}
+
 const INITIAL_SLAB_VALUES: &[&str] = &["undefined", "null", "true", "false"];
 
 impl<'a> Context<'a> {
@@ -1933,7 +1944,7 @@ impl<'a, 'b> SubContext<'a, 'b> {
             Some(d) => d,
         };
 
-        let target = self.generated_import_target(info, import, &descriptor)?;
+        let target = self.generated_import_target(info, import)?;
 
         let js = Rust2Js::new(self.cx)
             .catch(import.catch)
@@ -1948,140 +1959,103 @@ impl<'a, 'b> SubContext<'a, 'b> {
         &mut self,
         info: &decode::Import<'b>,
         import: &decode::ImportFunction,
-        descriptor: &Descriptor,
+    ) -> Result<ImportTarget, Error> {
-    ) -> Result<String, Error> {
         let method_data = match &import.method {
             Some(data) => data,
             None => {
                 let name = self.import_name(info, &import.function.name)?;
-                return Ok(if name.contains(".") {
+                if import.structural || !name.contains(".") {
-                    self.cx.global(&format!(
+                    return Ok(ImportTarget::Function(name))
-                        "
+                }
-                        const {}_target = {};
+                self.cx.global(&format!("const {}_target = {};", import.shim, name));
-                        ",
+                let target = format!("{}_target", import.shim);
-                        import.shim, name
+                return Ok(ImportTarget::Function(target))
-                    ));
-                    format!("{}_target", import.shim)
-                } else {
-                    name
-                });
             }
         };
 
         let class = self.import_name(info, &method_data.class)?;
         let op = match &method_data.kind {
-            decode::MethodKind::Constructor => return Ok(format!("new {}", class)),
+            decode::MethodKind::Constructor => {
+                return Ok(ImportTarget::Constructor(class.to_string()))
+            }
             decode::MethodKind::Operation(op) => op,
         };
-        let target = if import.structural {
+        if import.structural {
-            let location = if op.is_static { &class } else { "this" };
+            let class = if op.is_static { Some(class.clone()) } else { None };
 
-            match &op.kind {
+            return Ok(match &op.kind {
                 decode::OperationKind::Regular => {
-                    let nargs = descriptor.unwrap_function().arguments.len();
+                    let name = import.function.name.to_string();
-                    let nargs = nargs - if op.is_static { 0 } else { 1 };
+                    match class {
-                    let mut s = format!("function(");
+                        Some(c) => ImportTarget::Function(format!("{}.{}", c, name)),
-                    for i in 0..nargs {
+                        None => ImportTarget::StructuralMethod(name),
-                        if i > 0 {
-                            drop(write!(s, ", "));
-                        }
-                        drop(write!(s, "x{}", i));
                     }
-                    s.push_str(") { \nreturn ");
-                    s.push_str(&location);
-                    s.push_str(".");
-                    s.push_str(&import.function.name);
-                    s.push_str("(");
-                    for i in 0..nargs {
-                        if i > 0 {
-                            drop(write!(s, ", "));
-                        }
-                        drop(write!(s, "x{}", i));
-                    }
-                    s.push_str(");\n}");
-                    s
-                }
-                decode::OperationKind::Getter(g) => format!(
-                    "function() {{
-                        return {}.{};
-                    }}",
-                    location, g
-                ),
-                decode::OperationKind::Setter(s) => format!(
-                    "function(y) {{
-                        {}.{} = y;
-                    }}",
-                    location, s
-                ),
-                decode::OperationKind::IndexingGetter => format!(
-                    "function(y) {{
-                        return {}[y];
-                    }}",
-                    location
-                ),
-                decode::OperationKind::IndexingSetter => format!(
-                    "function(y, z) {{
-                        {}[y] = z;
-                    }}",
-                    location
-                ),
-                decode::OperationKind::IndexingDeleter => format!(
-                    "function(y) {{
-                        delete {}[y];
-                    }}",
-                    location
-                ),
-            }
-        } else {
-            let target = format!("typeof {0} === 'undefined' ? null : {}{}",
-                                 class,
-                                 if op.is_static { "" } else { ".prototype" });
-            let (mut target, name) = match &op.kind {
-                decode::OperationKind::Regular => {
-                    (format!("{}.{}", target, import.function.name), &import.function.name)
                 }
                 decode::OperationKind::Getter(g) => {
-                    self.cx.expose_get_inherited_descriptor();
+                    ImportTarget::StructuralGetter(class, g.to_string())
-                    (format!(
-                        "GetOwnOrInheritedPropertyDescriptor({}, '{}').get",
-                        target, g,
-                    ), g)
                 }
                 decode::OperationKind::Setter(s) => {
-                    self.cx.expose_get_inherited_descriptor();
+                    ImportTarget::StructuralSetter(class, s.to_string())
-                    (format!(
-                        "GetOwnOrInheritedPropertyDescriptor({}, '{}').set",
-                        target, s,
-                    ), s)
                 }
                 decode::OperationKind::IndexingGetter => {
-                    panic!("indexing getter should be structural")
+                    ImportTarget::StructuralIndexingGetter(class)
                 }
                 decode::OperationKind::IndexingSetter => {
-                    panic!("indexing setter should be structural")
+                    ImportTarget::StructuralIndexingSetter(class)
                 }
                 decode::OperationKind::IndexingDeleter => {
-                    panic!("indexing deleter should be structural")
+                    ImportTarget::StructuralIndexingDeleter(class)
                 }
-            };
+            })
-            target.push_str(&format!(" || function() {{
+        }
-                throw new Error(`wasm-bindgen: {}.{} does not exist`);
+
-            }}", class, name));
+        let target = format!("typeof {0} === 'undefined' ? null : {}{}",
-            if op.is_static {
+                             class,
-                target.insert(0, '(');
+                             if op.is_static { "" } else { ".prototype" });
-                target.push_str(").bind(");
+        let (mut target, name) = match &op.kind {
-                target.push_str(&class);
+            decode::OperationKind::Regular => {
-                target.push_str(")");
+                (format!("{}.{}", target, import.function.name), &import.function.name)
+            }
+            decode::OperationKind::Getter(g) => {
+                self.cx.expose_get_inherited_descriptor();
+                (format!(
+                    "GetOwnOrInheritedPropertyDescriptor({}, '{}').get",
+                    target, g,
+                ), g)
+            }
+            decode::OperationKind::Setter(s) => {
+                self.cx.expose_get_inherited_descriptor();
+                (format!(
+                    "GetOwnOrInheritedPropertyDescriptor({}, '{}').set",
+                    target, s,
+                ), s)
+            }
+            decode::OperationKind::IndexingGetter => {
+                panic!("indexing getter should be structural")
+            }
+            decode::OperationKind::IndexingSetter => {
+                panic!("indexing setter should be structural")
+            }
+            decode::OperationKind::IndexingDeleter => {
+                panic!("indexing deleter should be structural")
             }
-            target
         };
+        target.push_str(&format!(" || function() {{
+            throw new Error(`wasm-bindgen: {}.{} does not exist`);
+        }}", class, name));
+        if op.is_static {
+            target.insert(0, '(');
+            target.push_str(").bind(");
+            target.push_str(&class);
+            target.push_str(")");
+        }
 
         self.cx.global(&format!("const {}_target = {};", import.shim, target));
-        Ok(format!(
+        Ok(if op.is_static {
-            "{}_target{}",
+            ImportTarget::Function(format!("{}_target", import.shim))
-            import.shim,
+        } else {
-            if op.is_static { "" } else { ".call" }
+            ImportTarget::Method(format!("{}_target", import.shim))
-        ))
+        })
     }
 
     fn generate_import_type(

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

@@ -1,6 +1,6 @@
-use failure::{self, Error};
+use failure::Error;
 
-use super::{Context, Js2Rust};
+use super::{Context, Js2Rust, ImportTarget};
 use descriptor::{Descriptor, Function};
 
 /// Helper struct for manufacturing a shim in JS used to translate Rust types to
@@ -499,7 +499,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
         Ok(())
     }
 
-    pub fn finish(&self, invoc: &str) -> Result<String, Error> {
+    pub fn finish(&self, invoc: &ImportTarget) -> Result<String, Error> {
         let mut ret = String::new();
         ret.push_str("function(");
         ret.push_str(&self.shim_arguments.join(", "));
@@ -512,35 +512,92 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
         ret.push_str(") {\n");
         ret.push_str(&self.prelude);
 
-        let mut invoc = if self.variadic {
+        let handle_variadic = |invoc: &str, js_arguments: &[String]| {
-            if self.js_arguments.is_empty() {
+            let ret = if self.variadic {
-                return Err(failure::err_msg(
+                let (last_arg, args) = match js_arguments.split_last() {
-                    "a function with no arguments cannot be variadic",
+                    Some(pair) => pair,
-                ));
+                    None => bail!("a function with no arguments cannot be variadic"),
-            }
+                };
-            let last_arg = self.js_arguments.len() - 1; // check implies >= 0
+                if args.len() > 0 {
-            if self.js_arguments.len() != 1 {
+                    self.ret_expr.replace(
-                self.ret_expr.replace(
+                        "JS",
-                    "JS",
+                        &format!("{}({}, ...{})", invoc, args.join(", "), last_arg),
-                    &format!(
+                    )
-                        "{}({}, ...{})",
+                } else {
-                        invoc,
+                    self.ret_expr.replace(
-                        self.js_arguments[..last_arg].join(", "),
+                        "JS",
-                        self.js_arguments[last_arg],
+                        &format!("{}(...{})", invoc, last_arg),
-                    ),
+                    )
-                )
+                }
             } else {
                 self.ret_expr.replace(
                     "JS",
-                    &format!("{}(...{})", invoc, self.js_arguments[last_arg],),
+                    &format!("{}({})", invoc, js_arguments.join(", ")),
                 )
-            }
+            };
-        } else {
+            Ok(ret)
-            self.ret_expr.replace(
-                "JS",
-                &format!("{}({})", invoc, self.js_arguments.join(", ")),
-            )
         };
+
+        let fixed = |desc: &str, class: &Option<String>, amt: usize| {
+            if self.variadic {
+                bail!("{} cannot be variadic", desc);
+            }
+            match (class, self.js_arguments.len()) {
+                (None, n) if n == amt + 1 => {
+                    Ok((self.js_arguments[0].clone(), &self.js_arguments[1..]))
+                }
+                (None, _) => bail!("setters must have {} arguments", amt + 1),
+                (Some(class), n) if n == amt => {
+                    Ok((class.clone(), &self.js_arguments[..]))
+                }
+                (Some(_), _) => bail!("static setters must have {} arguments", amt),
+            }
+        };
+
+        let mut invoc = match invoc {
+            ImportTarget::Function(f) => {
+                handle_variadic(&f, &self.js_arguments)?
+            }
+            ImportTarget::Constructor(c) => {
+                handle_variadic(&format!("new {}", c), &self.js_arguments)?
+            }
+            ImportTarget::Method(f) => {
+                handle_variadic(&format!("{}.call", f), &self.js_arguments)?
+            }
+            ImportTarget::StructuralMethod(f) => {
+                let (receiver, args) = match self.js_arguments.split_first() {
+                    Some(pair) => pair,
+                    None => bail!("methods must have at least one argument"),
+                };
+                handle_variadic(&format!("{}.{}", receiver, f), args)?
+            }
+            ImportTarget::StructuralGetter(class, field) => {
+                let (receiver, _) = fixed("getter", class, 0)?;
+                let expr = format!("{}.{}", receiver, field);
+                self.ret_expr.replace("JS", &expr)
+            }
+            ImportTarget::StructuralSetter(class, field) => {
+                let (receiver, val) = fixed("setter", class, 1)?;
+                let expr = format!("{}.{} = {}", receiver, field, val[0]);
+                self.ret_expr.replace("JS", &expr)
+            }
+            ImportTarget::StructuralIndexingGetter(class) => {
+                let (receiver, field) = fixed("indexing getter", class, 1)?;
+                let expr = format!("{}[{}]", receiver, field[0]);
+                self.ret_expr.replace("JS", &expr)
+            }
+            ImportTarget::StructuralIndexingSetter(class) => {
+                let (receiver, field) = fixed("indexing setter", class, 2)?;
+                let expr = format!("{}[{}] = {}", receiver, field[0], field[1]);
+                self.ret_expr.replace("JS", &expr)
+            }
+            ImportTarget::StructuralIndexingDeleter(class) => {
+                let (receiver, field) = fixed("indexing deleter", class, 1)?;
+                let expr = format!("delete {}[{}]", receiver, field[0]);
+                self.ret_expr.replace("JS", &expr)
+            }
+        };
+
         if self.catch {
             let catch = "\
                          const view = getUint32Memory();\n\

crates/js-sys/tests/wasm/Object.rs

@@ -129,7 +129,7 @@ fn get_own_property_descriptor() {
     let desc = Object::get_own_property_descriptor(&foo, &"foo".into());
     assert_eq!(PropertyDescriptor::from(desc).value(), 42);
     let desc = Object::get_own_property_descriptor(&foo, &"bar".into());
-    assert!(PropertyDescriptor::from(desc).value().is_undefined());
+    assert!(desc.is_undefined());
 }
 
 #[wasm_bindgen_test]

crates/js-sys/tests/wasm/Reflect.rs

@@ -114,7 +114,7 @@ fn get_own_property_descriptor() {
     let desc = Reflect::get_own_property_descriptor(&obj, &"x".into()).unwrap();
     assert_eq!(PropertyDescriptor::from(desc).value(), 10);
     let desc = Reflect::get_own_property_descriptor(&obj, &"foo".into()).unwrap();
-    assert!(PropertyDescriptor::from(desc).value().is_undefined());
+    assert!(desc.is_undefined());
 }
 
 #[wasm_bindgen_test]