Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove class wrap for constructors in Rust exports #3561

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
bindgen placeholder.
[#3233](https://github.com/rustwasm/wasm-bindgen/pull/3233)

* Changed inheritance behavior in generated JS bindings, it is now possible to
override methods from generated JS classes using inheritance.
[#3561](https://github.com/rustwasm/wasm-bindgen/pull/3561)

### Fixed

* Fixed bindings and comments for `Atomics.wait`.
Expand All @@ -53,6 +57,10 @@
* Fixed `wasm_bindgen_test` macro to handle raw identifiers in test names.
[#3541](https://github.com/rustwasm/wasm-bindgen/pull/3541)

* Fixed bug where constructors of exported Rust structs are allowed to return
other exported Rust structs (i.e. not `Self`).
[#3561](https://github.com/rustwasm/wasm-bindgen/pull/3561)

## [0.2.87](https://github.com/rustwasm/wasm-bindgen/compare/0.2.86...0.2.87)

Released 2023-06-12.
Expand Down
4 changes: 2 additions & 2 deletions crates/cli-support/src/externref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ fn find_call_export(instrs: &[InstructionData]) -> Option<Export> {
.iter()
.enumerate()
.filter_map(|(i, instr)| match instr.instr {
Instruction::CallExport(e) => Some(Export::Export(e)),
Instruction::CallExport(e, _) => Some(Export::Export(e)),
Instruction::CallTableElement(e) => Some(Export::TableElement {
idx: e,
call_idx: i,
Expand Down Expand Up @@ -267,7 +267,7 @@ fn export_xform(cx: &mut Context, export: Export, instrs: &mut Vec<InstructionDa
// also maintain indices of the instructions to delete.
for (i, instr) in iter.by_ref() {
match instr.instr {
Instruction::CallExport(_) | Instruction::CallTableElement(_) => break,
Instruction::CallExport(_, _) | Instruction::CallTableElement(_) => break,
Instruction::I32FromExternrefOwned => {
args.pop();
args.push(Some(true));
Expand Down
10 changes: 8 additions & 2 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
}

Instruction::CallCore(_)
| Instruction::CallExport(_)
| Instruction::CallExport(_, _)
| Instruction::CallAdapter(_)
| Instruction::CallTableElement(_)
| Instruction::DeferFree { .. } => {
Expand Down Expand Up @@ -986,6 +986,12 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
js.push(format!("String.fromCodePoint({})", val));
}

Instruction::SelfFromI32 => {
let val = js.pop();
js.prelude(&format!("this.__wbg_ptr = {} >>> 0;", val));
js.push("this".to_string());
}

Instruction::RustFromI32 { class } => {
js.cx.require_class_wrap(class);
let val = js.pop();
Expand Down Expand Up @@ -1203,7 +1209,7 @@ impl Invocation {
defer: true,
},

CallExport(e) => match module.exports.get(*e).item {
CallExport(e, _) => match module.exports.get(*e).item {
walrus::ExportItem::Function(id) => Invocation::Core { id, defer: false },
_ => panic!("can only call exported function"),
},
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2749,7 +2749,7 @@ impl<'a> Context<'a> {
call = Some(id);
}
}
Instruction::CallExport(_)
Instruction::CallExport(_, _)
| Instruction::CallTableElement(_)
| Instruction::CallCore(_) => return Ok(false),
_ => {}
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-support/src/multivalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn extract_xform<'a>(
.iter_mut()
.find_map(|i| match &mut i.instr {
Instruction::CallCore(f) => Some(Slot::Id(f)),
Instruction::CallExport(e) => Some(Slot::Export(*e)),
Instruction::CallExport(e, _) => Some(Slot::Export(*e)),
Instruction::CallTableElement(index) => Some(Slot::TableElement(*index)),
_ => None,
})
Expand Down
52 changes: 35 additions & 17 deletions crates/cli-support/src/wit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl<'a> Context<'a> {
None => AuxExportKind::Function(export.function.name.to_string()),
};

let id = self.export_adapter(export_id, descriptor)?;
let id = self.export_adapter(export_id, descriptor, &kind)?;
self.aux.export_map.insert(
id,
AuxExport {
Expand Down Expand Up @@ -870,20 +870,21 @@ impl<'a> Context<'a> {
ret: descriptor.clone(),
inner_ret: Some(descriptor.clone()),
};
let getter_id = self.export_adapter(getter_id, getter_descriptor)?;
let kind = AuxExportKind::Method {
class: struct_.name.to_string(),
name: field.name.to_string(),
receiver: AuxReceiverKind::Borrowed,
kind: AuxExportedMethodKind::Getter,
};
let getter_id = self.export_adapter(getter_id, getter_descriptor, &kind)?;
self.aux.export_map.insert(
getter_id,
AuxExport {
debug_name: format!("getter for `{}::{}`", struct_.name, field.name),
arg_names: None,
asyncness: false,
comments: concatenate_comments(&field.comments),
kind: AuxExportKind::Method {
class: struct_.name.to_string(),
name: field.name.to_string(),
receiver: AuxReceiverKind::Borrowed,
kind: AuxExportedMethodKind::Getter,
},
kind,
generate_typescript: field.generate_typescript,
generate_jsdoc: field.generate_jsdoc,
variadic: false,
Expand All @@ -902,20 +903,21 @@ impl<'a> Context<'a> {
ret: Descriptor::Unit,
inner_ret: None,
};
let setter_id = self.export_adapter(setter_id, setter_descriptor)?;
let kind = AuxExportKind::Method {
class: struct_.name.to_string(),
name: field.name.to_string(),
receiver: AuxReceiverKind::Borrowed,
kind: AuxExportedMethodKind::Setter,
};
let setter_id = self.export_adapter(setter_id, setter_descriptor, &kind)?;
self.aux.export_map.insert(
setter_id,
AuxExport {
debug_name: format!("setter for `{}::{}`", struct_.name, field.name),
arg_names: None,
asyncness: false,
comments: concatenate_comments(&field.comments),
kind: AuxExportKind::Method {
class: struct_.name.to_string(),
name: field.name.to_string(),
receiver: AuxReceiverKind::Borrowed,
kind: AuxExportedMethodKind::Setter,
},
kind,
generate_typescript: field.generate_typescript,
generate_jsdoc: field.generate_jsdoc,
variadic: false,
Expand Down Expand Up @@ -1196,11 +1198,12 @@ impl<'a> Context<'a> {
&mut self,
export: ExportId,
signature: Function,
kind: &AuxExportKind,
) -> Result<AdapterId, Error> {
let export = self.module.exports.get(export);
let name = export.name.clone();
// Do the actual heavy lifting elsewhere to generate the `binding`.
let call = Instruction::CallExport(export.id());
let call = Instruction::CallExport(export.id(), kind.clone());
let id = self.register_export_adapter(call, signature)?;
self.adapters.exports.push((name, id));
Ok(id)
Expand Down Expand Up @@ -1275,7 +1278,22 @@ impl<'a> Context<'a> {
};

let mut ret = args.cx.instruction_builder(true);
ret.outgoing(&signature.ret)?;
if let Instruction::CallExport(_, AuxExportKind::Constructor(ref class)) = call {
if let Descriptor::RustStruct(ref name) = signature.ret {
if class != name {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How useful is this check really? I mean all you have to do to bypass it is to wrap your foreign return type inside an Option or any other parameterized type and you've gone back to the old behaviour.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should reject Option<T> too - only the Rust struct that the constructor's for should be allowed.

bail!("constructor for `{}` cannot return `{}`", class, name);
}
ret.instruction(
&[AdapterType::I32],
Instruction::SelfFromI32,
&[AdapterType::Struct(class.clone())],
);
} else {
ret.outgoing(&signature.ret)?;
}
} else {
ret.outgoing(&signature.ret)?;
}
let uses_retptr = ret.input.len() > 1;

// Our instruction stream starts out with the return pointer as the first
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-support/src/wit/nonstandard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub struct AuxExport {
/// sort of webidl import to customize behavior or something like that. In any
/// case this doesn't feel quite right in terms of privilege separation, so
/// we'll want to work on this. For now though it works.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub enum AuxExportKind {
/// A free function that's just listed on the exported module
Function(String),
Expand Down
6 changes: 4 additions & 2 deletions crates/cli-support/src/wit/standard.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::descriptor::VectorKind;
use crate::wit::{AuxImport, WasmBindgenAux};
use crate::wit::{AuxExportKind, AuxImport, WasmBindgenAux};
use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use walrus::{FunctionId, ImportId, TypedCustomSectionId};
Expand Down Expand Up @@ -102,7 +102,7 @@ pub enum Instruction {
/// call-adapter instruction
CallAdapter(AdapterId),
/// Call an exported function in the core module
CallExport(walrus::ExportId),
CallExport(walrus::ExportId, AuxExportKind),
/// Call an element in the function table of the core module
CallTableElement(u32),

Expand Down Expand Up @@ -249,6 +249,8 @@ pub enum Instruction {
},
/// pops `i32`, pushes string from that `char`
StringFromChar,
/// pops `i32`, pushed an externref for rust class without wrapping
SelfFromI32,
/// pops `i32`, pushes an externref for the wrapped rust class
RustFromI32 {
class: String,
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(String);

#[wasm_bindgen]
impl ClassBuilder {
pub fn builder() -> Self {
ClassBuilder(String::from("Test"))
}
}
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 $classbuilder_builder (;0;) (type 0) (result i32))
(func $__wbg_classbuilder_free (;1;) (type 1) (param 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();
}
53 changes: 53 additions & 0 deletions crates/cli/tests/reference/constructor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
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;
return this;
}
}

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

Loading