-
Notifications
You must be signed in to change notification settings - Fork 762
Support repr(C) for rustified enums #3265
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
base: main
Are you sure you want to change the base?
Conversation
33d5339
to
1885dc0
Compare
Note addition of rust-lang#3265
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.
Thanks! One nit
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.
Actually, if we implement the suggested check for enum class Foo : uint8_t
and so, we might not even need to add this switch? We could just repr(C) by default...
I think with this patch if you have something like:
enum class Foo : uint8_t {
};
Using this option will miscompile it.
At the very least we should add a big warning on this option since it'd cause wrong struct layout.
I agree this would be good. However, it appears that LLVM does not expose the necessary APIs. Even if we exposed the necessary functionality, it would presumably only work for recent clang versions. Thoughts on how to proceed? |
It is not possible to control the repr via custom attributes so add a new rustified enum variant which does not use repr(u*) or repr(i*). Using repr(C) is sometimes necessary to bindgen enums used in functions subject to cross-language CFI checks. Closes 3263. Link: https://rcvalle.com/docs/rust-cfi-design-doc/ Signed-off-by: Per Larsen <[email protected]>
Note addition of rust-lang#3265
ec7b724
to
d020ef1
Compare
TODOs:
--rustified-non-exhaustive-repr-c-enum
flag?Closes #3263.