Skip to content

Commit

Permalink
Support Option with custom enums in JS
Browse files Browse the repository at this point in the history
Find a hole automatically to use a sentinel value for `None`, and then
just wire everything up!

Closes #1198
  • Loading branch information
alexcrichton committed Jan 28, 2019
1 parent 03e52c7 commit 9224455
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 5 deletions.
1 change: 1 addition & 0 deletions crates/backend/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ pub struct Enum {
pub name: Ident,
pub variants: Vec<Variant>,
pub comments: Vec<String>,
pub hole: u32,
}

#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))]
Expand Down
13 changes: 13 additions & 0 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,7 @@ impl<'a> ToTokens for DescribeImport<'a> {
impl ToTokens for ast::Enum {
fn to_tokens(&self, into: &mut TokenStream) {
let enum_name = &self.name;
let hole = &self.hole;
let cast_clauses = self.variants.iter().map(|variant| {
let variant_name = &variant.name;
quote! {
Expand All @@ -1034,6 +1035,7 @@ impl ToTokens for ast::Enum {
impl ::wasm_bindgen::convert::FromWasmAbi for #enum_name {
type Abi = u32;

#[inline]
unsafe fn from_abi(
js: u32,
_extra: &mut ::wasm_bindgen::convert::Stack,
Expand All @@ -1044,10 +1046,21 @@ impl ToTokens for ast::Enum {
}
}

impl ::wasm_bindgen::convert::OptionFromWasmAbi for #enum_name {
#[inline]
fn is_none(val: &u32) -> bool { *val == #hole }
}

impl ::wasm_bindgen::convert::OptionIntoWasmAbi for #enum_name {
#[inline]
fn none() -> Self::Abi { #hole }
}

impl ::wasm_bindgen::describe::WasmDescribe for #enum_name {
fn describe() {
use wasm_bindgen::describe::*;
inform(ENUM);
inform(#hole);
}
}
})
Expand Down
6 changes: 3 additions & 3 deletions crates/cli-support/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub enum Descriptor {
Vector(Box<Descriptor>),
String,
Anyref,
Enum,
Enum { hole: u32 },
RustStruct(String),
Char,
Option(Box<Descriptor>),
Expand Down Expand Up @@ -128,7 +128,7 @@ impl Descriptor {
OPTIONAL => Descriptor::Option(Box::new(Descriptor::_decode(data))),
STRING => Descriptor::String,
ANYREF => Descriptor::Anyref,
ENUM => Descriptor::Enum,
ENUM => Descriptor::Enum { hole: get(data) },
RUST_STRUCT => {
let name = (0..get(data))
.map(|_| char::from_u32(get(data)).unwrap())
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Descriptor {
| Descriptor::U32
| Descriptor::F32
| Descriptor::F64
| Descriptor::Enum => true,
| Descriptor::Enum { .. } => true,
_ => return false,
}
}
Expand Down
16 changes: 16 additions & 0 deletions crates/cli-support/src/js/js2rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,14 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
.push(format!("isLikeNone({0}) ? 0 : {0}.codePointAt(0)", name));
return Ok(self);
}
Descriptor::Enum { hole } => {
self.cx.expose_is_like_none();
self.js_arguments
.push((name.clone(), "number | undefined".to_string()));
self.rust_arguments
.push(format!("isLikeNone({0}) ? {1} : {0}", name, hole));
return Ok(self);
}
_ => bail!(
"unsupported optional argument type for calling Rust function from JS: {:?}",
arg
Expand Down Expand Up @@ -609,6 +617,14 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
.to_string();
return Ok(self);
}
Descriptor::Enum { hole } => {
self.ret_ty = "number | undefined".to_string();
self.ret_expr = format!("
const ret = RET;
return ret === {} ? undefined : ret;
", hole);
return Ok(self);
}
_ => bail!(
"unsupported optional return type for calling Rust function from JS: {:?}",
ty
Expand Down
13 changes: 13 additions & 0 deletions crates/cli-support/src/js/rust2js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
.push(format!("{0} === 0xFFFFFF ? undefined : {0} !== 0", abi));
return Ok(());
}
Descriptor::Enum { hole } => {
self.js_arguments
.push(format!("{0} === {1} ? undefined : {0}", abi, hole));
return Ok(());
}
Descriptor::Char => {
let value = self.shim_argument();
self.js_arguments.push(format!(
Expand Down Expand Up @@ -441,6 +446,14 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
.to_string();
return Ok(());
}
Descriptor::Enum { hole } => {
self.cx.expose_is_like_none();
self.ret_expr = format!("
const val = JS;
return isLikeNone(val) ? {} : val;
", hole);
return Ok(());
}
_ => bail!(
"unsupported optional return type for calling JS function from Rust: {:?}",
ty
Expand Down
34 changes: 33 additions & 1 deletion crates/macro-support/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,12 @@ impl MacroParse<()> for syn::ItemEnum {
_ => bail_span!(self, "only public enums are allowed with #[wasm_bindgen]"),
}

if self.variants.len() == 0 {
bail_span!(self, "cannot export empty enums to JS");
}

let has_discriminant = self.variants[0].discriminant.is_some();

let variants = self
.variants
.iter()
Expand All @@ -962,6 +968,14 @@ impl MacroParse<()> for syn::ItemEnum {
syn::Fields::Unit => (),
_ => bail_span!(v.fields, "only C-Style enums allowed with #[wasm_bindgen]"),
}

// Require that everything either has a discriminant or doesn't.
// We don't really want to get in the business of emulating how
// rustc assigns values to enums.
if v.discriminant.is_some() != has_discriminant {
bail_span!(v, "must either annotate discriminant of all variants or none");
}

let value = match v.discriminant {
Some((
_,
Expand Down Expand Up @@ -992,12 +1006,30 @@ impl MacroParse<()> for syn::ItemEnum {
value,
})
})
.collect::<Result<_, Diagnostic>>()?;
.collect::<Result<Vec<_>, Diagnostic>>()?;

let mut values = variants.iter().map(|v| v.value).collect::<Vec<_>>();
values.sort();
let hole = values.windows(2)
.filter_map(|window| {
if window[0] + 1 != window[1] {
Some(window[0] + 1)
} else {
None
}
})
.next()
.unwrap_or(*values.last().unwrap() + 1);
for value in values {
assert!(hole != value);
}

let comments = extract_doc_comments(&self.attrs);
program.enums.push(ast::Enum {
name: self.ident,
variants,
comments,
hole,
});
Ok(())
}
Expand Down
10 changes: 10 additions & 0 deletions tests/wasm/enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,13 @@ exports.js_c_style_enum_with_custom_values = () => {

assert.strictEqual(wasm.enum_with_custom_values_cycle(wasm.ColorWithCustomValues.Green), wasm.ColorWithCustomValues.Yellow);
};

exports.js_handle_optional_enums = x => wasm.handle_optional_enums(x);

exports.js_expect_enum = (a, b) => {
assert.strictEqual(a, b);
};

exports.js_expect_enum_none = a => {
assert.strictEqual(a, undefined);
};
31 changes: 30 additions & 1 deletion tests/wasm/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ use wasm_bindgen_test::*;
extern "C" {
fn js_c_style_enum();
fn js_c_style_enum_with_custom_values();
fn js_handle_optional_enums(x: Option<Color>) -> Option<Color>;
fn js_expect_enum(x: Color, y: Option<Color>);
fn js_expect_enum_none(x: Option<Color>);
}

#[wasm_bindgen]
#[derive(PartialEq, Debug)]
pub enum Color {
Green,
Yellow,
Expand All @@ -22,7 +26,7 @@ pub mod inner {
pub enum ColorWithCustomValues {
Green = 21,
Yellow = 34,
Red,
Red = 2,
}
}

Expand Down Expand Up @@ -53,3 +57,28 @@ fn c_style_enum() {
fn c_style_enum_with_custom_values() {
js_c_style_enum_with_custom_values();
}

#[wasm_bindgen]
pub fn handle_optional_enums(x: Option<Color>) -> Option<Color> {
x
}

#[wasm_bindgen_test]
fn test_optional_enums() {
use self::Color::*;

assert_eq!(js_handle_optional_enums(None), None);
assert_eq!(js_handle_optional_enums(Some(Green)), Some(Green));
assert_eq!(js_handle_optional_enums(Some(Yellow)), Some(Yellow));
assert_eq!(js_handle_optional_enums(Some(Red)), Some(Red));
}

#[wasm_bindgen_test]
fn test_optional_enum_values() {
use self::Color::*;

js_expect_enum(Green, Some(Green));
js_expect_enum(Yellow, Some(Yellow));
js_expect_enum(Red, Some(Red));
js_expect_enum_none(None);
}

0 comments on commit 9224455

Please sign in to comment.