diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index d745f325f9e..0991dac66cf 100644 --- a/crates/backend/src/ast.rs +++ b/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, /// The doc comments on this enum, if any diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index e5f3cfb9dd9..558a01d56ae 100644 --- a/crates/backend/src/codegen.rs +++ b/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::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)* { diff --git a/crates/backend/src/encode.rs b/crates/backend/src/encode.rs index bc82ba5c336..baae8d72533 100644 --- a/crates/backend/src/encode.rs +++ b/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() diff --git a/crates/cli-support/src/wit/mod.rs b/crates/cli-support/src/wit/mod.rs index 3e3e355c366..07a09860e0c 100644 --- a/crates/cli-support/src/wit/mod.rs +++ b/crates/cli-support/src/wit/mod.rs @@ -894,6 +894,7 @@ 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), @@ -901,11 +902,12 @@ impl<'a> Context<'a> { .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, diff --git a/crates/cli-support/src/wit/nonstandard.rs b/crates/cli-support/src/wit/nonstandard.rs index ac0f6d6eb6b..e43a49988df 100644 --- a/crates/cli-support/src/wit/nonstandard.rs +++ b/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, } diff --git a/crates/cli/tests/reference/enums.d.ts b/crates/cli/tests/reference/enums.d.ts index 1c830d483f9..da84bbb7740 100644 --- a/crates/cli/tests/reference/enums.d.ts +++ b/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. */ diff --git a/crates/cli/tests/reference/enums.js b/crates/cli/tests/reference/enums.js index ab820abbc3f..5b58920bb44 100644 --- a/crates/cli/tests/reference/enums.js +++ b/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"]; diff --git a/crates/cli/tests/reference/enums.rs b/crates/cli/tests/reference/enums.rs index 480dba7ce89..553d63d9a4d 100644 --- a/crates/cli/tests/reference/enums.rs +++ b/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) -> Option { + order +} diff --git a/crates/cli/tests/reference/enums.wat b/crates/cli/tests/reference/enums.wat index 479c1c3ec89..7ea009a06b7 100644 --- a/crates/cli/tests/reference/enums.wat +++ b/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") ) diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 39638bca4fc..67adf4a6531 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -1383,6 +1383,21 @@ fn string_enum( Ok(()) } +fn parse_number(expr: &syn::Expr) -> Option { + match get_expr(expr) { + syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Int(int_lit), + .. + }) => int_lit.base10_digits().parse::().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 = None; - let mut discriminate_map: HashMap = HashMap::new(); + let mut last_discriminant: Option = None; + let mut discriminant_map: HashMap = 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::() { - 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::, 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, diff --git a/crates/macro/ui-tests/invalid-enums.rs b/crates/macro/ui-tests/invalid-enums.rs index e0241f1eedb..571a39fb7c9 100644 --- a/crates/macro/ui-tests/invalid-enums.rs +++ b/crates/macro/ui-tests/invalid-enums.rs @@ -46,13 +46,13 @@ pub enum H { #[wasm_bindgen] pub enum I { A = 4294967294, // = u32::MAX - 1 - B, // would be u32::MAX - C, // would be u32::MAX + 1 + B, // = u32::MAX + C, // = u32::MAX + 1 } #[wasm_bindgen] pub enum J { - A, // = 0 + A, // = 0 B = 0, // collision } @@ -63,4 +63,16 @@ pub enum K { C, // = 3 -> collision } +#[wasm_bindgen] +pub enum L { + A = -2147483648, // i32::MIN + B = -2147483649, // i32::MIN - 1 +} + +#[wasm_bindgen] +pub enum M { + A = -1, + B = 2147483648, // i32::MAX + 1 +} + fn main() {} diff --git a/crates/macro/ui-tests/invalid-enums.stderr b/crates/macro/ui-tests/invalid-enums.stderr index e2e5b1a6c04..0164a369249 100644 --- a/crates/macro/ui-tests/invalid-enums.stderr +++ b/crates/macro/ui-tests/invalid-enums.stderr @@ -10,17 +10,17 @@ error: enum variants with associated data are not supported with #[wasm_bindgen] 8 | D(u32), | ^^^^^ -error: C-style enums with #[wasm_bindgen] may only have number literal values +error: C-style enums with #[wasm_bindgen] may only have numeric literal values that fit in a 32-bit integer as discriminants --> ui-tests/invalid-enums.rs:13:9 | 13 | X = 1 + 3, | ^^^^^ -error: C-style enums with #[wasm_bindgen] can only support numbers that can be represented as u32 - --> ui-tests/invalid-enums.rs:18:9 +error: C-style enums with #[wasm_bindgen] can only support numbers that can be represented as a 32-bit integer, but `4294967296` is too large to be represented by `u32` + --> ui-tests/invalid-enums.rs:18:5 | 18 | X = 4294967296, - | ^^^^^^^^^^ + | ^^^^^^^^^^^^^^ error: enums with #[wasm_bindgen] cannot mix string and non-string values --> ui-tests/invalid-enums.rs:23:9 @@ -46,10 +46,10 @@ error: discriminant value `1` is already used by A in this enum 43 | B = 1, // collision | ^^^^^ -error: the discriminants of C-style enums with #[wasm_bindgen] must be representable as u32 +error: C-style enums with #[wasm_bindgen] can only support numbers that can be represented as a 32-bit integer, but `4294967296` is too large to be represented by `u32` --> ui-tests/invalid-enums.rs:50:5 | -50 | C, // would be u32::MAX + 1 +50 | C, // = u32::MAX + 1 | ^ error: discriminant value `0` is already used by A in this enum @@ -63,3 +63,15 @@ error: discriminant value `3` is already used by A in this enum | 63 | C, // = 3 -> collision | ^ + +error: C-style enums with #[wasm_bindgen] can only support numbers that can be represented as a 32-bit integer, but `-2147483649` is too small to be represented by `i32` + --> ui-tests/invalid-enums.rs:69:5 + | +69 | B = -2147483649, // i32::MIN - 1 + | ^^^^^^^^^^^^^^^ + +error: 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`. `2147483648` 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`. + --> ui-tests/invalid-enums.rs:75:5 + | +75 | B = 2147483648, // i32::MAX + 1 + | ^^^^^^^^^^^^^^ diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index 57faecfcd84..13b893e4685 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -122,6 +122,7 @@ macro_rules! shared_api { struct Enum<'a> { name: &'a str, + signed: bool, variants: Vec>, comments: Vec<&'a str>, generate_typescript: bool,