Skip to content

Commit

Permalink
Remove class wrap for constructors in Rust exports
Browse files Browse the repository at this point in the history
After rustwasm#1594 constructors of Rust exported structs started using class
wrapping when generating JS shims. Wrapping erases prototype information
from the object instance in JS and as a result it is not possible to
override methods (via inheritance) of the generated class.

Additionally, some checks to ensure constructors always return an
instance of `Self` were lost.

This PR is rebased from rustwasm#3561, it passes the constructor information
from the `Builder` into the instruction translation function which
is then used to modify the generated bindings.

The `return` statement is also removed instead the value is directly
attached to the instance.

Signed-off-by: Oliver T <geronimooliver00@gmail.com>
  • Loading branch information
snOm3ad committed Aug 18, 2023
1 parent ed43f54 commit bd5da44
Show file tree
Hide file tree
Showing 11 changed files with 281 additions and 10 deletions.
48 changes: 38 additions & 10 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,24 @@ impl<'a, 'b> Builder<'a, 'b> {
// more JIT-friendly. The generated code should be equivalent to the
// wasm interface types stack machine, however.
for instr in instructions {
instruction(&mut js, &instr.instr, &mut self.log_error)?;
instruction(
&mut js,
&instr.instr,
&mut self.log_error,
&self.constructor,
)?;
}

assert_eq!(js.stack.len(), adapter.results.len());
match js.stack.len() {
0 => {}
1 => {
let val = js.pop();
js.prelude(&format!("return {};", val));
if self.constructor.is_some() {
js.prelude(&format!("this.__wbg_ptr = {}", val));
} else {
js.prelude(&format!("return {};", val));
}
}

// TODO: this should be pretty trivial to support (commented out
Expand Down Expand Up @@ -537,7 +546,12 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
}
}

fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) -> Result<(), Error> {
fn instruction(
js: &mut JsBuilder,
instr: &Instruction,
log_error: &mut bool,
constructor: &Option<String>,
) -> Result<(), Error> {
match instr {
Instruction::ArgGet(n) => {
let arg = js.arg(*n).to_string();
Expand Down Expand Up @@ -987,18 +1001,32 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
}

Instruction::RustFromI32 { class } => {
js.cx.require_class_wrap(class);
let val = js.pop();
js.push(format!("{}.__wrap({})", class, val));
if let Some(name) = constructor {
if name != class {
bail!("constructor for `{}` cannot return `{}`", name, class);
}
js.push(format!("{} >>> 0;", val));
} else {
js.cx.require_class_wrap(class);
js.push(format!("{}.__wrap({})", class, val));
}
}

Instruction::OptionRustFromI32 { class } => {
js.cx.require_class_wrap(class);
let val = js.pop();
js.push(format!(
"{0} === 0 ? undefined : {1}.__wrap({0})",
val, class,
))
if let Some(name) = constructor {
if name != class {
bail!("constructor for `{}` cannot return `{}`", name, class);
}
js.push(format!("{val} === 0 ? 0 : ({val} >>> 0);"));
} else {
js.cx.require_class_wrap(class);
js.push(format!(
"{0} === 0 ? undefined : {1}.__wrap({0})",
val, class,
))
}
}

Instruction::CachedStringLoad {
Expand Down
11 changes: 11 additions & 0 deletions crates/cli/tests/reference/builder.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* tslint:disable */
/* eslint-disable */
/**
*/
export class ClassBuilder {
free(): void;
/**
* @returns {ClassBuilder}
*/
static builder(): ClassBuilder;
}
61 changes: 61 additions & 0 deletions crates/cli/tests/reference/builder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
let wasm;
export function __wbg_set_wasm(val) {
wasm = val;
}


const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder;

let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true });

cachedTextDecoder.decode();

let cachedUint8Memory0 = null;

function getUint8Memory0() {
if (cachedUint8Memory0 === null || cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
}
return cachedUint8Memory0;
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}
/**
*/
export class ClassBuilder {

static __wrap(ptr) {
ptr = ptr >>> 0;
const obj = Object.create(ClassBuilder.prototype);
obj.__wbg_ptr = ptr;

return obj;
}

__destroy_into_raw() {
const ptr = this.__wbg_ptr;
this.__wbg_ptr = 0;

return ptr;
}

free() {
const ptr = this.__destroy_into_raw();
wasm.__wbg_classbuilder_free(ptr);
}
/**
* @returns {ClassBuilder}
*/
static builder() {
const ret = wasm.classbuilder_builder();
return ClassBuilder.__wrap(ret);
}
}

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
};

11 changes: 11 additions & 0 deletions crates/cli/tests/reference/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct ClassBuilder(());

#[wasm_bindgen]
impl ClassBuilder {
pub fn builder() -> ClassBuilder {
ClassBuilder(())
}
}
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/builder.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(module
(type (;0;) (func (result i32)))
(type (;1;) (func (param i32)))
(func $__wbg_classbuilder_free (;0;) (type 1) (param i32))
(func $classbuilder_builder (;1;) (type 0) (result i32))
(memory (;0;) 17)
(export "memory" (memory 0))
(export "__wbg_classbuilder_free" (func $__wbg_classbuilder_free))
(export "classbuilder_builder" (func $classbuilder_builder))
)
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/constructor.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/* tslint:disable */
/* eslint-disable */
/**
*/
export class ClassConstructor {
free(): void;
/**
*/
constructor();
}
52 changes: 52 additions & 0 deletions crates/cli/tests/reference/constructor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
let wasm;
export function __wbg_set_wasm(val) {
wasm = val;
}


const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder;

let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true });

cachedTextDecoder.decode();

let cachedUint8Memory0 = null;

function getUint8Memory0() {
if (cachedUint8Memory0 === null || cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
}
return cachedUint8Memory0;
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}
/**
*/
export class ClassConstructor {

__destroy_into_raw() {
const ptr = this.__wbg_ptr;
this.__wbg_ptr = 0;

return ptr;
}

free() {
const ptr = this.__destroy_into_raw();
wasm.__wbg_classconstructor_free(ptr);
}
/**
*/
constructor() {
const ret = wasm.classconstructor_new();
this.__wbg_ptr = ret >>> 0;
}
}

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
};

13 changes: 13 additions & 0 deletions crates/cli/tests/reference/constructor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct ClassConstructor(());

#[wasm_bindgen]
impl ClassConstructor {

#[wasm_bindgen(constructor)]
pub fn new() -> ClassConstructor {
ClassConstructor(())
}
}
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/constructor.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(module
(type (;0;) (func (result i32)))
(type (;1;) (func (param i32)))
(func $__wbg_classconstructor_free (;0;) (type 1) (param i32))
(func $classconstructor_new (;1;) (type 0) (result i32))
(memory (;0;) 17)
(export "memory" (memory 0))
(export "__wbg_classconstructor_free" (func $__wbg_classconstructor_free))
(export "classconstructor_new" (func $classconstructor_new))
)
27 changes: 27 additions & 0 deletions crates/cli/tests/wasm-bindgen/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,30 @@ fn function_table_preserved() {
.wasm_bindgen("");
cmd.assert().success();
}

#[test]
fn constructor_cannot_return_foreign_struct() {
let (mut cmd, _out_dir) = Project::new("constructor_cannot_return_foreign_struct")
.file(
"src/lib.rs",
r#"
use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct Foo(());
#[wasm_bindgen]
pub struct Bar(());
#[wasm_bindgen]
impl Foo {
#[wasm_bindgen(constructor)]
pub fn new() -> Bar {
Bar(())
}
}
"#,
)
.wasm_bindgen("--target web");
cmd.assert().failure();
}
38 changes: 38 additions & 0 deletions guide/src/reference/attributes/on-rust-exports/constructor.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,41 @@ import { Foo } from './my_module';
const f = new Foo();
console.log(f.get_contents());
```

## Caveats

Starting from v0.2.48 there is a bug in the `wasm-bindgen` which breaks inheritance of exported Rust structs from JavaScript side (see [#3213](https://github.com/rustwasm/wasm-bindgen/issues/3213)). If you want to inherit from a Rust struct such as:

```rust
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct Parent {
msg: String,
}

#[wasm_bindgen]
impl Parent {
#[wasm_bindgen(constructor)]
fn new() -> Self {
Parent {
msg: String::from("Hello from Parent!"),
}
}
}
```

You will need to reset the prototype of `this` back to the `Child` class prototype after calling the `Parent`'s constructor via `super`.

```js
import { Parent } from './my_module';

class Child extends Parent {
constructor() {
super();
Object.setPrototypeOf(this, Child.prototype);
}
}
```

This is no longer required as of v0.2.88.

0 comments on commit bd5da44

Please sign in to comment.