-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Allow enum discriminants to not be uint, and use smallest possible size by default. #9006
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
Also export enum attrs into metadata, and add a convenient interface for obtaining the repr hint from either a local or remote definition.
Note that raising an error during trans doesn't stop the compile or cause rustc to exit with a failure status, currently, so this is of more than cosmetic importance.
The variant used in debug-info/method-on-enum.rs had its layout changed by the smaller discriminant, so that the `u32` no longer overlaps both of the `u16`s, and thus the debugger is printing partially uninitialized data when it prints the wrong variant. Thus, the test runner is modified to accept wildcards (using a string that should be unlikely to occur literally), to allow for this.
f?(@brson, @nikomatsakis, …); feel free to tag whoever else looks after these parts of the code these days. |
@bors: retry |
(cc @kemurphy) |
let discriminant_type_metadata = |inttype| { | ||
let discriminant_llvm_type = adt::ll_inttype(cx, inttype); | ||
let (discriminant_size, discriminant_align) = size_and_align_of(cx, discriminant_llvm_type); | ||
let discriminant_type_metadata = type_metadata(cx, match inttype { |
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.
Instead of duplicating the logic here, I'd rather use adt::ty_of_inttype()
so it doesn't go out of sync.
This looks great |
match item.node { | ||
ast::MetaWord(word) => { | ||
let word: &str = word; | ||
let hint = match word { |
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.
This could be match word.as_slice() {
rather than using the temporary &str
manually. (Just a note; it doesn't matter particularly.)
Does this happen to have any bearing on #8761? |
@jld: there's a real failure in the second merge/test attempt |
@jld Is it possible to use type aliases with this? Like, |
Closing due to lack of activity -- please reopen if necessary! |
Yes, that definitely is an error in the second test run, and I'm going to have to alter the visitor interface to fix it.... |
Superseded by #9613. (Making a new PR so I can change the branch name to not say “for feedback”.) Answering lingering questions:
|
feat(fix): ignore `todo!` and `unimplemented!` in `if_same_then_else` close: rust-lang#8836 take over: rust-lang#8853 This PR adds check `todo!` and `unimplemented!` in if_same_then_else. ( I thought `unimplemented` should not be checked as well as todo!.) Thank you in advance. changelog: ignore todo! and unimplemented! in if_same_then_else r? `@Jarcho`
This is the long-awaited branch to allow enum discriminants to be any integral type. The default is the smallest one that can represent the possible values, but this can be overridden by an attribute, either to a specific type or to whatever the platform's C ABI would use. The
ctypes
lint pass is extended to detect non-FFI-safe enums in foreign item types.I've fixed the months of bit rot, and it passes
make check
on x86_64 Linux, and I've squashed it into a reasonably comprehensible form — but it definitely needs more tests, and I think some of the commit messages could stand to be more informative. The former could be addressed in a followup PR, to reduce the possibility of further bit rot.