-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Implement non-exhaustive enums #4191
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
Conversation
* error for '_' prong on exhaustive enum * todo panic for `@tagName` on non-exhaustive enum * don't require '_' field on tagged unions
andrewrk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this is substantial language progress.
src/analyze.cpp
Outdated
| add_node_error(g, field_node, buf_sprintf("non-exhaustive enum must specify size")); | ||
| enum_type->data.enumeration.resolve_status = ResolveStatusInvalid; | ||
| } | ||
| if (field_count > 1 && log2_u64(field_count - 1) == enum_type->size_in_bits) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this impossible to trigger for a non-extern enum? The enum tag size is chosen before the _ field is skipped.
Moreover the field_count is now invalid and enumeration.fields contains one entry too many.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag size must be specified for non-exhaustive enums so it is possible to hit.
const E = enum(u2) {
a,
b,
c,
d,
_, // error: non-exhaustive enum specifies every value
};a3faf7c to
6c8f01d
Compare
andrewrk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I consider this to be mergeable right now, but I'll leave it up to you if you want to take the test case suggestion before I hit the button.
Closes #2524
Any suggestions to improve the documentation are welcome.