Add support for enums with negative discriminants

crates/backend/src/ast.rs

@@ -445,6 +445,9 @@ pub struct Enum {
     pub rust_name: Ident,
     /// The name of this enum in JS code
     pub js_name: String,
+    /// Whether the variant values and hole are signed, meaning that they
+    /// represent the bits of a `i32` value.
+    pub signed: bool,
     /// The variants provided by this enum
     pub variants: Vec<Variant>,
     /// The doc comments on this enum, if any

crates/backend/src/codegen.rs

@@ -1535,10 +1535,15 @@ impl ToTokens for ast::Enum {
         let name_len = name_str.len() as u32;
         let name_chars = name_str.chars().map(|c| c as u32);
         let hole = &self.hole;
+        let underlying = if self.signed {
+            quote! { i32 }
+        } else {
+            quote! { u32 }
+        };
         let cast_clauses = self.variants.iter().map(|variant| {
             let variant_name = &variant.name;
             quote! {
-                if js == #enum_name::#variant_name as u32 {
+                if js == #enum_name::#variant_name as #underlying {
                     #enum_name::#variant_name
                 }
             }
@@ -1548,20 +1553,20 @@ impl ToTokens for ast::Enum {
         (quote! {
             #[automatically_derived]
             impl #wasm_bindgen::convert::IntoWasmAbi for #enum_name {
-                type Abi = u32;
+                type Abi = #underlying;
 
                 #[inline]
-                fn into_abi(self) -> u32 {
-                    self as u32
+                fn into_abi(self) -> #underlying {
+                    self as #underlying
                 }
             }
 
             #[automatically_derived]
             impl #wasm_bindgen::convert::FromWasmAbi for #enum_name {
-                type Abi = u32;
+                type Abi = #underlying;
 
                 #[inline]
-                unsafe fn from_abi(js: u32) -> Self {
+                unsafe fn from_abi(js: #underlying) -> Self {
                     #(#cast_clauses else)* {
                         #wasm_bindgen::throw_str("invalid enum value passed")
                     }
@@ -1571,13 +1576,13 @@ impl ToTokens for ast::Enum {
             #[automatically_derived]
             impl #wasm_bindgen::convert::OptionFromWasmAbi for #enum_name {
                 #[inline]
-                fn is_none(val: &u32) -> bool { *val == #hole }
+                fn is_none(val: &Self::Abi) -> bool { *val == #hole as #underlying }
             }
 
             #[automatically_derived]
             impl #wasm_bindgen::convert::OptionIntoWasmAbi for #enum_name {
                 #[inline]
-                fn none() -> Self::Abi { #hole }
+                fn none() -> Self::Abi { #hole as #underlying }
             }
 
             #[automatically_derived]
@@ -1597,7 +1602,7 @@ impl ToTokens for ast::Enum {
                 #wasm_bindgen::JsValue
             {
                 fn from(value: #enum_name) -> Self {
-                    #wasm_bindgen::JsValue::from_f64((value as u32).into())
+                    #wasm_bindgen::JsValue::from_f64((value as #underlying).into())
                 }
             }
 
@@ -1608,7 +1613,7 @@ impl ToTokens for ast::Enum {
                 fn try_from_js_value(value: #wasm_bindgen::JsValue)
                     -> #wasm_bindgen::__rt::core::result::Result<Self, <#enum_name as #wasm_bindgen::convert::TryFromJsValue>::Error> {
                     use #wasm_bindgen::__rt::core::convert::TryFrom;
-                    let js = f64::try_from(&value)? as u32;
+                    let js = f64::try_from(&value)? as #underlying;
 
                     #wasm_bindgen::__rt::core::result::Result::Ok(
                         #(#try_from_cast_clauses else)* {

crates/backend/src/encode.rs

@@ -239,6 +239,7 @@ fn shared_function<'a>(func: &'a ast::Function, _intern: &'a Interner) -> Functi
 fn shared_enum<'a>(e: &'a ast::Enum, intern: &'a Interner) -> Enum<'a> {
     Enum {
         name: &e.js_name,
+        signed: e.signed,
         variants: e
             .variants
             .iter()

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

@@ -894,18 +894,20 @@ impl<'a> Context<'a> {
     }
 
     fn enum_(&mut self, enum_: decode::Enum<'_>) -> Result<(), Error> {
+        let signed = enum_.signed;
         let aux = AuxEnum {
             name: enum_.name.to_string(),
             comments: concatenate_comments(&enum_.comments),
             variants: enum_
                 .variants
                 .iter()
                 .map(|v| {
-                    (
-                        v.name.to_string(),
-                        v.value,
-                        concatenate_comments(&v.comments),
-                    )
+                    let value = if signed {
+                        v.value as i32 as i64
+                    } else {
+                        v.value as i64
+                    };
+                    (v.name.to_string(), value, concatenate_comments(&v.comments))
                 })
                 .collect(),
             generate_typescript: enum_.generate_typescript,

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

@@ -170,7 +170,7 @@ pub struct AuxEnum {
     pub comments: String,
     /// A list of variants with their name, value and comments
     /// and whether typescript bindings should be generated for each variant
-    pub variants: Vec<(String, u32, String)>,
+    pub variants: Vec<(String, i64, String)>,
     /// Whether typescript bindings should be generated for this enum.
     pub generate_typescript: bool,
 }

crates/cli/tests/reference/enums.d.ts

@@ -4,6 +4,7 @@ export function enum_echo(color: Color): Color;
 export function option_enum_echo(color?: Color): Color | undefined;
 export function get_name(color: Color): ColorName;
 export function option_string_enum_echo(color?: ColorName): ColorName | undefined;
+export function option_order(order?: Ordering): Ordering | undefined;
 /**
  * A color.
  */
@@ -27,6 +28,14 @@ export enum ImplicitDiscriminant {
   C = 42,
   D = 43,
 }
+/**
+ * A C-style enum with negative discriminants.
+ */
+export enum Ordering {
+  Less = -1,
+  Equal = 0,
+  Greater = 1,
+}
 /**
  * The name of a color.
  */

crates/cli/tests/reference/enums.js

@@ -62,6 +62,15 @@ export function option_string_enum_echo(color) {
     return __wbindgen_enum_ColorName[ret];
 }
 
+/**
+ * @param {Ordering | undefined} [order]
+ * @returns {Ordering | undefined}
+ */
+export function option_order(order) {
+    const ret = wasm.option_order(isLikeNone(order) ? 2 : order);
+    return ret === 2 ? undefined : ret;
+}
+
 /**
  * A color.
  * @enum {0 | 1 | 2}
@@ -83,6 +92,11 @@ Red:2,"2":"Red", });
  * @enum {0 | 1 | 42 | 43}
  */
 export const ImplicitDiscriminant = Object.freeze({ A:0,"0":"A",B:1,"1":"B",C:42,"42":"C",D:43,"43":"D", });
+/**
+ * A C-style enum with negative discriminants.
+ * @enum {-1 | 0 | 1}
+ */
+export const Ordering = Object.freeze({ Less:-1,"-1":"Less",Equal:0,"0":"Equal",Greater:1,"1":"Greater", });
 
 const __wbindgen_enum_ColorName = ["green", "yellow", "red"];
 

crates/cli/tests/reference/enums.rs

@@ -65,3 +65,16 @@ pub enum ImplicitDiscriminant {
     C = 42,
     D,
 }
+
+/// A C-style enum with negative discriminants.
+#[wasm_bindgen]
+pub enum Ordering {
+    Less = -1,
+    Equal = 0,
+    Greater = 1,
+}
+
+#[wasm_bindgen]
+pub fn option_order(order: Option<Ordering>) -> Option<Ordering> {
+    order
+}

crates/cli/tests/reference/enums.wat

@@ -4,12 +4,14 @@
   (func $option_enum_echo (;1;) (type 0) (param i32) (result i32))
   (func $get_name (;2;) (type 0) (param i32) (result i32))
   (func $option_string_enum_echo (;3;) (type 0) (param i32) (result i32))
+  (func $option_order (;4;) (type 0) (param i32) (result i32))
   (memory (;0;) 17)
   (export "memory" (memory 0))
   (export "enum_echo" (func $enum_echo))
   (export "option_enum_echo" (func $option_enum_echo))
   (export "get_name" (func $get_name))
   (export "option_string_enum_echo" (func $option_string_enum_echo))
+  (export "option_order" (func $option_order))
   (@custom "target_features" (after code) "\02+\0fmutable-globals+\08sign-ext")
 )
 

crates/macro-support/src/parser.rs

@@ -1383,6 +1383,21 @@ fn string_enum(
     Ok(())
 }
 
+fn parse_number(expr: &syn::Expr) -> Option<i64> {
+    match get_expr(expr) {
+        syn::Expr::Lit(syn::ExprLit {
+            lit: syn::Lit::Int(int_lit),
+            ..
+        }) => int_lit.base10_digits().parse::<i64>().ok(),
+        syn::Expr::Unary(syn::ExprUnary {
+            op: syn::UnOp::Neg(_),
+            expr,
+            ..
+        }) => parse_number(expr).and_then(|n| n.checked_neg()),
+        _ => None,
+    }
+}
+
 impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
     fn macro_parse(
         self,
@@ -1433,54 +1448,56 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
             _ => bail_span!(self, "only public enums are allowed with #[wasm_bindgen]"),
         }
 
-        let mut last_discriminant: Option<u32> = None;
-        let mut discriminate_map: HashMap<u32, &syn::Variant> = HashMap::new();
+        let mut last_discriminant: Option<i64> = None;
+        let mut discriminant_map: HashMap<i64, &syn::Variant> = HashMap::new();
+        let mut signed: bool = false;
 
         let variants = self
             .variants
             .iter()
             .map(|v| {
                 let value = match &v.discriminant {
-                    Some((_, expr)) => match get_expr(expr) {
-                        syn::Expr::Lit(syn::ExprLit {
-                            attrs: _,
-                            lit: syn::Lit::Int(int_lit),
-                        }) => match int_lit.base10_digits().parse::<u32>() {
-                            Ok(v) => v,
-                            Err(_) => {
-                                bail_span!(
-                                    int_lit,
-                                    "C-style enums with #[wasm_bindgen] can only support \
-                                 numbers that can be represented as u32"
-                                );
-                            }
-                        },
-                        expr => bail_span!(
+                    Some((_, expr)) => match parse_number(expr) {
+                        Some(value) => value,
+                        _ => bail_span!(
                             expr,
                             "C-style enums with #[wasm_bindgen] may only have \
-                             number literal values",
+                             numeric literal values that fit in a 32-bit integer as discriminants",
                         ),
                     },
                     None => {
                         // Use the same algorithm as rustc to determine the next discriminant
                         // https://doc.rust-lang.org/reference/items/enumerations.html#implicit-discriminants
                         if let Some(last) = last_discriminant {
-                            if let Some(value) = last.checked_add(1) {
-                                value
-                            } else {
-                                bail_span!(
-                                    v,
-                                    "the discriminants of C-style enums with #[wasm_bindgen] must be representable as u32"
-                                );
-                            }
+                            // we don't have to worry about overflow, since -2^31 <= last < 2^32
+                            last+1
                         } else {
                             0
                         }
                     }
                 };
+
+                if value < i32::MIN as i64 {
+                    bail_span!(
+                        v,
+                        "C-style enums with #[wasm_bindgen] can only support numbers that can be represented as a 32-bit integer, but `{}` is too small to be represented by `i32`",
+                        value
+                    );
+                }
+                if value > u32::MAX as i64 {
+                    bail_span!(
+                        v,
+                        "C-style enums with #[wasm_bindgen] can only support numbers that can be represented as a 32-bit integer, but `{}` is too large to be represented by `u32`",
+                        value
+                    );
+                }
+
                 last_discriminant = Some(value);
+                if value < 0 {
+                    signed = true;
+                }
 
-                if let Some(old) = discriminate_map.insert(value, v) {
+                if let Some(old) = discriminant_map.insert(value, v) {
                     bail_span!(
                         v,
                         "discriminant value `{}` is already used by {} in this enum",
@@ -1489,6 +1506,11 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
                     );
                 }
 
+                // We'll check whether the bits of the value are actually valid
+                // later. We can't do it right now, because a variants with a
+                // negative discriminant might change the enum to signed later.
+                let value = value as u32;
+
                 let comments = extract_doc_comments(&v.attrs);
                 Ok(ast::Variant {
                     name: v.ident.clone(),
@@ -1498,15 +1520,38 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
             })
             .collect::<Result<Vec<_>, Diagnostic>>()?;
 
-        let hole = (0..=u32::MAX)
-            .find(|v| !discriminate_map.contains_key(v))
-            .unwrap();
+        if signed {
+            // If the enum has signed discriminants, the entire enum is signed.
+            // This means that some variants with values >= 2^31 will be
+            // invalid, and we have to generate errors for them.
+            // Since it's enough to generate one error, we just check the
+            // variant with the largest value.
+            let max = discriminant_map.iter().max_by_key(|(k, _)| *k).unwrap();
+            if *max.0 > i32::MAX as i64 {
+                bail_span!(
+                    max.1,
+                    "this C-style enums with #[wasm_bindgen] contains at least one negative discriminant, so it can only support numbers that can be represented by `i32`. \
+                    `{}` is too large for `i32`, so either pick a smaller value for this variant or remove all negative discriminants to support values up to `u32::MAX`.",
+                    max.0
+                );
+            }
+        }
+
+        // By making the assumption that enums will have less than 2^31
+        // variants, we are guaranteed to find a hole by checking that the
+        // positive values of i32. This will work for both signed and unsigned
+        // enums and find the smallest non-negative hole.
+        assert!(variants.len() < i32::MAX as usize);
+        let hole = (0..i32::MAX as i64 + 1)
+            .find(|v| !discriminant_map.contains_key(v))
+            .unwrap() as u32;
 
         self.to_tokens(tokens);
 
         program.enums.push(ast::Enum {
             rust_name: self.ident,
             js_name,
+            signed,
             variants,
             comments,
             hole,