Add support for bytemuck by KodrAus · Pull Request #336 · bitflags/bitflags · GitHub
Skip to content

Add support for bytemuck - #336

Merged
KodrAus merged 3 commits into
bitflags:mainfrom
KodrAus:feat/bytemuck
Apr 17, 2023
Merged

KodrAus merged 3 commits into
bitflags:mainfrom
KodrAus:feat/bytemuck

Conversation

@KodrAus

@KodrAus KodrAus commented Apr 11, 2023

Copy link
Copy Markdown
Member

Closes #311

I've also added some docs on how to support external libraries here for future contributors. Since the bytemuck impls themselves are unsafe, I've added a note to where we generate our internal flags type warning that its ABI can't be changed.

@KodrAus

KodrAus commented Apr 11, 2023

Copy link
Copy Markdown
Member Author

Comment thread src/lib.rs
pub use core;

#[cfg(feature = "serde")]
pub use serde;

@KodrAus KodrAus Apr 11, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These have moved into the external module alongside their supporting code

Comment thread src/public.rs
) => {
$(#[$outer])*
$vis struct $BitFlags(<Self as $crate::__private::PublicFlags>::Internal);
$vis struct $BitFlags(<$BitFlags as $crate::__private::PublicFlags>::Internal);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was getting a build error with Self being unknown here. Not sure if it was bytemuck's derives or a change in rustc but it was easily fixed.

@KodrAus

KodrAus commented Apr 17, 2023 •

Copy link
Copy Markdown
Member Author

cc @Lokathor 👋

I just wanted to make sure I've got these Pod and Zeroable impls right 🙂 In bitflags, we can automatically implement them on an internal type that looks like:

#[repr(transparent)]
struct FlagsField<T> {
    bits: T
}

where T may be a numeric type, like i32, usize etc. We may allow you to specify this type as something you've invented at some point in the future though. The type will always be a #[repr(transparent)] struct with a single field though.

The impls I've come up with look like this. Does that look right to you? Is there anything else we need to guarantee to make sure these impls are safe to do?

@Lokathor

Copy link
Copy Markdown

yep, impl<T> Pod for ReprTransparentIntNewtype<T> where T: Pod is basically exactly what I was expecting to see before I clicked, and then it's what I saw.

@KodrAus

KodrAus commented Apr 17, 2023

Copy link
Copy Markdown
Member Author

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.

Support bytemuck

2 participants