Skip to content

Commit

Permalink
Fix codegen of consuming setters/getters (rustwasm#2172)
Browse files Browse the repository at this point in the history
Make sure they reset their internal pointer to null after we call Rust
since it invalidates the Rust pointer after being called!

Closes rustwasm#2168
  • Loading branch information
alexcrichton authored and Perseus101 committed Jun 7, 2020
1 parent 7904264 commit 1cfd258
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 10 deletions.
15 changes: 9 additions & 6 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2190,10 +2190,9 @@ impl<'a> Context<'a> {
AuxExportKind::Function(_) => {}
AuxExportKind::StaticFunction { .. } => {}
AuxExportKind::Constructor(class) => builder.constructor(class),
AuxExportKind::Getter { .. } | AuxExportKind::Setter { .. } => {
builder.method(false)
}
AuxExportKind::Method { consumed, .. } => builder.method(*consumed),
AuxExportKind::Getter { consumed, .. }
| AuxExportKind::Setter { consumed, .. }
| AuxExportKind::Method { consumed, .. } => builder.method(*consumed),
}
}
Kind::Import(_) => {}
Expand Down Expand Up @@ -2257,7 +2256,7 @@ impl<'a> Context<'a> {
exported.has_constructor = true;
exported.push(&docs, "constructor", "", &code, ts_sig);
}
AuxExportKind::Getter { class, field } => {
AuxExportKind::Getter { class, field, .. } => {
let ret_ty = match export.generate_typescript {
true => match &ts_ret_ty {
Some(s) => Some(s.as_str()),
Expand All @@ -2268,7 +2267,7 @@ impl<'a> Context<'a> {
let exported = require_class(&mut self.exported_classes, class);
exported.push_getter(&docs, field, &code, ret_ty);
}
AuxExportKind::Setter { class, field } => {
AuxExportKind::Setter { class, field, .. } => {
let arg_ty = match export.generate_typescript {
true => Some(ts_arg_tys[0].as_str()),
false => None,
Expand Down Expand Up @@ -3247,20 +3246,24 @@ fn check_duplicated_getter_and_setter_names(
AuxExportKind::Getter {
class: first_class,
field: first_field,
consumed: _,
},
AuxExportKind::Getter {
class: second_class,
field: second_field,
consumed: _,
},
) => verify_exports(first_class, first_field, second_class, second_field)?,
(
AuxExportKind::Setter {
class: first_class,
field: first_field,
consumed: _,
},
AuxExportKind::Setter {
class: second_class,
field: second_field,
consumed: _,
},
) => verify_exports(first_class, first_field, second_class, second_field)?,
_ => {}
Expand Down
4 changes: 4 additions & 0 deletions crates/cli-support/src/wit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,15 @@ impl<'a> Context<'a> {
AuxExportKind::Getter {
class,
field: f.to_string(),
consumed: export.consumed,
}
}
decode::OperationKind::Setter(f) => {
descriptor.arguments.insert(0, Descriptor::I32);
AuxExportKind::Setter {
class,
field: f.to_string(),
consumed: export.consumed,
}
}
_ if op.is_static => AuxExportKind::StaticFunction {
Expand Down Expand Up @@ -806,6 +808,7 @@ impl<'a> Context<'a> {
kind: AuxExportKind::Getter {
class: struct_.name.to_string(),
field: field.name.to_string(),
consumed: false,
},
generate_typescript: field.generate_typescript,
},
Expand All @@ -832,6 +835,7 @@ impl<'a> Context<'a> {
kind: AuxExportKind::Setter {
class: struct_.name.to_string(),
field: field.name.to_string(),
consumed: false,
},
generate_typescript: field.generate_typescript,
},
Expand Down
14 changes: 12 additions & 2 deletions crates/cli-support/src/wit/nonstandard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,22 @@ pub enum AuxExportKind {
/// This function is intended to be a getter for a field on a class. The
/// first argument is the internal pointer and the returned value is
/// expected to be the field.
Getter { class: String, field: String },
Getter {
class: String,
field: String,
// same as `consumed` in `Method`
consumed: bool,
},

/// This function is intended to be a setter for a field on a class. The
/// first argument is the internal pointer and the second argument is
/// expected to be the field's new value.
Setter { class: String, field: String },
Setter {
class: String,
field: String,
// same as `consumed` in `Method`
consumed: bool,
},

/// This is a free function (ish) but scoped inside of a class name.
StaticFunction { class: String, name: String },
Expand Down
4 changes: 2 additions & 2 deletions crates/cli-support/src/wit/section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,15 +369,15 @@ fn check_standard_export(export: &AuxExport) -> Result<(), Error> {
name,
);
}
AuxExportKind::Getter { class, field } => {
AuxExportKind::Getter { class, field, .. } => {
bail!(
"cannot export `{}::{}` getter function when generating \
a standalone WebAssembly module with no JS glue",
class,
field,
);
}
AuxExportKind::Setter { class, field } => {
AuxExportKind::Setter { class, field, .. } => {
bail!(
"cannot export `{}::{}` setter function when generating \
a standalone WebAssembly module with no JS glue",
Expand Down
18 changes: 18 additions & 0 deletions examples/add/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,21 @@ use wasm_bindgen::prelude::*;
pub fn add(a: u32, b: u32) -> u32 {
a + b
}

#[wasm_bindgen]
#[derive(Copy, Clone)]
pub struct Answer(u32);

#[wasm_bindgen]
impl Answer {
pub fn new() -> Answer {
Answer(41)
}
#[wasm_bindgen(getter)]
pub fn the_answer(self) -> u32 {
self.0 + 1
}
pub fn foo(self) -> u32 {
self.0 + 1
}
}
7 changes: 7 additions & 0 deletions tests/wasm/validate_prt.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,11 @@ exports.js_works = () => {
useMoved();
moveMoved();
methodMoved();

const a = new wasm.Fruit('a');
a.prop;
assertMovedPtrThrows(() => a.prop);
const b = new wasm.Fruit('a');
b.prop = 3;
assertMovedPtrThrows(() => { b.prop = 4; });
};
8 changes: 8 additions & 0 deletions tests/wasm/validate_prt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ impl Fruit {
pub fn rot(self) {
drop(self);
}

#[wasm_bindgen(getter)]
pub fn prop(self) -> u32 {
0
}

#[wasm_bindgen(setter)]
pub fn set_prop(self, _val: u32) {}
}

#[wasm_bindgen]
Expand Down

0 comments on commit 1cfd258

Please sign in to comment.