Merge pull request #132 from rustwasm/closure-more-fun-types

Support closures with "rich" arguments

README.md

@@ -399,10 +399,8 @@ booted.then(main);
 
 ## Closures
 
-The `#[wasm_bindgen]` attribute supports a limited subset of Rust closures being
+The `#[wasm_bindgen]` attribute supports some Rust closures being passed to JS.
-passed to JS at this time. There are plans to expand this support currently but
+Examples of what you can do are:
-it's not clear how to proceed unfortunately. In any case some examples of what
-you can do are:
 
 ```rust
 #[wasm_bindgen]
@@ -416,26 +414,23 @@ closure*. You can call this function with a `&Fn()` argument and JS will receive
 a JS function. When the `foo` function returns, however, the JS function will be
 invalidated and any future usage of it will raise an exception.
 
-Closures also support arguments and return values native to the wasm type
+Closures also support arguments and return values like exports do, for example:
-system, aka f32/u32:
 
 ```rust
 #[wasm_bindgen]
 extern {
-    fn bar(a: &Fn(u32, f32) -> f64);
+    type Foo;
+
+    fn bar(a: &Fn(u32, String) -> Foo);
 }
 ```
 
-At this time [types like strings aren't supported][cbstr] unfortunately.
-
-[cbstr]: https://github.com/rustwasm/wasm-bindgen/issues/104
-
 Sometimes the stack behavior of these closures is not desired. For example you'd
 like to schedule a closure to be run on the next turn of the event loop in JS
 through `setTimeout`. For this you want the imported function to return but the
 JS closure still needs to be valid!
 
-To support this use case you can also do:
+To support this use case you can do:
 
 ```rust
 use wasm_bindgen::prelude::*;
@@ -463,8 +458,6 @@ extern {
 }
 ```
 
-Like stack closures, however, only wasm types like u32/f32 are supported today.
-
 At this time you cannot [pass a JS closure to Rust][cbjs], you can only pass a
 Rust closure to JS in limited circumstances.
 

crates/cli-support/src/descriptor.rs

@@ -48,7 +48,7 @@ pub enum Descriptor {
     F64,
     Boolean,
     Function(Box<Function>),
-    Closure(Box<Function>),
+    Closure(Box<Function>, bool),
     Ref(Box<Descriptor>),
     RefMut(Box<Descriptor>),
     Slice(Box<Descriptor>),
@@ -101,8 +101,9 @@ impl Descriptor {
             BOOLEAN => Descriptor::Boolean,
             FUNCTION => Descriptor::Function(Box::new(Function::decode(data))),
             CLOSURE => {
+                let mutable = get(data) == REFMUT;
                 assert_eq!(get(data), FUNCTION);
-                Descriptor::Closure(Box::new(Function::decode(data)))
+                Descriptor::Closure(Box::new(Function::decode(data)), mutable)
             }
             REF => Descriptor::Ref(Box::new(Descriptor::_decode(data))),
             REFMUT => Descriptor::RefMut(Box::new(Descriptor::_decode(data))),
@@ -152,16 +153,16 @@ impl Descriptor {
         }
     }
 
-    pub fn ref_closure(&self) -> Option<&Function> {
+    pub fn ref_closure(&self) -> Option<(&Function, bool)> {
         match *self {
             Descriptor::Ref(ref s) => s.closure(),
             _ => None,
         }
     }
 
-    pub fn closure(&self) -> Option<&Function> {
+    pub fn closure(&self) -> Option<(&Function, bool)> {
         match *self {
-            Descriptor::Closure(ref s) => Some(s),
+            Descriptor::Closure(ref s, mutable) => Some((s, mutable)),
             _ => None,
         }
     }

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

@@ -0,0 +1,300 @@
+use super::Context;
+use descriptor::{Descriptor, Function};
+
+/// Helper struct for manfuacturing a shim in JS used to translate JS types to
+/// Rust, aka pass from JS back into Rust
+pub struct Js2Rust<'a, 'b: 'a> {
+    cx: &'a mut Context<'b>,
+
+    /// Arguments passed to the invocation of the wasm function, aka things that
+    /// are only numbers.
+    rust_arguments: Vec<String>,
+
+    /// Arguments and their types to the JS shim.
+    js_arguments: Vec<(String, String)>,
+
+    /// Conversions that happen before we invoke the wasm function, such as
+    /// converting a string to a ptr/length pair.
+    prelude: String,
+
+    /// "Destructors" or cleanup that must happen after the wasm function
+    /// finishes. This is scheduled in a `finally` block.
+    finally: String,
+
+    /// Next global index to write to when passing arguments via the single
+    /// global stack.
+    global_idx: usize,
+
+    /// Index of the next argument for unique name generation purposes.
+    arg_idx: usize,
+
+    /// Typescript expression representing the type of the return value of this
+    /// function.
+    ret_ty: String,
+
+    /// Expression used to generate the return value. The string "RET" in this
+    /// expression is replaced with the actual wasm invocation eventually.
+    ret_expr: String,
+
+    /// Name of the JS shim/function that we're generating, primarily for
+    /// TypeScript right now.
+    js_name: String,
+}
+
+impl<'a, 'b> Js2Rust<'a, 'b> {
+    pub fn new(js_name: &str, cx: &'a mut Context<'b>) -> Js2Rust<'a, 'b> {
+        Js2Rust {
+            cx,
+            js_name: js_name.to_string(),
+            rust_arguments: Vec::new(),
+            js_arguments: Vec::new(),
+            prelude: String::new(),
+            finally: String::new(),
+            global_idx: 0,
+            arg_idx: 0,
+            ret_ty: String::new(),
+            ret_expr: String::new(),
+        }
+    }
+
+    /// Generates all bindings necessary for the signature in `Function`,
+    /// creating necessary argument conversions and return value processing.
+    pub fn process(&mut self, function: &Function) -> &mut Self {
+        for arg in function.arguments.iter() {
+            self.argument(arg);
+        }
+        self.ret(&function.ret);
+        self
+    }
+
+    /// Flag this shim as a method call into Rust, so the first Rust argument
+    /// passed should be `this.ptr`.
+    pub fn method(&mut self, method: bool) -> &mut Self {
+        if method {
+            self.rust_arguments.insert(0, "this.ptr".to_string());
+        }
+        self
+    }
+
+    /// Add extra processing to the prelude of this shim.
+    pub fn prelude(&mut self, s: &str) -> &mut Self {
+        self.prelude.push_str(s);
+        self
+    }
+
+    /// Add extra processing to the finally block of this shim.
+    pub fn finally(&mut self, s: &str) -> &mut Self {
+        self.finally.push_str(s);
+        self
+    }
+
+    /// Add an Rust argument to be passed manually.
+    pub fn rust_argument(&mut self, s: &str) -> &mut Self {
+        self.rust_arguments.push(s.to_string());
+        self
+    }
+
+    fn argument(&mut self, arg: &Descriptor) {
+        let i = self.arg_idx;
+        self.arg_idx += 1;
+        let name = format!("arg{}", i);
+
+        if let Some(kind) = arg.vector_kind() {
+            self.js_arguments.push((name.clone(), kind.js_ty().to_string()));
+
+            let func = self.cx.pass_to_wasm_function(kind);
+            self.cx.expose_set_global_argument();
+            let global_idx = self.global_idx();
+            self.prelude.push_str(&format!("\
+                const [ptr{i}, len{i}] = {func}({arg});
+                setGlobalArgument(len{i}, {global_idx});
+            ", i = i, func = func, arg = name, global_idx = global_idx));
+            if arg.is_by_ref() {
+                self.finally.push_str(&format!("\n\
+                    wasm.__wbindgen_free(ptr{i}, len{i} * {size});\n\
+                ", i = i, size = kind.size()));
+                self.cx.required_internal_exports.insert(
+                    "__wbindgen_free",
+                );
+            }
+            self.rust_arguments.push(format!("ptr{}", i));
+            return
+        }
+
+        if let Some(s) = arg.rust_struct() {
+            self.js_arguments.push((name.clone(), s.to_string()));
+
+            if self.cx.config.debug {
+                self.cx.expose_assert_class();
+                self.prelude.push_str(&format!("\
+                    _assertClass({arg}, {struct_});
+                ", arg = name, struct_ = s));
+            }
+
+            if arg.is_by_ref() {
+                self.rust_arguments.push(format!("{}.ptr", name));
+            } else {
+                self.prelude.push_str(&format!("\
+                    const ptr{i} = {arg}.ptr;
+                    {arg}.ptr = 0;
+                ", i = i, arg = name));
+                self.rust_arguments.push(format!("ptr{}", i));
+            }
+            return
+        }
+
+        if arg.is_number() {
+            self.js_arguments.push((name.clone(), "number".to_string()));
+
+            if self.cx.config.debug {
+                self.cx.expose_assert_num();
+                self.prelude.push_str(&format!("_assertNum({});\n", name));
+            }
+
+            self.rust_arguments.push(name);
+            return
+        }
+
+        if arg.is_ref_anyref() {
+            self.js_arguments.push((name.clone(), "any".to_string()));
+            self.cx.expose_borrowed_objects();
+            self.finally.push_str("stack.pop();\n");
+            self.rust_arguments.push(format!("addBorrowedObject({})", name));
+            return
+        }
+
+        match *arg {
+            Descriptor::Boolean => {
+                self.js_arguments.push((name.clone(), "boolean".to_string()));
+                if self.cx.config.debug {
+                    self.cx.expose_assert_bool();
+                    self.prelude.push_str(&format!("\
+                        _assertBoolean({name});
+                    ", name = name));
+                }
+                self.rust_arguments.push(format!("arg{i} ? 1 : 0", i = i));
+            }
+            Descriptor::Anyref => {
+                self.js_arguments.push((name.clone(), "any".to_string()));
+                self.cx.expose_add_heap_object();
+                self.rust_arguments.push(format!("addHeapObject({})", name));
+            }
+            _ => {
+                panic!("unsupported argument to rust function {:?}", arg)
+            }
+        }
+    }
+
+    fn ret(&mut self, ret: &Option<Descriptor>) {
+        let ty = match *ret {
+            Some(ref t) => t,
+            None => {
+                self.ret_ty = "void".to_string();
+                self.ret_expr = format!("return RET;");
+                return
+            }
+        };
+
+        if ty.is_ref_anyref() {
+            self.ret_ty = "any".to_string();
+            self.cx.expose_get_object();
+            self.ret_expr = format!("return getObject(RET);");
+            return
+        }
+
+        if ty.is_by_ref() {
+            panic!("cannot return references from Rust to JS yet")
+        }
+
+        if let Some(ty) = ty.vector_kind() {
+            self.ret_ty = ty.js_ty().to_string();
+            let f = self.cx.expose_get_vector_from_wasm(ty);
+            self.cx.expose_get_global_argument();
+            self.cx.required_internal_exports.insert("__wbindgen_free");
+            self.ret_expr = format!("
+                const ret = RET;
+                const len = getGlobalArgument(0);
+                const realRet = {}(ret, len);
+                wasm.__wbindgen_free(ret, len * {});
+                return realRet;
+            ", f, ty.size());
+            return
+        }
+
+        if let Some(name) = ty.rust_struct() {
+            self.ret_ty = name.to_string();
+            self.ret_expr = format!("return {name}.__construct(RET);", name = name);
+            return
+        }
+
+        if ty.is_number() {
+            self.ret_ty = "number".to_string();
+            self.ret_expr = format!("return RET;");
+            return
+        }
+
+        match *ty {
+            Descriptor::Boolean => {
+                self.ret_ty = "boolean".to_string();
+                self.ret_expr = format!("return (RET) !== 0;");
+            }
+            Descriptor::Anyref => {
+                self.ret_ty = "any".to_string();
+                self.cx.expose_take_object();
+                self.ret_expr = format!("return takeObject(RET);");
+            }
+            _ => panic!("unsupported return from Rust to JS {:?}", ty),
+        }
+    }
+
+    /// Generate the actual function.
+    ///
+    /// The `prefix` specified is typically the string "function" but may be
+    /// different for classes. The `invoc` is the function expression that we're
+    /// invoking, like `wasm.bar` or `this.f`.
+    ///
+    /// Returns two strings, the first of which is the JS expression for the
+    /// generated function shim and the second is a TyepScript signature of rthe
+    /// JS expression.
+    pub fn finish(&self, prefix: &str, invoc: &str) -> (String, String) {
+        let js_args = self.js_arguments
+            .iter()
+            .map(|s| &s.0[..])
+            .collect::<Vec<_>>()
+            .join(", ");
+        let mut js = format!("{}({}) {{\n", prefix, js_args);
+        js.push_str(&self.prelude);
+        let rust_args = self.rust_arguments.join(", ");
+
+        let invoc = self.ret_expr.replace("RET", &format!("{}({})", invoc, rust_args));
+        if self.finally.len() == 0 {
+            js.push_str(&invoc);
+        } else {
+            js.push_str(&format!("\
+                try {{
+                    {}
+                }} finally {{
+                    {}
+                }}
+            ",
+                invoc,
+                self.finally,
+            ));
+        }
+        js.push_str("}");
+
+        let ts_args = self.js_arguments
+            .iter()
+            .map(|s| format!("{}: {}", s.0, s.1))
+            .collect::<Vec<_>>()
+            .join(", ");
+        let ts = format!("{} {}({}): {};\n", prefix, self.js_name, ts_args, self.ret_ty);
+        (js, ts)
+    }
+
+    fn global_idx(&mut self) -> usize {
+        let ret = self.global_idx;
+        self.global_idx += 1;
+        ret
+    }
+}

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

@@ -8,7 +8,10 @@ use shared;
 use wasm_gc;
 
 use super::Bindgen;
-use descriptor::{Descriptor, VectorKind, Function};
+use descriptor::{Descriptor, VectorKind};
+
+mod js2rust;
+use self::js2rust::Js2Rust;
 
 pub struct Context<'a> {
     pub globals: String,
@@ -1215,67 +1218,6 @@ impl<'a> Context<'a> {
         Descriptor::decode(&ret)
     }
 
-    fn return_from_rust(&mut self, ty: &Option<Descriptor>, dst_ts: &mut String)
-        -> String
-    {
-        let ty = match *ty {
-            Some(ref t) => t,
-            None => {
-                dst_ts.push_str(": void");
-                return format!("return ret;")
-            }
-        };
-
-        if ty.is_ref_anyref() {
-            dst_ts.push_str(": any");
-            self.expose_get_object();
-            return format!("return getObject(ret);")
-        }
-
-        if ty.is_by_ref() {
-            panic!("cannot return references from Rust to JS yet")
-        }
-
-        if let Some(ty) = ty.vector_kind() {
-            dst_ts.push_str(": ");
-            dst_ts.push_str(ty.js_ty());
-            let f = self.expose_get_vector_from_wasm(ty);
-            self.expose_get_global_argument();
-            self.required_internal_exports.insert("__wbindgen_free");
-            return format!("
-                const len = getGlobalArgument(0);
-                const realRet = {}(ret, len);
-                wasm.__wbindgen_free(ret, len * {});
-                return realRet;
-            ", f, ty.size())
-        }
-
-        if let Some(name) = ty.rust_struct() {
-            dst_ts.push_str(": ");
-            dst_ts.push_str(name);
-
-            return format!("return {}.__construct(ret)",&name);
-        }
-
-        if ty.is_number() {
-            dst_ts.push_str(": number");
-            return format!("return ret;")
-        }
-
-        match *ty {
-            Descriptor::Boolean => {
-                dst_ts.push_str(": boolean");
-                format!("return ret !== 0;")
-            }
-            Descriptor::Anyref => {
-                dst_ts.push_str(": any");
-                self.expose_take_object();
-                format!("return takeObject(ret);")
-            }
-            _ => panic!("unsupported return from Rust to JS {:?}", ty),
-        }
-    }
-
     fn return_from_js(&mut self, ty: &Option<Descriptor>, invoc: &str) -> String {
         let ty = match *ty {
             Some(ref t) => t,
@@ -1326,11 +1268,9 @@ impl<'a, 'b> SubContext<'a, 'b> {
             return self.generate_export_for_class(class, export);
         }
         let descriptor = self.cx.describe(&export.function.name);
-        let (js, ts) = self.generate_function("function",
+        let (js, ts) = Js2Rust::new(&export.function.name, self.cx)
-                                              &export.function.name,
+            .process(descriptor.unwrap_function())
-                                              &export.function.name,
+            .finish("function", &format!("wasm.{}", export.function.name));
-                                              false,
-                                              descriptor.unwrap_function());
         self.cx.export(&export.function.name, &js);
         self.cx.globals.push_str("\n");
         self.cx.typescript.push_str("export ");
@@ -1341,14 +1281,10 @@ impl<'a, 'b> SubContext<'a, 'b> {
     pub fn generate_export_for_class(&mut self, class_name: &str, export: &shared::Export) {
         let wasm_name = shared::struct_function_export_name(class_name, &export.function.name);
         let descriptor = self.cx.describe(&wasm_name);
-        let (js, ts) = self.generate_function(
+        let (js, ts) = Js2Rust::new(&export.function.name, self.cx)
-            "",
+            .method(export.method)
-            &export.function.name,
+            .process(descriptor.unwrap_function())
-            &wasm_name,
+            .finish("", &format!("wasm.{}", wasm_name));
-            export.method,
-            &descriptor.unwrap_function(),
-        );
-
         let class = self.cx.exported_classes.entry(class_name.to_string())
             .or_insert(ExportedClass::default());
         if !export.method {
@@ -1375,154 +1311,6 @@ impl<'a, 'b> SubContext<'a, 'b> {
         class.typescript.push_str("\n");
     }
 
-    fn generate_function(&mut self,
-                         prefix: &str,
-                         js_name: &str,
-                         wasm_name: &str,
-                         is_method: bool,
-                         function: &Function) -> (String, String) {
-        let mut dst = String::from("(");
-        let mut dst_ts = format!("{}(", js_name);
-        let mut passed_args = String::new();
-        let mut arg_conversions = String::new();
-        let mut destructors = String::new();
-
-        if is_method {
-            passed_args.push_str("this.ptr");
-        }
-
-        let mut global_idx = 0;
-        for (i, arg) in function.arguments.iter().enumerate() {
-            let name = format!("arg{}", i);
-            if i > 0 {
-                dst.push_str(", ");
-                dst_ts.push_str(", ");
-            }
-            dst.push_str(&name);
-            dst_ts.push_str(&name);
-
-            let mut pass = |arg: &str| {
-                if passed_args.len() > 0 {
-                    passed_args.push_str(", ");
-                }
-                passed_args.push_str(arg);
-            };
-
-            if let Some(kind) = arg.vector_kind() {
-                dst_ts.push_str(": ");
-                dst_ts.push_str(kind.js_ty());
-                let func = self.cx.pass_to_wasm_function(kind);
-                self.cx.expose_set_global_argument();
-                arg_conversions.push_str(&format!("\
-                    const [ptr{i}, len{i}] = {func}({arg});
-                    setGlobalArgument(len{i}, {global_idx});
-                ", i = i, func = func, arg = name, global_idx = global_idx));
-                global_idx += 1;
-                pass(&format!("ptr{}", i));
-                if arg.is_by_ref() {
-                    destructors.push_str(&format!("\n\
-                        wasm.__wbindgen_free(ptr{i}, len{i} * {size});\n\
-                    ", i = i, size = kind.size()));
-                    self.cx.required_internal_exports.insert(
-                        "__wbindgen_free",
-                    );
-                }
-                continue
-            }
-
-            if let Some(s) = arg.rust_struct() {
-                dst_ts.push_str(&format!(": {}", s));
-                if self.cx.config.debug {
-                    self.cx.expose_assert_class();
-                    arg_conversions.push_str(&format!("\
-                        _assertClass({arg}, {struct_});
-                    ", arg = name, struct_ = s));
-                }
-
-                if arg.is_by_ref() {
-                    pass(&format!("{}.ptr", name));
-                } else {
-                    arg_conversions.push_str(&format!("\
-                        const ptr{i} = {arg}.ptr;
-                        {arg}.ptr = 0;
-                    ", i = i, arg = name));
-                    pass(&format!("ptr{}", i));
-                }
-                continue
-            }
-
-            match *arg {
-                ref d if d.is_number() => {
-                    dst_ts.push_str(": number");
-                    if self.cx.config.debug {
-                        self.cx.expose_assert_num();
-                        arg_conversions.push_str(&format!("_assertNum({});\n", name));
-                    }
-                    pass(&name);
-                    continue
-                }
-                Descriptor::Boolean => {
-                    dst_ts.push_str(": boolean");
-                    if self.cx.config.debug {
-                        self.cx.expose_assert_bool();
-                        arg_conversions.push_str(&format!("\
-                            _assertBoolean({name});
-                        ", name = name));
-                    }
-                    pass(&format!("arg{i} ? 1 : 0", i = i));
-                    continue
-                }
-                Descriptor::Anyref => {
-                    dst_ts.push_str(": any");
-                    self.cx.expose_add_heap_object();
-                    pass(&format!("addHeapObject({})", name));
-                    continue
-                }
-                ref r if r.is_ref_anyref() => {
-                    dst_ts.push_str(": any");
-                    self.cx.expose_borrowed_objects();
-                    destructors.push_str("stack.pop();\n");
-                    pass(&format!("addBorrowedObject({})", name));
-                    continue
-                }
-                _ => {}
-            }
-            panic!("unsupported argument to rust function {:?}", arg)
-        }
-        dst.push_str(")");
-        dst_ts.push_str(")");
-        let convert_ret = self.cx.return_from_rust(&function.ret, &mut dst_ts);
-        dst_ts.push_str(";");
-        dst.push_str(" {\n        ");
-        dst.push_str(&arg_conversions);
-        if destructors.len() == 0 {
-            dst.push_str(&format!("\
-                const ret = wasm.{f}({passed});
-                {convert_ret}
-            ",
-                                  f = wasm_name,
-                                  passed = passed_args,
-                                  convert_ret = convert_ret,
-            ));
-        } else {
-            dst.push_str(&format!("\
-                try {{
-                    const ret = wasm.{f}({passed});
-                    {convert_ret}
-                }} finally {{
-                    {destructors}
-                }}
-            ",
-                                  f = wasm_name,
-                                  passed = passed_args,
-                                  destructors = destructors,
-                                  convert_ret = convert_ret,
-            ));
-        }
-        dst.push_str("}");
-        (format!("{} {}", prefix, dst), format!("{} {}", prefix, dst_ts))
-    }
-
     pub fn generate_import(&mut self, import: &shared::Import) {
         match import.kind {
             shared::ImportKind::Function(ref f) => {
@@ -1599,32 +1387,29 @@ impl<'a, 'b> SubContext<'a, 'b> {
             }
 
             if let Some((f, mutable)) = arg.stack_closure() {
-                let args = (0..f.arguments.len())
+                let (js, _ts) = {
-                    .map(|i| format!("arg{}", i))
+                    let mut builder = Js2Rust::new("", self.cx);
-                    .collect::<Vec<_>>()
+                    if mutable {
-                    .join(", ");
+                        builder.prelude("let a = this.a;\n")
+                            .prelude("this.a = 0;\n")
+                            .rust_argument("a")
+                            .finally("this.a = a;\n");
+                    } else {
+                        builder.rust_argument("this.a");
+                    }
+                    builder
+                        .rust_argument("this.b")
+                        .process(f)
+                        .finish("function", "this.f")
+                };
                 self.cx.expose_get_global_argument();
                 self.cx.function_table_needed = true;
-                let sep = if f.arguments.len() == 0 {""} else {","};
-                let body = if mutable {
-                    format!("
-                        let a = this.a;
-                        this.a = 0;
-                        try {{
-                            return this.f(a, this.b {} {});
-                        }} finally {{
-                            this.a = a;
-                        }}
-                    ", sep, args)
-                } else {
-                    format!("return this.f(this.a, this.b {} {});", sep, args)
-                };
                 extra.push_str(&format!("
-                    let cb{0} = function({args}) {{ {body} }};
+                    let cb{0} = {js};
                     cb{0}.f = wasm.__wbg_function_table.get(arg{0});
                     cb{0}.a = getGlobalArgument({next_global});
                     cb{0}.b = getGlobalArgument({next_global} + 1);
-                ", i, next_global = next_global, body = body, args = args));
+                ", i, js = js, next_global = next_global));
                 next_global += 2;
                 finally.push_str(&format!("
                     cb{0}.a = cb{0}.b = 0;
@@ -1633,9 +1418,41 @@ impl<'a, 'b> SubContext<'a, 'b> {
                 continue
             }
 
-            if let Some(_f) = arg.ref_closure() {
+            if let Some((f, mutable)) = arg.ref_closure() {
+                let (js, _ts) = {
+                    let mut builder = Js2Rust::new("", self.cx);
+                    if mutable {
+                        builder.prelude("let a = this.a;\n")
+                            .prelude("this.a = 0;\n")
+                            .rust_argument("a")
+                            .finally("this.a = a;\n");
+                    } else {
+                        builder.rust_argument("this.a");
+                    }
+                    builder
+                        .rust_argument("this.b")
+                        .process(f)
+                        .finish("function", "this.f")
+                };
+                self.cx.expose_get_global_argument();
+                self.cx.expose_uint32_memory();
+                self.cx.expose_add_heap_object();
+                self.cx.function_table_needed = true;
+                extra.push_str(&format!("
+                    let idx{0} = getUint32Memory()[arg{0} / 4];
+                    if (idx{0} === 0xffffffff) {{
+                        let cb{0} = {js};
+                        cb{0}.a = getGlobalArgument({next_global});
+                        cb{0}.b = getGlobalArgument({next_global} + 1);
+                        cb{0}.f = wasm.__wbg_function_table.get(getGlobalArgument({next_global} + 2));
+                        let real = cb{0}.bind(cb{0});
+                        real.original = cb{0};
+                        idx{0} = getUint32Memory()[arg{0} / 4] = addHeapObject(real);
+                    }}
+                ", i, js = js, next_global = next_global));
+                next_global += 3;
                 self.cx.expose_get_object();
-                invoc_args.push(format!("getObject(arg{})", i));
+                invoc_args.push(format!("getObject(idx{})", i));
                 continue
             }
 

src/closure.rs

@@ -4,13 +4,13 @@
 //! closures" from Rust to JS. Some more details can be found on the `Closure`
 //! type itself.
 
-use std::mem::{self, ManuallyDrop};
+use std::cell::UnsafeCell;
 use std::marker::Unsize;
+use std::mem::{self, ManuallyDrop};
 
-use {throw, JsValue};
+use JsValue;
 use convert::*;
 use describe::*;
-use __rt::WasmRefCell;
 
 /// A handle to both a closure in Rust as well as JS closure which will invoke
 /// the Rust closure.
@@ -63,13 +63,13 @@ use __rt::WasmRefCell;
 ///     ClosureHandle(cb)
 /// }
 /// ```
-pub struct Closure<T: WasmShim + ?Sized> {
+pub struct Closure<T: ?Sized> {
-    _inner: T::Wrapper,
+    inner: UnsafeCell<Box<T>>,
-    js: ManuallyDrop<JsValue>,
+    js: UnsafeCell<ManuallyDrop<JsValue>>,
 }
 
 impl<T> Closure<T>
-    where T: WasmShim + ?Sized,
+    where T: ?Sized,
 {
     /// Creates a new instance of `Closure` from the provided Rust closure.
     ///
@@ -86,21 +86,17 @@ impl<T> Closure<T>
     pub fn new<F>(t: F) -> Closure<T>
         where F: Unsize<T> + 'static
     {
-        Closure::wrap(T::wrap(t))
+        Closure::wrap(Box::new(t) as Box<T>)
     }
 
     /// A mostly internal function to wrap a boxed closure inside a `Closure`
     /// type.
     ///
     /// This is the function where the JS closure is manufactured.
-    pub fn wrap(t: T::Wrapper) -> Closure<T> {
+    pub fn wrap(t: Box<T>) -> Closure<T> {
-        unsafe {
+        Closure {
-            let data = T::data(&t);
+            inner: UnsafeCell::new(t),
-            let js = T::factory()(T::shim(), data[0], data[1]);
+            js: UnsafeCell::new(ManuallyDrop::new(JsValue { idx: !0 })),
-            Closure {
-                _inner: t,
-                js: ManuallyDrop::new(JsValue { idx: js }),
-            }
         }
     }
 
@@ -117,14 +113,17 @@ impl<T> Closure<T>
     /// cleanup as it can.
     pub fn forget(self) {
         unsafe {
-            super::__wbindgen_cb_forget(self.js.idx);
+            let idx = (*self.js.get()).idx;
+            if idx != !0 {
+                super::__wbindgen_cb_forget(idx);
+            }
             mem::forget(self);
         }
     }
 }
 
 impl<T> WasmDescribe for Closure<T>
-    where T: WasmShim + ?Sized,
+    where T: WasmClosure + ?Sized,
 {
     fn describe() {
         inform(CLOSURE);
@@ -134,21 +133,38 @@ impl<T> WasmDescribe for Closure<T>
 
 // `Closure` can only be passed by reference to imports.
 impl<'a, T> IntoWasmAbi for &'a Closure<T>
-    where T: WasmShim + ?Sized,
+    where T: WasmClosure + ?Sized,
 {
     type Abi = u32;
 
-    fn into_abi(self, _extra: &mut Stack) -> u32 {
+    fn into_abi(self, extra: &mut Stack) -> u32 {
-        self.js.idx
+        unsafe {
+            let fnptr = WasmClosure::into_abi(&mut **self.inner.get(), extra);
+            extra.push(fnptr);
+            &mut (*self.js.get()).idx as *const u32 as u32
+        }
     }
 }
 
+fn _check() {
+    fn _assert<T: IntoWasmAbi>() {}
+    _assert::<&Closure<Fn()>>();
+    _assert::<&Closure<Fn(String)>>();
+    _assert::<&Closure<Fn() -> String>>();
+    _assert::<&Closure<FnMut()>>();
+    _assert::<&Closure<FnMut(String)>>();
+    _assert::<&Closure<FnMut() -> String>>();
+}
+
 impl<T> Drop for Closure<T>
-    where T: WasmShim + ?Sized,
+    where T: ?Sized,
 {
     fn drop(&mut self) {
         unsafe {
-            super::__wbindgen_cb_drop(self.js.idx);
+            let idx = (*self.js.get()).idx;
+            if idx != !0 {
+                super::__wbindgen_cb_drop(idx);
+            }
         }
     }
 }
@@ -157,191 +173,76 @@ impl<T> Drop for Closure<T>
 ///
 /// This trait is not stable and it's not recommended to use this in bounds or
 /// implement yourself.
-pub unsafe trait WasmShim: WasmDescribe {
+pub unsafe trait WasmClosure: 'static {
-    #[doc(hidden)]
+    fn describe();
-    type Wrapper;
-    #[doc(hidden)]
-    fn shim() -> u32;
-    #[doc(hidden)]
-    fn factory() -> unsafe extern fn(u32, u32, u32) -> u32;
-    #[doc(hidden)]
-    fn wrap<U>(u: U) -> Self::Wrapper where U: Unsize<Self> + 'static;
-    #[doc(hidden)]
-    fn data(t: &Self::Wrapper) -> [u32; 2];
-}
 
-union RawPtr<T: ?Sized> {
+    unsafe fn into_abi(me: *mut Self, extra: &mut Stack) -> u32;
-    ptr: *const T,
-    data: [u32; 2]
 }
 
 macro_rules! doit {
     ($(
-        ($($var:ident)*) => $arity:ident
+        ($($var:ident)*)
     )*) => ($(
         // Fn with no return
-        unsafe impl<$($var),*> WasmShim for Fn($($var),*)
+        unsafe impl<$($var),*> WasmClosure for Fn($($var),*)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi + 'static,)*
         {
-            type Wrapper = Box<Fn($($var),*)>;
+            fn describe() {
-
+                <&Self>::describe();
-            fn shim() -> u32 {
-                #[allow(non_snake_case)]
-                unsafe extern fn shim<$($var),*>(
-                    a: u32,
-                    b: u32,
-                    $($var:$var),*
-                ) {
-                    if a == 0 {
-                        throw("closure has been destroyed already");
-                    }
-                    (*RawPtr::<Fn($($var),*)> { data: [a, b] }.ptr)($($var),*)
-                }
-                shim::<$($var),*> as u32
             }
 
-            fn factory() -> unsafe extern fn(u32, u32, u32) -> u32 {
+            unsafe fn into_abi(me: *mut Self, extra: &mut Stack) -> u32 {
-                super::$arity
+                IntoWasmAbi::into_abi(&*me, extra)
-            }
-
-            fn wrap<U>(u: U) -> Self::Wrapper where U: Unsize<Self> + 'static {
-                Box::new(u) as Box<Self>
-            }
-
-            fn data(t: &Self::Wrapper) -> [u32; 2] {
-                unsafe {
-                    RawPtr::<Self> { ptr: &**t }.data
-                }
             }
         }
-
+        // Fn with return
-        // Fn with a return
+        unsafe impl<$($var,)* R> WasmClosure for Fn($($var),*) -> R
-        unsafe impl<$($var,)* R> WasmShim for Fn($($var),*) -> R
+            where $($var: FromWasmAbi + 'static,)*
-            where $($var: WasmAbi + WasmDescribe,)*
+                  R: IntoWasmAbi + 'static,
-                  R: WasmAbi + WasmDescribe,
         {
-            type Wrapper = Box<Fn($($var),*) -> R>;
+            fn describe() {
-
+                <&Self>::describe();
-            fn shim() -> u32 {
-                #[allow(non_snake_case)]
-                unsafe extern fn shim<$($var,)* R>(
-                    a: u32,
-                    b: u32,
-                    $($var:$var),*
-                ) -> R {
-                    if a == 0 {
-                        throw("closure has been destroyed already");
-                    }
-                    (*RawPtr::<Fn($($var),*) -> R> { data: [a, b] }.ptr)($($var),*)
-                }
-                shim::<$($var,)* R> as u32
             }
 
-            fn factory() -> unsafe extern fn(u32, u32, u32) -> u32 {
+            unsafe fn into_abi(me: *mut Self, extra: &mut Stack) -> u32 {
-                super::$arity
+                IntoWasmAbi::into_abi(&*me, extra)
-            }
-
-            fn wrap<U>(u: U) -> Self::Wrapper where U: Unsize<Self> + 'static {
-                Box::new(u) as Box<Self>
-            }
-
-            fn data(t: &Self::Wrapper) -> [u32; 2] {
-                unsafe {
-                    RawPtr::<Self> { ptr: &**t }.data
-                }
             }
         }
-
         // FnMut with no return
-        unsafe impl<$($var),*> WasmShim for FnMut($($var),*)
+        unsafe impl<$($var),*> WasmClosure for FnMut($($var),*)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi + 'static,)*
         {
-            type Wrapper = Box<WasmRefCell<FnMut($($var),*)>>;
+            fn describe() {
-
+                <&mut Self>::describe();
-            fn shim() -> u32 {
-                #[allow(non_snake_case)]
-                unsafe extern fn shim<$($var),*>(
-                    a: u32,
-                    b: u32,
-                    $($var:$var),*
-                ) {
-                    if a == 0 {
-                        throw("closure has been destroyed already");
-                    }
-                    let ptr: *const WasmRefCell<FnMut($($var),*)> = RawPtr {
-                        data: [a, b],
-                    }.ptr;
-                    let mut ptr = (*ptr).borrow_mut();
-                    (&mut *ptr)($($var),*)
-                }
-                shim::<$($var),*> as u32
             }
 
-            fn factory() -> unsafe extern fn(u32, u32, u32) -> u32 {
+            unsafe fn into_abi(me: *mut Self, extra: &mut Stack) -> u32 {
-                super::$arity
+                IntoWasmAbi::into_abi(&mut *me, extra)
-            }
-
-            fn wrap<U>(u: U) -> Self::Wrapper where U: Unsize<Self> + 'static {
-                Box::new(WasmRefCell::new(u)) as Box<_>
-            }
-
-            fn data(t: &Self::Wrapper) -> [u32; 2] {
-                unsafe {
-                    RawPtr::<WasmRefCell<Self>> { ptr: &**t }.data
-                }
             }
         }
-
+        // FnMut with return
-        // FnMut with a return
+        unsafe impl<$($var,)* R> WasmClosure for FnMut($($var),*) -> R
-        unsafe impl<$($var,)* R> WasmShim for FnMut($($var),*) -> R
+            where $($var: FromWasmAbi + 'static,)*
-            where $($var: WasmAbi + WasmDescribe,)*
+                  R: IntoWasmAbi + 'static,
-                  R: WasmAbi + WasmDescribe,
         {
-            type Wrapper = Box<WasmRefCell<FnMut($($var),*) -> R>>;
+            fn describe() {
-
+                <&Self>::describe();
-            fn shim() -> u32 {
-                #[allow(non_snake_case)]
-                unsafe extern fn shim<$($var,)* R>(
-                    a: u32,
-                    b: u32,
-                    $($var:$var),*
-                ) -> R {
-                    if a == 0 {
-                        throw("closure has been destroyed already");
-                    }
-                    let ptr: *const WasmRefCell<FnMut($($var),*) -> R> = RawPtr {
-                        data: [a, b],
-                    }.ptr;
-                    let mut ptr = (*ptr).borrow_mut();
-                    (&mut *ptr)($($var),*)
-                }
-                shim::<$($var,)* R> as u32
             }
 
-            fn factory() -> unsafe extern fn(u32, u32, u32) -> u32 {
+            unsafe fn into_abi(me: *mut Self, extra: &mut Stack) -> u32 {
-                super::$arity
+                IntoWasmAbi::into_abi(&mut *me, extra)
-            }
-
-            fn wrap<U>(u: U) -> Self::Wrapper where U: Unsize<Self> + 'static {
-                Box::new(WasmRefCell::new(u)) as Box<_>
-            }
-
-            fn data(t: &Self::Wrapper) -> [u32; 2] {
-                unsafe {
-                    RawPtr::<WasmRefCell<Self>> { ptr: &**t }.data
-                }
             }
         }
     )*)
 }
 
 doit! {
-    () => __wbindgen_cb_arity0
+    ()
-    (A) => __wbindgen_cb_arity1
+    (A)
-    (A B) => __wbindgen_cb_arity2
+    (A B)
-    (A B C) => __wbindgen_cb_arity3
+    (A B C)
-    (A B C D) => __wbindgen_cb_arity4
+    (A B C D)
-    (A B C D E) => __wbindgen_cb_arity5
+    (A B C D E)
-    (A B C D E F) => __wbindgen_cb_arity6
+    (A B C D E F)
-    (A B C D E F G) => __wbindgen_cb_arity7
+    (A B C D E F G)
 }

src/convert.rs

@@ -325,24 +325,28 @@ pub unsafe extern fn __wbindgen_global_argument_ptr() -> *mut u32 {
 
 macro_rules! stack_closures {
     ($( ($($var:ident)*) )*) => ($(
-        impl<'a, $($var,)* R> IntoWasmAbi for &'a (Fn($($var),*) -> R + 'a)
+        impl<'a, 'b, $($var,)* R> IntoWasmAbi for &'a (Fn($($var),*) -> R + 'b)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi,)*
-                  R: WasmAbi + WasmDescribe
+                  R: IntoWasmAbi
         {
             type Abi = u32;
 
             fn into_abi(self, extra: &mut Stack) -> u32 {
                 #[allow(non_snake_case)]
-                unsafe extern fn invoke<$($var,)* R>(
+                unsafe extern fn invoke<$($var: FromWasmAbi,)* R: IntoWasmAbi>(
                     a: usize,
                     b: usize,
-                    $($var: $var),*
+                    $($var: <$var as FromWasmAbi>::Abi),*
-                ) -> R {
+                ) -> <R as IntoWasmAbi>::Abi {
                     if a == 0 {
-                        throw("closure has been destroyed already");
+                        throw("closure invoked recursively or destroyed already");
                     }
                     let f: &Fn($($var),*) -> R = mem::transmute((a, b));
-                    f($($var),*)
+                    let mut _stack = GlobalStack::new();
+                    $(
+                        let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack);
+                    )*
+                    f($($var),*).into_abi(&mut GlobalStack::new())
                 }
                 unsafe {
                     let (a, b): (usize, usize) = mem::transmute(self);
@@ -353,22 +357,26 @@ macro_rules! stack_closures {
             }
         }
 
-        impl<'a, $($var,)*> IntoWasmAbi for &'a (Fn($($var),*) + 'a)
+        impl<'a, 'b, $($var,)*> IntoWasmAbi for &'a (Fn($($var),*) + 'b)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi,)*
         {
             type Abi = u32;
 
             fn into_abi(self, extra: &mut Stack) -> u32 {
                 #[allow(non_snake_case)]
-                unsafe extern fn invoke<$($var,)* >(
+                unsafe extern fn invoke<$($var: FromWasmAbi,)* >(
                     a: usize,
                     b: usize,
-                    $($var: $var),*
+                    $($var: <$var as FromWasmAbi>::Abi),*
                 ) {
                     if a == 0 {
-                        throw("closure has been destroyed already");
+                        throw("closure invoked recursively or destroyed already");
                     }
                     let f: &Fn($($var),*) = mem::transmute((a, b));
+                    let mut _stack = GlobalStack::new();
+                    $(
+                        let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack);
+                    )*
                     f($($var),*)
                 }
                 unsafe {
@@ -380,24 +388,28 @@ macro_rules! stack_closures {
             }
         }
 
-        impl<'a, $($var,)* R> IntoWasmAbi for &'a mut (FnMut($($var),*) -> R + 'a)
+        impl<'a, 'b, $($var,)* R> IntoWasmAbi for &'a mut (FnMut($($var),*) -> R + 'b)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi,)*
-                  R: WasmAbi + WasmDescribe
+                  R: IntoWasmAbi
         {
             type Abi = u32;
 
             fn into_abi(self, extra: &mut Stack) -> u32 {
                 #[allow(non_snake_case)]
-                unsafe extern fn invoke<$($var,)* R>(
+                unsafe extern fn invoke<$($var: FromWasmAbi,)* R: IntoWasmAbi>(
                     a: usize,
                     b: usize,
-                    $($var: $var),*
+                    $($var: <$var as FromWasmAbi>::Abi),*
-                ) -> R {
+                ) -> <R as IntoWasmAbi>::Abi {
                     if a == 0 {
                         throw("closure invoked recursively or destroyed already");
                     }
                     let f: &mut FnMut($($var),*) -> R = mem::transmute((a, b));
-                    f($($var),*)
+                    let mut _stack = GlobalStack::new();
+                    $(
+                        let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack);
+                    )*
+                    f($($var),*).into_abi(&mut GlobalStack::new())
                 }
                 unsafe {
                     let (a, b): (usize, usize) = mem::transmute(self);
@@ -408,22 +420,26 @@ macro_rules! stack_closures {
             }
         }
 
-        impl<'a, $($var,)*> IntoWasmAbi for &'a mut (FnMut($($var),*) + 'a)
+        impl<'a, 'b, $($var,)*> IntoWasmAbi for &'a mut (FnMut($($var),*) + 'b)
-            where $($var: WasmAbi + WasmDescribe,)*
+            where $($var: FromWasmAbi,)*
         {
             type Abi = u32;
 
             fn into_abi(self, extra: &mut Stack) -> u32 {
                 #[allow(non_snake_case)]
-                unsafe extern fn invoke<$($var,)* >(
+                unsafe extern fn invoke<$($var: FromWasmAbi,)* >(
                     a: usize,
                     b: usize,
-                    $($var: $var),*
+                    $($var: <$var as FromWasmAbi>::Abi),*
                 ) {
                     if a == 0 {
                         throw("closure invoked recursively or destroyed already");
                     }
                     let f: &mut FnMut($($var),*) = mem::transmute((a, b));
+                    let mut _stack = GlobalStack::new();
+                    $(
+                        let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack);
+                    )*
                     f($($var),*)
                 }
                 unsafe {

tests/all/closures.rs

@@ -433,3 +433,86 @@ fn fnmut_bad() {
         .test();
 }
 
+#[test]
+fn string_arguments() {
+    project()
+        .file("src/lib.rs", r#"
+            #![feature(proc_macro, wasm_custom_section, wasm_import_module)]
+
+            extern crate wasm_bindgen;
+
+            use std::cell::Cell;
+            use wasm_bindgen::prelude::*;
+
+            #[wasm_bindgen(module = "./test")]
+            extern {
+                fn call(a: &mut FnMut(String));
+            }
+
+            #[wasm_bindgen]
+            pub fn run() {
+                let mut x = false;
+                call(&mut |s| {
+                    assert_eq!(s, "foo");
+                    x = true;
+                });
+                assert!(x);
+            }
+        "#)
+        .file("test.ts", r#"
+            import { run } from "./out";
+
+            export function call(a: any) {
+                a("foo")
+            }
+
+            export function test() {
+                run();
+            }
+        "#)
+        .test();
+}
+
+#[test]
+fn string_ret() {
+    project()
+        .file("src/lib.rs", r#"
+            #![feature(proc_macro, wasm_custom_section, wasm_import_module)]
+
+            extern crate wasm_bindgen;
+
+            use wasm_bindgen::prelude::*;
+
+            #[wasm_bindgen(module = "./test")]
+            extern {
+                fn call(a: &mut FnMut(String) -> String);
+            }
+
+            #[wasm_bindgen]
+            pub fn run() {
+                let mut x = false;
+                call(&mut |mut s| {
+                    assert_eq!(s, "foo");
+                    s.push_str("bar");
+                    x = true;
+                    s
+                });
+                assert!(x);
+            }
+        "#)
+        .file("test.ts", r#"
+            import { run } from "./out";
+            import * as assert from "assert";
+
+            export function call(a: any) {
+                const s = a("foo");
+                assert.strictEqual(s, "foobar");
+            }
+
+            export function test() {
+                run();
+            }
+        "#)
+        .test();
+}
+