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

Should repr(C) enums be linted? #3229

Open
HadrienG2 opened this issue Sep 27, 2018 · 2 comments
Open

Should repr(C) enums be linted? #3229

HadrienG2 opened this issue Sep 27, 2018 · 2 comments
Labels
A-lint Area: New lints S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@HadrienG2
Copy link

HadrienG2 commented Sep 27, 2018

The domain of applicability of #[repr(C)] enums is much smaller than people new to FFI think, and much existing usage of it is erronerous. Two common C enum idioms which are not supported by Rust enums come to bite people particularly often:

  • In C, receiving an unknown enum variant from a more recent version of an enum-based library is mostly legal, it will only test the quality of the "default:" clause of your switch statement. In Rust, that's undefined behavior.
  • In C, people often (mis-)use enum's implicit conversion to integer as a more portable alternative to bitfields or as a way to count the amount of enum variants. In Rust, randomly BitOr-ing enum variants or subtracting things from them doesn't make sense.

From this perspective, I would join other people who consider repr(C) enums as an FFI footgun, and suggest linting people towards using integer newtypes and constants instead.

That's not always the right thing to do, there are some cases where the set of enum variants in a C API is known to be closed to further modification forever and where enums are only used as enumerated cases, which is why a lint that can be silenced instead of a hard error is most appropriate.

@Manishearth
Copy link
Member

Hmm. I've definitely been bitten by this before, but also this is a pretty common pattern to use in a legit way.

I kinda wish we had "btw, watch out for this" lints that you dismiss once and it remembers that you dismissed it (without needing explicit annotation). This is a bit tricky to make work.

@vlisivka
Copy link

vlisivka commented Nov 8, 2018

Maybe it's better to fix problem with repr(C) enum's and allow them to be unsafe in unsafe blocks?

@phansch phansch added A-lint Area: New lints S-needs-discussion Status: Needs further discussion before merging or work can be started labels Dec 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

4 participants