Skip to content

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

Merged
daxpedda merged 6 commits intowasm-bindgen:mainfrom
RunDevelopment:Implicit-discriminants
Oct 10, 2024
Merged

Add support for C-style enums with implicit discriminants#4152
daxpedda merged 6 commits intowasm-bindgen:mainfrom
RunDevelopment:Implicit-discriminants

Conversation

@RunDevelopment
Copy link
Copy Markdown
Contributor

fixes #2273

Changes:

  1. Allow implicit discriminant by using the same algorithm as rustc. This behavior is explicitly documented and guaranteed by the compiler.
  2. Simpler algorithm for finding holes.

1. Implicit discriminants

I won't go into the algorithm for determining implicit discriminants, just read the docs. I just want to talk about a few details.

Firstly, while the behavior of implicit discriminants is consistent with rustc, everything would still work correctly even if it wasn't. Since we generate both the Rust and JS/TS enum with all discriminants explicitly specified, there will never a case where Rust and JS/TS disagree on what number a certain variant is. The worst that can happen is that we assign implicit discriminants numbers that users don't expect. (But again, we do it the same way rustc does, so it should be what users expect.)

Secondly, TS enums use the same algorithm for assigning implicit discriminants. The only difference is that TS allows duplicate discriminants while Rust doesn't. So JS/TS devs copying over TS enums with duplicate discriminants will result in a compiler error with and without #[wasm_bindgen].

2. Holes

Since I keep track of all discriminant values for better error messages, I implemented a simpler algorithm for finding holes. It's guaranteed to find the hole with the smallest numeric value and doesn't have issues with arithmetic overflow.

Note that the old algorithm didn't always find the hole with smallest numeric value, so Wasm bindgen might use different holes now. E.g. enum E { A = 1, B = 3 } previously used the hole 2 but now uses 0. I don't think this is an issue, but I wanted to mention it.

Copy link
Copy Markdown
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit, otherwise looks great.
Thank you!

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Oct 9, 2024
Copy link
Copy Markdown
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

The changelog entry needs to be moved to a new "Unreleased" section.

@RunDevelopment
Copy link
Copy Markdown
Contributor Author

The changelog entry needs to be moved to a new "Unreleased" section.

Done :)

@daxpedda daxpedda merged commit 02679e1 into wasm-bindgen:main Oct 10, 2024
@RunDevelopment RunDevelopment deleted the Implicit-discriminants branch October 10, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting for author Waiting for author to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enum: Enable annotating discriminant of only some variants

2 participants