Skip to content

Commit 6bb08b8

Browse files
committed
Fix handling of u32 between Rust and JS
All numbers in WebAssembly are signed and then each operation on them may optionally have an unsigned version. This means that when we pass large signed numbers to JS they actually show up as large negative numbers even though JS numbers can faithfully represent the type. This is fixed by adding `>>>0` in a few locations in the generated bindings to coerce the JS value into an unsigned value. Closes #1388
1 parent e3aabcb commit 6bb08b8

File tree

5 files changed

+114
-12
lines changed

5 files changed

+114
-12
lines changed

crates/cli-support/src/descriptor.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,18 +149,21 @@ impl Descriptor {
149149
}
150150
}
151151

152-
pub fn is_number(&self) -> bool {
152+
/// Returns `Some` if this type is a number, and the boolean returned
153+
/// represents whether it's `u32` and needs the sign reset when it goes
154+
/// into JS because JS is otherwise receiving a `i32`
155+
pub fn number(&self) -> Option<bool> {
153156
match *self {
154157
Descriptor::I8
155-
| Descriptor::U8
156158
| Descriptor::I16
157-
| Descriptor::U16
158159
| Descriptor::I32
159-
| Descriptor::U32
160160
| Descriptor::F32
161-
| Descriptor::F64
162-
| Descriptor::Enum { .. } => true,
163-
_ => return false,
161+
| Descriptor::U8
162+
| Descriptor::U16
163+
| Descriptor::F64 => Some(false),
164+
Descriptor::U32 => Some(true),
165+
| Descriptor::Enum { .. } => Some(false),
166+
_ => None,
164167
}
165168
}
166169

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
384384
return Ok(self);
385385
}
386386

387-
if arg.is_number() {
387+
if arg.number().is_some() {
388388
self.js_arguments.push((name.clone(), "number".to_string()));
389389

390390
if self.cx.config.debug {
@@ -681,9 +681,13 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
681681
return Ok(self);
682682
}
683683

684-
if ty.is_number() {
684+
if let Some(is_u32) = ty.number() {
685685
self.ret_ty = "number".to_string();
686-
self.ret_expr = format!("return RET;");
686+
if is_u32 {
687+
self.ret_expr = format!("return RET >>> 0;");
688+
} else {
689+
self.ret_expr = format!("return RET;");
690+
}
687691
return Ok(self);
688692
}
689693

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,16 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
309309
return Ok(());
310310
}
311311

312+
if let Some(is_u32) = arg.number() {
313+
if is_u32 {
314+
self.js_arguments.push(format!("{} >>> 0", abi));
315+
} else {
316+
self.js_arguments.push(abi);
317+
}
318+
return Ok(());
319+
}
320+
312321
let invoc_arg = match *arg {
313-
ref d if d.is_number() => abi,
314322
Descriptor::Boolean => format!("{} !== 0", abi),
315323
Descriptor::Char => format!("String.fromCodePoint({})", abi),
316324
_ => bail!(
@@ -504,7 +512,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
504512

505513
return Ok(());
506514
}
507-
if ty.is_number() {
515+
if ty.number().is_some() {
508516
self.ret_expr = "return JS;".to_string();
509517
return Ok(());
510518
}

tests/wasm/math.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,37 @@
11
const wasm = require('wasm-bindgen-test.js');
2+
const assert = require('assert');
23

34
exports.js_auto_bind_math = () => {
45
wasm.math(1.0, 2.0);
56
};
7+
8+
exports.roundtrip = x => x;
9+
10+
exports.test_js_roundtrip = () => {
11+
assert.strictEqual(wasm.rust_roundtrip_i8(0), 0);
12+
assert.strictEqual(wasm.rust_roundtrip_i8(0x80), -128);
13+
assert.strictEqual(wasm.rust_roundtrip_i8(0x7f), 127);
14+
15+
assert.strictEqual(wasm.rust_roundtrip_i16(0), 0);
16+
assert.strictEqual(wasm.rust_roundtrip_i16(0x8000), -32768);
17+
assert.strictEqual(wasm.rust_roundtrip_i16(0x7fff), 32767);
18+
19+
assert.strictEqual(wasm.rust_roundtrip_i32(0), 0);
20+
assert.strictEqual(wasm.rust_roundtrip_i32(0x80000000), -2147483648);
21+
assert.strictEqual(wasm.rust_roundtrip_i32(0x7fffffff), 2147483647);
22+
23+
assert.strictEqual(wasm.rust_roundtrip_u8(0), 0);
24+
assert.strictEqual(wasm.rust_roundtrip_u8(0x80), 128);
25+
assert.strictEqual(wasm.rust_roundtrip_u8(0x7f), 127);
26+
assert.strictEqual(wasm.rust_roundtrip_u8(0xff), 255);
27+
28+
assert.strictEqual(wasm.rust_roundtrip_u16(0), 0);
29+
assert.strictEqual(wasm.rust_roundtrip_u16(0x8000), 32768);
30+
assert.strictEqual(wasm.rust_roundtrip_u16(0x7fff), 32767);
31+
assert.strictEqual(wasm.rust_roundtrip_u16(0xffff), 65535);
32+
33+
assert.strictEqual(wasm.rust_roundtrip_u32(0), 0);
34+
assert.strictEqual(wasm.rust_roundtrip_u32(0x80000000), 2147483648);
35+
assert.strictEqual(wasm.rust_roundtrip_u32(0x7fffffff), 2147483647);
36+
assert.strictEqual(wasm.rust_roundtrip_u32(0xffffffff), 4294967295);
37+
};

tests/wasm/math.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ use wasm_bindgen_test::*;
44
#[wasm_bindgen(module = "tests/wasm/math.js")]
55
extern "C" {
66
fn js_auto_bind_math();
7+
8+
#[wasm_bindgen(js_name = roundtrip)]
9+
fn roundtrip_i8(a: i8) -> f64;
10+
#[wasm_bindgen(js_name = roundtrip)]
11+
fn roundtrip_i16(a: i16) -> f64;
12+
#[wasm_bindgen(js_name = roundtrip)]
13+
fn roundtrip_i32(a: i32) -> f64;
14+
#[wasm_bindgen(js_name = roundtrip)]
15+
fn roundtrip_u8(a: u8) -> f64;
16+
#[wasm_bindgen(js_name = roundtrip)]
17+
fn roundtrip_u16(a: u16) -> f64;
18+
#[wasm_bindgen(js_name = roundtrip)]
19+
fn roundtrip_u32(a: u32) -> f64;
20+
fn test_js_roundtrip();
721
}
822

923
#[wasm_bindgen]
@@ -65,3 +79,44 @@ pub fn math(a: f32, b: f64) -> f64 {
6579
fn auto_bind_math() {
6680
js_auto_bind_math();
6781
}
82+
83+
macro_rules! t_roundtrip {
84+
($f:ident($e:expr)) => (assert_eq!($f($e), $e as f64))
85+
}
86+
87+
#[wasm_bindgen_test]
88+
fn limits_correct() {
89+
t_roundtrip!(roundtrip_i8(i8::min_value()));
90+
t_roundtrip!(roundtrip_i8(0));
91+
t_roundtrip!(roundtrip_i8(i8::max_value()));
92+
t_roundtrip!(roundtrip_i16(i16::min_value()));
93+
t_roundtrip!(roundtrip_i16(0));
94+
t_roundtrip!(roundtrip_i16(i16::max_value()));
95+
t_roundtrip!(roundtrip_i32(i32::min_value()));
96+
t_roundtrip!(roundtrip_i32(0));
97+
t_roundtrip!(roundtrip_i32(i32::max_value()));
98+
t_roundtrip!(roundtrip_u8(u8::min_value()));
99+
t_roundtrip!(roundtrip_u8(0));
100+
t_roundtrip!(roundtrip_u8(u8::max_value()));
101+
t_roundtrip!(roundtrip_u16(u16::min_value()));
102+
t_roundtrip!(roundtrip_u16(0));
103+
t_roundtrip!(roundtrip_u16(u16::max_value()));
104+
t_roundtrip!(roundtrip_u32(u32::min_value()));
105+
t_roundtrip!(roundtrip_u32(0));
106+
t_roundtrip!(roundtrip_u32(u32::max_value()));
107+
108+
test_js_roundtrip();
109+
110+
#[wasm_bindgen]
111+
pub fn rust_roundtrip_i8(a: i8) -> i8 { a }
112+
#[wasm_bindgen]
113+
pub fn rust_roundtrip_i16(a: i16) -> i16 { a }
114+
#[wasm_bindgen]
115+
pub fn rust_roundtrip_i32(a: i32) -> i32 { a }
116+
#[wasm_bindgen]
117+
pub fn rust_roundtrip_u8(a: u8) -> u8 { a }
118+
#[wasm_bindgen]
119+
pub fn rust_roundtrip_u16(a: u16) -> u16 { a }
120+
#[wasm_bindgen]
121+
pub fn rust_roundtrip_u32(a: u32) -> u32 { a }
122+
}

0 commit comments

Comments
 (0)