diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index 406fb0a3..368130bf 100644 --- a/crates/backend/src/ast.rs +++ b/crates/backend/src/ast.rs @@ -34,11 +34,9 @@ pub struct Export { pub class: Option, /// The type of `self` (either `self`, `&self`, or `&mut self`) pub method_self: Option, - /// The name of the constructor function (e.g. new). - /// - /// This allows javascript to expose an `Object` interface, where calling - /// the constructor maps correctly to rust. - pub constructor: Option, + /// Whether or not this export is flagged as a constructor, returning an + /// instance of the `impl` type + pub is_constructor: bool, /// The rust function pub function: Function, /// Comments extracted from the rust source. @@ -317,7 +315,7 @@ impl Export { class: self.class.as_ref().map(|s| s.to_string()), method, consumed, - constructor: self.constructor.clone(), + is_constructor: self.is_constructor, function: self.function.shared(), comments: self.comments.clone(), } diff --git a/crates/cli-support/src/js/js2rust.rs b/crates/cli-support/src/js/js2rust.rs index 7d9b23d1..f9b47229 100644 --- a/crates/cli-support/src/js/js2rust.rs +++ b/crates/cli-support/src/js/js2rust.rs @@ -37,6 +37,14 @@ pub struct Js2Rust<'a, 'b: 'a> { /// Name of the JS shim/function that we're generating, primarily for /// TypeScript right now. js_name: String, + + /// whether or not this generated function body will act like a constructor, + /// meaning it doesn't actually return something but rather assigns to + /// `this` + /// + /// The string value here is the class that this should be a constructor + /// for. + constructor: Option, } impl<'a, 'b> Js2Rust<'a, 'b> { @@ -51,6 +59,7 @@ impl<'a, 'b> Js2Rust<'a, 'b> { arg_idx: 0, ret_ty: String::new(), ret_expr: String::new(), + constructor: None, } } @@ -64,6 +73,11 @@ impl<'a, 'b> Js2Rust<'a, 'b> { Ok(self) } + pub fn constructor(&mut self, class: Option<&str>) -> &mut Self { + self.constructor = class.map(|s| s.to_string()); + 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, consumed: bool) -> &mut Self { @@ -393,6 +407,32 @@ impl<'a, 'b> Js2Rust<'a, 'b> { } pub fn ret(&mut self, ty: &Descriptor) -> Result<&mut Self, Error> { + if let Some(name) = ty.rust_struct() { + match &self.constructor { + Some(class) if class == name => { + self.ret_expr = format!("this.ptr = RET;"); + if self.cx.config.weak_refs { + self.ret_expr.push_str(&format!("\ + addCleanup(this, this.ptr, free{}); + ", name)); + } + } + Some(class) => { + bail!("constructor for `{}` cannot return `{}`", class, name) + } + None => { + self.ret_ty = name.to_string(); + self.cx.require_class_wrap(name); + self.ret_expr = format!("return {name}.__wrap(RET);", name = name); + } + } + return Ok(self); + } + + if self.constructor.is_some() { + bail!("constructor functions must return a Rust structure") + } + if let Descriptor::Unit = ty { self.ret_ty = "void".to_string(); self.ret_expr = format!("return RET;"); @@ -551,7 +591,8 @@ impl<'a, 'b> Js2Rust<'a, 'b> { if let Some(name) = ty.rust_struct() { self.ret_ty = name.to_string(); - self.ret_expr = format!("return {name}.__construct(RET);", name = name); + self.cx.require_class_wrap(name); + self.ret_expr = format!("return {name}.__wrap(RET);", name = name); return Ok(self); } diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index e19c05fe..e6e2adce 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -54,7 +54,8 @@ pub struct ExportedClass { comments: String, contents: String, typescript: String, - constructor: Option, + has_constructor: bool, + wrap_needed: bool, fields: Vec, } @@ -532,11 +533,7 @@ impl<'a> Context<'a> { // `drop()` the `WeakRef` (cancel finalization) whenever it is // finalized. self.expose_cleanup_groups(); - let mk = format!(" - const cleanup_ptr = this.ptr; - const ref = CLEANUPS.makeRef(this, () => free{}(cleanup_ptr)); - CLEANUPS_MAP.set(this.ptr, ref); - ", name); + let mk = format!("addCleanup(this, this.ptr, free{});", name); let free = " CLEANUPS_MAP.get(ptr).drop(); CLEANUPS_MAP.delete(ptr); @@ -546,73 +543,28 @@ impl<'a> Context<'a> { (String::new(), "") }; - if self.config.debug || class.constructor.is_some() { - self.expose_constructor_token(); - - dst.push_str(&format!( + if self.config.debug && !class.has_constructor { + dst.push_str( " - static __construct(ptr) {{ - return new {}(new ConstructorToken(ptr)); - }} - - constructor(...args) {{ - if (args.length === 1 && args[0] instanceof ConstructorToken) {{ - this.ptr = args[0].ptr; - return; - }} - ", - name - )); - - if let Some(ref constructor) = class.constructor { - ts_dst.push_str(&format!("constructor(...args: any[]);\n")); - - dst.push_str(&format!( - " - // This invocation of new will call this constructor with a ConstructorToken - let instance = {class}.{constructor}(...args); - this.ptr = instance.ptr; - {mkweakref} - ", - class = name, - constructor = constructor, - mkweakref = mkweakref, - )); - } else { - dst.push_str( - "throw new Error('you cannot invoke `new` directly without having a \ - method annotated a constructor');\n", - ); - } - - dst.push_str("}"); - } else { - dst.push_str(&format!( + constructor() { + throw new Error('cannot invoke `new` directly'); + } " - static __construct(ptr) {{ - return new {}(ptr); - }} - - constructor(ptr) {{ - this.ptr = ptr; - {} - }} - ", - name, - mkweakref, - )); + ); } + let mut wrap_needed = class.wrap_needed; let new_name = shared::new_function(&name); if self.wasm_import_needed(&new_name) { self.expose_add_heap_object(); + wrap_needed = true; self.export( &new_name, &format!( " function(ptr) {{ - return addHeapObject({}.__construct(ptr)); + return addHeapObject({}.__wrap(ptr)); }} ", name @@ -621,6 +573,21 @@ impl<'a> Context<'a> { ); } + if wrap_needed { + dst.push_str(&format!( + " + static __wrap(ptr) {{ + const obj = Object.create({}.prototype); + obj.ptr = ptr; + {} + return obj; + }} + ", + name, + mkweakref.replace("this", "obj"), + )); + } + for field in class.fields.iter() { let wasm_getter = shared::struct_field_get(name, &field.name); let wasm_setter = shared::struct_field_set(name, &field.name); @@ -1165,22 +1132,6 @@ impl<'a> Context<'a> { ); } - fn expose_constructor_token(&mut self) { - if !self.exposed_globals.insert("ConstructorToken") { - return; - } - - self.global( - " - class ConstructorToken { - constructor(ptr) { - this.ptr = ptr; - } - } - ", - ); - } - fn expose_get_string_from_wasm(&mut self) { if !self.exposed_globals.insert("get_string_from_wasm") { return; @@ -1717,6 +1668,11 @@ impl<'a> Context<'a> { " const CLEANUPS = new WeakRefGroup(x => x.holdings()); const CLEANUPS_MAP = new Map(); + + function addCleanup(obj, ptr, free) { + const ref = CLEANUPS.makeRef(obj, () => free(ptr)); + CLEANUPS_MAP.set(ptr, ref); + } " ); } @@ -1785,6 +1741,13 @@ impl<'a> Context<'a> { self.memory_init = Some(mem.limits().clone()); "memory" } + + fn require_class_wrap(&mut self, class: &str) { + self.exported_classes + .entry(class.to_string()) + .or_insert_with(ExportedClass::default) + .wrap_needed = true; + } } impl<'a, 'b> SubContext<'a, 'b> { @@ -1857,8 +1820,14 @@ impl<'a, 'b> SubContext<'a, 'b> { Some(d) => d, }; - let (js, ts, js_doc) = Js2Rust::new(&export.function.name, self.cx) + let function_name = if export.is_constructor { + "constructor" + } else { + &export.function.name + }; + let (js, ts, js_doc) = Js2Rust::new(function_name, self.cx) .method(export.method, export.consumed) + .constructor(if export.is_constructor { Some(class_name) } else { None }) .process(descriptor.unwrap_function())? .finish("", &format!("wasm.{}", wasm_name)); @@ -1870,25 +1839,19 @@ impl<'a, 'b> SubContext<'a, 'b> { class .contents .push_str(&format_doc_comments(&export.comments, Some(js_doc))); - if !export.method { + + if export.is_constructor { + if class.has_constructor { + bail!("found duplicate constructor `{}`", + export.function.name); + } + class.has_constructor = true; + } else if !export.method { class.contents.push_str("static "); class.typescript.push_str("static "); } - let constructors: Vec = self - .program - .exports - .iter() - .filter(|x| x.class == Some(class_name.to_string())) - .filter_map(|x| x.constructor.clone()) - .collect(); - - class.constructor = match constructors.len() { - 0 => None, - 1 => Some(constructors[0].clone()), - x @ _ => bail!("there must be only one constructor, not {}", x), - }; - class.contents.push_str(&export.function.name); + class.contents.push_str(function_name); class.contents.push_str(&js); class.contents.push_str("\n"); class.typescript.push_str(&ts); diff --git a/crates/cli-support/src/js/rust2js.rs b/crates/cli-support/src/js/rust2js.rs index afbdfd01..f2173eb5 100644 --- a/crates/cli-support/src/js/rust2js.rs +++ b/crates/cli-support/src/js/rust2js.rs @@ -232,7 +232,8 @@ impl<'a, 'b> Rust2Js<'a, 'b> { if arg.is_by_ref() { bail!("cannot invoke JS functions with custom ref types yet") } - let assign = format!("let c{0} = {1}.__construct({0});", abi, class); + self.cx.require_class_wrap(class); + let assign = format!("let c{0} = {1}.__wrap({0});", abi, class); self.prelude(&assign); self.js_arguments.push(format!("c{}", abi)); return Ok(()); diff --git a/crates/cli/src/bin/wasm-bindgen-test-runner/server.rs b/crates/cli/src/bin/wasm-bindgen-test-runner/server.rs index 1387bb3d..ea441bce 100644 --- a/crates/cli/src/bin/wasm-bindgen-test-runner/server.rs +++ b/crates/cli/src/bin/wasm-bindgen-test-runner/server.rs @@ -29,7 +29,7 @@ pub fn spawn( // native ESM support for wasm modules. await wasm.booted; - const cx = Context.new(); + const cx = new Context(); window.console_log_redirect = __wbgtest_console_log; window.console_error_redirect = __wbgtest_console_error; diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 76bee033..b0d030b0 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -729,7 +729,7 @@ impl<'a> MacroParse<(Option, &'a mut TokenStream)> for syn::Item { program.exports.push(ast::Export { class: None, method_self: None, - constructor: None, + is_constructor: false, comments, rust_name: f.ident.clone(), function: f.convert(opts)?, @@ -852,12 +852,6 @@ impl<'a, 'b> MacroParse<()> for (&'a Ident, &'b mut syn::ImplItem) { let opts = BindgenAttrs::find(&mut method.attrs)?; let comments = extract_doc_comments(&method.attrs); let is_constructor = opts.constructor(); - let constructor = if is_constructor { - Some(method.sig.ident.to_string()) - } else { - None - }; - let (function, method_self) = function_from_decl( opts.js_name().unwrap_or(&method.sig.ident.to_string()), Box::new(method.sig.decl.clone()), @@ -870,7 +864,7 @@ impl<'a, 'b> MacroParse<()> for (&'a Ident, &'b mut syn::ImplItem) { program.exports.push(ast::Export { class: Some(class.clone()), method_self, - constructor, + is_constructor, function, comments, rust_name: method.sig.ident.clone(), diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index 3af33c36..fcfd4968 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -97,7 +97,7 @@ pub struct Export { pub class: Option, pub method: bool, pub consumed: bool, - pub constructor: Option, + pub is_constructor: bool, pub function: Function, pub comments: Vec, } diff --git a/tests/wasm/classes.js b/tests/wasm/classes.js index 1ffd4a9b..ad09011d 100644 --- a/tests/wasm/classes.js +++ b/tests/wasm/classes.js @@ -2,7 +2,7 @@ const wasm = require('wasm-bindgen-test.js'); const assert = require('assert'); exports.js_simple = () => { - const r = wasm.ClassesSimple.new(); + const r = new wasm.ClassesSimple(); assert.strictEqual(r.add(0), 0); assert.strictEqual(r.add(1), 1); assert.strictEqual(r.add(1), 2); @@ -70,7 +70,8 @@ exports.js_constructors = () => { assert.strictEqual(foo.get_number(), 1); foo.free(); - const foo2 = wasm.ConstructorsFoo.new(2); + assert.strictEqual(wasm.ConstructorsBar.new, undefined); + const foo2 = new wasm.ConstructorsFoo(2); assert.strictEqual(foo2.get_number(), 2); foo2.free(); @@ -78,7 +79,8 @@ exports.js_constructors = () => { assert.strictEqual(bar.get_sum(), 7); bar.free(); - const bar2 = wasm.ConstructorsBar.other_name(5, 6); + assert.strictEqual(wasm.ConstructorsBar.other_name, undefined); + const bar2 = new wasm.ConstructorsBar(5, 6); assert.strictEqual(bar2.get_sum(), 11); bar2.free(); @@ -121,12 +123,12 @@ exports.js_readonly_fields = () => { }; exports.js_double_consume = () => { - const r = wasm.DoubleConsume.new(); + const r = new wasm.DoubleConsume(); assert.throws(() => r.consume(r), /Attempt to use a moved value/); }; exports.js_js_rename = () => { - wasm.JsRename.new().bar(); + (new wasm.JsRename()).bar(); wasm.classes_foo(); };