add #[repr(transparent)] by flier · Pull Request #187 · bitflags/bitflags · GitHub
Skip to content

add #[repr(transparent)] - #187

Merged
KodrAus merged 1 commit into
bitflags:masterfrom
flier:repr_transparent
Jul 19, 2021
Merged

KodrAus merged 1 commit into
bitflags:masterfrom
flier:repr_transparent

Conversation

@flier

@flier flier commented Jul 26, 2019

Copy link
Copy Markdown
Contributor

add #[repr(transparent)] for the the FFI layout, replace #184.

@KodrAus

KodrAus commented Jul 29, 2019

Copy link
Copy Markdown
Member

@josephlr

Copy link
Copy Markdown

I think this is a reasonable change and it would help me avoid boilerplate w/ FFI stuff.

I don't think an MSRV bump is needed, we can just check the version in the build.rs like we do for const fn stuff.

@KodrAus does that sound reasonable? @flier I'm happy to take this over if you don't have time.

@flier

flier commented May 3, 2020

Copy link
Copy Markdown
Contributor Author

Hi @josephlr

I have added a bitflags_repr_transparent feature gate for the #[repr(transparent)] attribute, could you help me to review the changes?
Thanks

@elmarco

elmarco commented Apr 22, 2021

Copy link
Copy Markdown

Hi @flier, can you update the MR? thanks

@KodrAus

KodrAus commented May 16, 2021

Copy link
Copy Markdown
Member

Thanks for your patience on this one @flier. Wow I really let this sit for a long time! 1.28.0 is now really quite old so I think we should be comfortable just bumping to it if we haven't already and avoid needing to add feature gates.

I was wondering whether #[repr(transparent)] came with a bit pattern guarantee (in bitflags we currently enforce that all set bits correspond to valid flags, so not all bit patterns of the underlying integer are necessarily valid bitflags), but since non-zero types like NonZeroUsize are also #[repr(transparent)] I think we're safe here.

If you'd like to update this PR to just unconditionally add the #[repr(transparent)] attribute or if somebody else would like to add that we can merge this in.

@KodrAus

KodrAus commented May 16, 2021

Copy link
Copy Markdown
Member

In #240 we bumped all the way up to 1.45.0, so can definitely unconditionally add the #[repr(transparent)] flag now 🙂

@flier
flier force-pushed the repr_transparent branch from 43da098 to 869ff95 Compare May 20, 2021 03:17

@KodrAus KodrAus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @flier! This looks good to me.

@KodrAus
KodrAus merged commit efab7f3 into bitflags:master Jul 19, 2021
@elmarco

elmarco commented Jul 19, 2021

Copy link
Copy Markdown

thanks! release time? :)

@njaard

njaard commented Aug 11, 2021

Copy link
Copy Markdown

This breaks at least the crates nix and rusqlite, because they both use #[repr(C)] on "enum structs"

@mockersf

Copy link
Copy Markdown

also wgpu-types and naga, which already added #[repr(transparent)]

error[E0692]: transparent struct cannot have other repr hints
    --> /.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-types-0.7.0/src/lib.rs:2791:12
     |
2779 | / bitflags::bitflags! {
2780 | |     /// Flags for which pipeline data should be recorded.
2781 | |     ///
2782 | |     /// The amount of values written when resolved depends
...    |
2791 | |     #[repr(transparent)]
     | |            ^^^^^^^^^^^
...    |
2812 | |     }
2813 | | }
     | |_^
     |
     = note: this error originates in the macro `bitflags::bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)

@KodrAus

KodrAus commented Aug 11, 2021

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants