Skip to content

Commit cc36bdc

Browse files
authored
Fix codegen of consuming setters/getters (#2172)
Make sure they reset their internal pointer to null after we call Rust since it invalidates the Rust pointer after being called! Closes #2168
1 parent b5e377d commit cc36bdc

File tree

7 files changed

+60
-10
lines changed

7 files changed

+60
-10
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2190,10 +2190,9 @@ impl<'a> Context<'a> {
21902190
AuxExportKind::Function(_) => {}
21912191
AuxExportKind::StaticFunction { .. } => {}
21922192
AuxExportKind::Constructor(class) => builder.constructor(class),
2193-
AuxExportKind::Getter { .. } | AuxExportKind::Setter { .. } => {
2194-
builder.method(false)
2195-
}
2196-
AuxExportKind::Method { consumed, .. } => builder.method(*consumed),
2193+
AuxExportKind::Getter { consumed, .. }
2194+
| AuxExportKind::Setter { consumed, .. }
2195+
| AuxExportKind::Method { consumed, .. } => builder.method(*consumed),
21972196
}
21982197
}
21992198
Kind::Import(_) => {}
@@ -2257,7 +2256,7 @@ impl<'a> Context<'a> {
22572256
exported.has_constructor = true;
22582257
exported.push(&docs, "constructor", "", &code, ts_sig);
22592258
}
2260-
AuxExportKind::Getter { class, field } => {
2259+
AuxExportKind::Getter { class, field, .. } => {
22612260
let ret_ty = match export.generate_typescript {
22622261
true => match &ts_ret_ty {
22632262
Some(s) => Some(s.as_str()),
@@ -2268,7 +2267,7 @@ impl<'a> Context<'a> {
22682267
let exported = require_class(&mut self.exported_classes, class);
22692268
exported.push_getter(&docs, field, &code, ret_ty);
22702269
}
2271-
AuxExportKind::Setter { class, field } => {
2270+
AuxExportKind::Setter { class, field, .. } => {
22722271
let arg_ty = match export.generate_typescript {
22732272
true => Some(ts_arg_tys[0].as_str()),
22742273
false => None,
@@ -3247,20 +3246,24 @@ fn check_duplicated_getter_and_setter_names(
32473246
AuxExportKind::Getter {
32483247
class: first_class,
32493248
field: first_field,
3249+
consumed: _,
32503250
},
32513251
AuxExportKind::Getter {
32523252
class: second_class,
32533253
field: second_field,
3254+
consumed: _,
32543255
},
32553256
) => verify_exports(first_class, first_field, second_class, second_field)?,
32563257
(
32573258
AuxExportKind::Setter {
32583259
class: first_class,
32593260
field: first_field,
3261+
consumed: _,
32603262
},
32613263
AuxExportKind::Setter {
32623264
class: second_class,
32633265
field: second_field,
3266+
consumed: _,
32643267
},
32653268
) => verify_exports(first_class, first_field, second_class, second_field)?,
32663269
_ => {}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,13 +419,15 @@ impl<'a> Context<'a> {
419419
AuxExportKind::Getter {
420420
class,
421421
field: f.to_string(),
422+
consumed: export.consumed,
422423
}
423424
}
424425
decode::OperationKind::Setter(f) => {
425426
descriptor.arguments.insert(0, Descriptor::I32);
426427
AuxExportKind::Setter {
427428
class,
428429
field: f.to_string(),
430+
consumed: export.consumed,
429431
}
430432
}
431433
_ if op.is_static => AuxExportKind::StaticFunction {
@@ -806,6 +808,7 @@ impl<'a> Context<'a> {
806808
kind: AuxExportKind::Getter {
807809
class: struct_.name.to_string(),
808810
field: field.name.to_string(),
811+
consumed: false,
809812
},
810813
generate_typescript: field.generate_typescript,
811814
},
@@ -832,6 +835,7 @@ impl<'a> Context<'a> {
832835
kind: AuxExportKind::Setter {
833836
class: struct_.name.to_string(),
834837
field: field.name.to_string(),
838+
consumed: false,
835839
},
836840
generate_typescript: field.generate_typescript,
837841
},

crates/cli-support/src/wit/nonstandard.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,22 @@ pub enum AuxExportKind {
105105
/// This function is intended to be a getter for a field on a class. The
106106
/// first argument is the internal pointer and the returned value is
107107
/// expected to be the field.
108-
Getter { class: String, field: String },
108+
Getter {
109+
class: String,
110+
field: String,
111+
// same as `consumed` in `Method`
112+
consumed: bool,
113+
},
109114

110115
/// This function is intended to be a setter for a field on a class. The
111116
/// first argument is the internal pointer and the second argument is
112117
/// expected to be the field's new value.
113-
Setter { class: String, field: String },
118+
Setter {
119+
class: String,
120+
field: String,
121+
// same as `consumed` in `Method`
122+
consumed: bool,
123+
},
114124

115125
/// This is a free function (ish) but scoped inside of a class name.
116126
StaticFunction { class: String, name: String },

crates/cli-support/src/wit/section.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,15 +369,15 @@ fn check_standard_export(export: &AuxExport) -> Result<(), Error> {
369369
name,
370370
);
371371
}
372-
AuxExportKind::Getter { class, field } => {
372+
AuxExportKind::Getter { class, field, .. } => {
373373
bail!(
374374
"cannot export `{}::{}` getter function when generating \
375375
a standalone WebAssembly module with no JS glue",
376376
class,
377377
field,
378378
);
379379
}
380-
AuxExportKind::Setter { class, field } => {
380+
AuxExportKind::Setter { class, field, .. } => {
381381
bail!(
382382
"cannot export `{}::{}` setter function when generating \
383383
a standalone WebAssembly module with no JS glue",

examples/add/src/lib.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,21 @@ use wasm_bindgen::prelude::*;
44
pub fn add(a: u32, b: u32) -> u32 {
55
a + b
66
}
7+
8+
#[wasm_bindgen]
9+
#[derive(Copy, Clone)]
10+
pub struct Answer(u32);
11+
12+
#[wasm_bindgen]
13+
impl Answer {
14+
pub fn new() -> Answer {
15+
Answer(41)
16+
}
17+
#[wasm_bindgen(getter)]
18+
pub fn the_answer(self) -> u32 {
19+
self.0 + 1
20+
}
21+
pub fn foo(self) -> u32 {
22+
self.0 + 1
23+
}
24+
}

tests/wasm/validate_prt.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,11 @@ exports.js_works = () => {
3333
useMoved();
3434
moveMoved();
3535
methodMoved();
36+
37+
const a = new wasm.Fruit('a');
38+
a.prop;
39+
assertMovedPtrThrows(() => a.prop);
40+
const b = new wasm.Fruit('a');
41+
b.prop = 3;
42+
assertMovedPtrThrows(() => { b.prop = 4; });
3643
};

tests/wasm/validate_prt.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ impl Fruit {
2525
pub fn rot(self) {
2626
drop(self);
2727
}
28+
29+
#[wasm_bindgen(getter)]
30+
pub fn prop(self) -> u32 {
31+
0
32+
}
33+
34+
#[wasm_bindgen(setter)]
35+
pub fn set_prop(self, _val: u32) {}
2836
}
2937

3038
#[wasm_bindgen]

0 commit comments

Comments
 (0)