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

Conversation

RunDevelopment
Copy link
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
Collaborator

@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!

crates/macro-support/src/parser.rs Outdated Show resolved Hide resolved
@daxpedda daxpedda added the waiting for author Waiting for author to respond label Oct 9, 2024
Copy link
Collaborator

@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
Contributor Author

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

Done :)

@daxpedda daxpedda merged commit 02679e1 into rustwasm:main Oct 10, 2024
41 checks passed
@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