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

fix: error on non-exhaustive enums with zero width backing type #21374

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WillLillis
Copy link
Sponsor Contributor

Closes #19856

@rohlem
Copy link
Contributor

rohlem commented Sep 10, 2024

Thanks for the fix!
Small nitpick, but re-testing I noticed that _ = enum(u1) {a, b, c}; results in error: enumeration value '2' too large for type 'u1'.
However, when adding _ to the fields, this compile error is overruled by error: non-exhaustive enum specifies every value.
IMO the order should be reversed, i.e. "enumeration value too large" is more important/pressing,
because if you remove _ and then get the error that the tag type is still too small,
you might increase the tag type again, in which case you might want it to be non-exhaustive again.

Just my opinion though, probably shouldn't block this PR, and it can be a separate discussion if we don't all agree on this.

@WillLillis
Copy link
Sponsor Contributor Author

Thanks for the fix!
Small nitpick, but re-testing I noticed that _ = enum(u1) {a, b, c}; results in error: enumeration value '2' too large for type 'u1'.
However, when adding _ to the fields, this compile error is overruled by error: non-exhaustive enum specifies every value. IMO the order should be reversed, i.e. "enumeration value too large" is more important/pressing, because if you remove _ and then get the error that the tag type is still too small, you might increase the tag type again, in which case you might want it to be non-exhaustive again.

Just my opinion though, probably shouldn't block this PR, and it can be a separate discussion if we don't all agree on this.

Sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing compile error for enum(u0){a,_}
2 participants