Initial bitflags migration

[richo]

I had an initial stab at this, basically picking the easiest looking thing in that gist.

This is really just a sanity check for the approach, although I'm pretty tempted to do this in small chunks at least at first to see what the impact is in 3rd parties.

Updates #14207
/cc @aturon

[aturon]

Thanks for getting to this so quickly! This looks like the right approach.

BTW: I'm currently trying to land a patch that will change the signature of from_bits to return an Option (reflecting whether the bits are a valid combination of flags) and adds a from_bits_truncate that just drops any irrelevant bits.

You'll want to use one or the other of these functions for converting from an integer representation. You should consider in each case whether extra bits should cause a noisy failure or just be silently ignored, and use the corresponding function.

[alexcrichton]

This may actually be one case where the enum should stay, but it should be tagged with #[repr(C)] but it wasn't in the past due to us having ABI issues with enums (which I think are now fixed).

The LLVM definitions for the functions take an LLVMAttribute which is itself an enum, so this may not want to change to bitflags! just yet

[aturon]

@alexcrichton Interesting! Most of the discriminants are powers of 2, except for StackAttribute and AlignmentAttribute. Does LLVM treat that attribute as a "set" of other attributes?

If so, maybe the point here is that LLVM doesn't want all combinations of these attributes? But of course bitflags is designed to allow free combination. So bitflags just wouldn't be expressive enough to capture the constraints statically?

[alexcrichton]

I think that LLVM internally or's together these variants into its own bit set, but the API level only ever takes one at a time.

This may just be an FFI case where the type of the argument really is just an enum with a specific value, rather than bitflags not being able to be expressive enough. I think that the ABI is technically not correct today because it's specified to be a uint where it should be an enum, and it just so happens that all relevant ABIs we use have those two coincide.

[richo]

Ok, cool. I'll close this and go looking for something more plausible.

Thanks for the feedback on the general approach though- appreciate it.