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

Fix handling of u32 between Rust and JS #1401

Merged
merged 1 commit into from
Mar 27, 2019
Merged
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
20 changes: 16 additions & 4 deletions crates/cli-support/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ pub enum VectorKind {
Anyref,
}

pub struct Number {
u32: bool,
}

impl Descriptor {
pub fn decode(mut data: &[u32]) -> Descriptor {
let descriptor = Descriptor::_decode(&mut data);
Expand Down Expand Up @@ -149,18 +153,20 @@ impl Descriptor {
}
}

pub fn is_number(&self) -> bool {
/// Returns `Some` if this type is a number, and the returned `Number` type
/// can be accessed to learn more about what kind of number this is.
pub fn number(&self) -> Option<Number> {
match *self {
Descriptor::I8
| Descriptor::U8
| Descriptor::I16
| Descriptor::U16
| Descriptor::I32
| Descriptor::U32
| Descriptor::F32
| Descriptor::F64
| Descriptor::Enum { .. } => true,
_ => return false,
| Descriptor::Enum { .. } => Some(Number { u32: false }),
Descriptor::U32 => Some(Number { u32: true }),
_ => None,
}
}

Expand Down Expand Up @@ -360,3 +366,9 @@ impl VectorKind {
}
}
}

impl Number {
pub fn is_u32(&self) -> bool {
self.u32
}
}
10 changes: 7 additions & 3 deletions crates/cli-support/src/js/js2rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
return Ok(self);
}

if arg.is_number() {
if arg.number().is_some() {
self.js_arguments.push((name.clone(), "number".to_string()));

if self.cx.config.debug {
Expand Down Expand Up @@ -681,9 +681,13 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
return Ok(self);
}

if ty.is_number() {
if let Some(num) = ty.number() {
self.ret_ty = "number".to_string();
self.ret_expr = format!("return RET;");
if num.is_u32() {
self.ret_expr = format!("return RET >>> 0;");
} else {
self.ret_expr = format!("return RET;");
}
return Ok(self);
}

Expand Down
12 changes: 10 additions & 2 deletions crates/cli-support/src/js/rust2js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,16 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
return Ok(());
}

if let Some(num) = arg.number() {
if num.is_u32() {
self.js_arguments.push(format!("{} >>> 0", abi));
} else {
self.js_arguments.push(abi);
}
return Ok(());
}

let invoc_arg = match *arg {
ref d if d.is_number() => abi,
Descriptor::Boolean => format!("{} !== 0", abi),
Descriptor::Char => format!("String.fromCodePoint({})", abi),
_ => bail!(
Expand Down Expand Up @@ -504,7 +512,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> {

return Ok(());
}
if ty.is_number() {
if ty.number().is_some() {
self.ret_expr = "return JS;".to_string();
return Ok(());
}
Expand Down
32 changes: 32 additions & 0 deletions tests/wasm/math.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
const wasm = require('wasm-bindgen-test.js');
const assert = require('assert');

exports.js_auto_bind_math = () => {
wasm.math(1.0, 2.0);
};

exports.roundtrip = x => x;

exports.test_js_roundtrip = () => {
assert.strictEqual(wasm.rust_roundtrip_i8(0), 0);
assert.strictEqual(wasm.rust_roundtrip_i8(0x80), -128);
assert.strictEqual(wasm.rust_roundtrip_i8(0x7f), 127);

assert.strictEqual(wasm.rust_roundtrip_i16(0), 0);
assert.strictEqual(wasm.rust_roundtrip_i16(0x8000), -32768);
assert.strictEqual(wasm.rust_roundtrip_i16(0x7fff), 32767);

assert.strictEqual(wasm.rust_roundtrip_i32(0), 0);
assert.strictEqual(wasm.rust_roundtrip_i32(0x80000000), -2147483648);
assert.strictEqual(wasm.rust_roundtrip_i32(0x7fffffff), 2147483647);

assert.strictEqual(wasm.rust_roundtrip_u8(0), 0);
assert.strictEqual(wasm.rust_roundtrip_u8(0x80), 128);
assert.strictEqual(wasm.rust_roundtrip_u8(0x7f), 127);
assert.strictEqual(wasm.rust_roundtrip_u8(0xff), 255);

assert.strictEqual(wasm.rust_roundtrip_u16(0), 0);
assert.strictEqual(wasm.rust_roundtrip_u16(0x8000), 32768);
assert.strictEqual(wasm.rust_roundtrip_u16(0x7fff), 32767);
assert.strictEqual(wasm.rust_roundtrip_u16(0xffff), 65535);

assert.strictEqual(wasm.rust_roundtrip_u32(0), 0);
assert.strictEqual(wasm.rust_roundtrip_u32(0x80000000), 2147483648);
assert.strictEqual(wasm.rust_roundtrip_u32(0x7fffffff), 2147483647);
assert.strictEqual(wasm.rust_roundtrip_u32(0xffffffff), 4294967295);
};
61 changes: 61 additions & 0 deletions tests/wasm/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,26 @@ use wasm_bindgen_test::*;
#[wasm_bindgen(module = "tests/wasm/math.js")]
extern "C" {
fn js_auto_bind_math();

// There's an identity function called `roundtrip` in the module and we bind
// that one function with multiple different signatures here. Note that the
// return value is always `f64` to faithfully capture what was sent to JS
// (what we're interested in) because all JS numbers fit in `f64` anyway.
// This is testing what happens when we pass numbers to JS and what it sees.
#[wasm_bindgen(js_name = roundtrip)]
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
fn roundtrip_i8(a: i8) -> f64;
#[wasm_bindgen(js_name = roundtrip)]
fn roundtrip_i16(a: i16) -> f64;
#[wasm_bindgen(js_name = roundtrip)]
fn roundtrip_i32(a: i32) -> f64;
#[wasm_bindgen(js_name = roundtrip)]
fn roundtrip_u8(a: u8) -> f64;
#[wasm_bindgen(js_name = roundtrip)]
fn roundtrip_u16(a: u16) -> f64;
#[wasm_bindgen(js_name = roundtrip)]
fn roundtrip_u32(a: u32) -> f64;

fn test_js_roundtrip();
}

#[wasm_bindgen]
Expand Down Expand Up @@ -65,3 +85,44 @@ pub fn math(a: f32, b: f64) -> f64 {
fn auto_bind_math() {
js_auto_bind_math();
}

macro_rules! t_roundtrip {
($f:ident($e:expr)) => (assert_eq!($f($e), $e as f64))
}

#[wasm_bindgen_test]
fn limits_correct() {
t_roundtrip!(roundtrip_i8(i8::min_value()));
t_roundtrip!(roundtrip_i8(0));
t_roundtrip!(roundtrip_i8(i8::max_value()));
t_roundtrip!(roundtrip_i16(i16::min_value()));
t_roundtrip!(roundtrip_i16(0));
t_roundtrip!(roundtrip_i16(i16::max_value()));
t_roundtrip!(roundtrip_i32(i32::min_value()));
t_roundtrip!(roundtrip_i32(0));
t_roundtrip!(roundtrip_i32(i32::max_value()));
t_roundtrip!(roundtrip_u8(u8::min_value()));
t_roundtrip!(roundtrip_u8(0));
t_roundtrip!(roundtrip_u8(u8::max_value()));
t_roundtrip!(roundtrip_u16(u16::min_value()));
t_roundtrip!(roundtrip_u16(0));
t_roundtrip!(roundtrip_u16(u16::max_value()));
t_roundtrip!(roundtrip_u32(u32::min_value()));
t_roundtrip!(roundtrip_u32(0));
t_roundtrip!(roundtrip_u32(u32::max_value()));

test_js_roundtrip();

#[wasm_bindgen]
pub fn rust_roundtrip_i8(a: i8) -> i8 { a }
#[wasm_bindgen]
pub fn rust_roundtrip_i16(a: i16) -> i16 { a }
#[wasm_bindgen]
pub fn rust_roundtrip_i32(a: i32) -> i32 { a }
#[wasm_bindgen]
pub fn rust_roundtrip_u8(a: u8) -> u8 { a }
#[wasm_bindgen]
pub fn rust_roundtrip_u16(a: u16) -> u16 { a }
#[wasm_bindgen]
pub fn rust_roundtrip_u32(a: u32) -> u32 { a }
}