Support Option with custom enums in JS

Find a hole automatically to use a sentinel value for `None`, and then just wire everything up! Closes rustwasm#1198
alexcrichton · Jan 28, 2019 · 409ebc8 · 409ebc8
1 parent 03e52c7
commit 409ebc8
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 5 deletions.
diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs
@@ -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))]

diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs
@@ -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! {
@@ -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,
@@ -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);
                 }
             }
         })

diff --git a/crates/cli-support/src/descriptor.rs b/crates/cli-support/src/descriptor.rs
@@ -59,7 +59,7 @@ pub enum Descriptor {
     Vector(Box<Descriptor>),
     String,
     Anyref,
-    Enum,
+    Enum { hole: u32 },
     RustStruct(String),
     Char,
     Option(Box<Descriptor>),
@@ -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())
@@ -159,7 +159,7 @@ impl Descriptor {
             | Descriptor::U32
             | Descriptor::F32
             | Descriptor::F64
-            | Descriptor::Enum => true,
+            | Descriptor::Enum { .. } => true,
             _ => return false,
         }
     }

diff --git a/crates/cli-support/src/js/js2rust.rs b/crates/cli-support/src/js/js2rust.rs
@@ -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
@@ -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

diff --git a/crates/cli-support/src/js/rust2js.rs b/crates/cli-support/src/js/rust2js.rs
@@ -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!(
@@ -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

diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs
@@ -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()
@@ -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((
                         _,
@@ -970,6 +984,8 @@ impl MacroParse<()> for syn::ItemEnum {
                             lit: syn::Lit::Int(ref int_lit),
                         }),
                     )) => {
+                        if !has_discriminant {
+                            }
                         if int_lit.value() > <u32>::max_value() as u64 {
                             bail_span!(
                                 int_lit,
@@ -992,12 +1008,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(())
     }

diff --git a/tests/wasm/enums.js b/tests/wasm/enums.js
@@ -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);
+};
diff --git a/tests/wasm/enums.rs b/tests/wasm/enums.rs
@@ -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,
@@ -22,7 +26,7 @@ pub mod inner {
     pub enum ColorWithCustomValues {
         Green = 21,
         Yellow = 34,
-        Red,
+        Red = 2,
     }
 }
 
@@ -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);
+}