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

Add support for C-style enums with implicit discriminants #4152

Merged
merged 6 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
* Added additional bindings for setters from WebIDL interface attributes with applicaple parameter types of just `JsValue`.
[#4156](https://github.com/rustwasm/wasm-bindgen/pull/4156)

* Added support for implicit discriminants in enums.
[#4152](https://github.com/rustwasm/wasm-bindgen/pull/4152)

### Changed

* Implicitly enable reference type and multivalue transformations if the module already makes use of the corresponding target features.
Expand Down
6 changes: 6 additions & 0 deletions crates/cli/tests/reference/enums.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export enum Color {
*/
Red = 2,
}
export enum ImplicitDiscriminant {
A = 0,
B = 1,
C = 42,
D = 43,
}
/**
* The name of a color.
*/
Expand Down
2 changes: 2 additions & 0 deletions crates/cli/tests/reference/enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Yellow:1,"1":"Yellow",
*/
Red:2,"2":"Red", });

export const ImplicitDiscriminant = Object.freeze({ A:0,"0":"A",B:1,"1":"B",C:42,"42":"C",D:43,"43":"D", });

const __wbindgen_enum_ColorName = ["green", "yellow", "red"];

const __wbindgen_enum_FooBar = ["foo", "bar"];
Expand Down
10 changes: 8 additions & 2 deletions crates/cli/tests/reference/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,21 @@ pub fn option_string_enum_echo(color: Option<ColorName>) -> Option<ColorName> {

/// An unused string enum.
#[wasm_bindgen(js_name = "FooBar")]
#[derive(PartialEq, Debug)]
pub enum UnusedStringEnum {
Foo = "foo",
Bar = "bar",
}

#[wasm_bindgen]
#[derive(PartialEq, Debug)]
enum PrivateStringEnum {
Foo = "foo",
Bar = "bar",
}

#[wasm_bindgen]
pub enum ImplicitDiscriminant {
A,
B,
C = 42,
D,
}
64 changes: 34 additions & 30 deletions crates/macro-support/src/parser.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::cell::{Cell, RefCell};
use std::char;
use std::collections::HashMap;
use std::str::Chars;

use ast::OperationKind;
Expand Down Expand Up @@ -1399,28 +1400,18 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
return string_enum(self, program, js_name, generate_typescript, comments);
}

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

match self.vis {
syn::Visibility::Public(_) => {}
_ => 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 variants = self
.variants
.iter()
.enumerate()
.map(|(i, v)| {
// 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"
);
}

.map(|v| {
let value = match &v.discriminant {
Some((_, expr)) => match get_expr(expr) {
syn::Expr::Lit(syn::ExprLit {
Expand All @@ -1442,8 +1433,33 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
number literal values",
),
},
None => i as u32,
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"
);
}
} else {
0
}
}
};
last_discriminant = Some(value);

if let Some(old) = discriminate_map.insert(value, v) {
bail_span!(
v,
"discriminant value `{}` is already used by {} in this enum",
value,
old.ident
);
}

let comments = extract_doc_comments(&v.attrs);
Ok(ast::Variant {
Expand All @@ -1454,21 +1470,9 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum {
})
.collect::<Result<Vec<_>, Diagnostic>>()?;

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

self.to_tokens(tokens);

Expand Down
26 changes: 26 additions & 0 deletions crates/macro/ui-tests/invalid-enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,30 @@ enum G {
C,
}

#[wasm_bindgen]
pub enum H {
A = 1,
B = 1, // collision
}

#[wasm_bindgen]
pub enum I {
A = 4294967294, // = u32::MAX - 1
B, // would be u32::MAX
C, // would be u32::MAX + 1
}

#[wasm_bindgen]
pub enum J {
A, // = 0
B = 0, // collision
}

#[wasm_bindgen]
pub enum K {
A = 3,
B = 2,
C, // = 3 -> collision
}

fn main() {}
24 changes: 24 additions & 0 deletions crates/macro/ui-tests/invalid-enums.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,27 @@ error: all variants of a string enum must have a string value
|
37 | C,
| ^

error: discriminant value `1` is already used by A in this enum
--> ui-tests/invalid-enums.rs:43:5
|
43 | B = 1, // collision
| ^^^^^

error: the discriminants of C-style enums with #[wasm_bindgen] must be representable as u32
--> ui-tests/invalid-enums.rs:50:5
|
50 | C, // would be u32::MAX + 1
| ^

error: discriminant value `0` is already used by A in this enum
--> ui-tests/invalid-enums.rs:56:5
|
56 | B = 0, // collision
| ^^^^^

error: discriminant value `3` is already used by A in this enum
--> ui-tests/invalid-enums.rs:63:5
|
63 | C, // = 3 -> collision
| ^