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

Add support for bytemuck #336

Merged
merged 3 commits into from Apr 17, 2023
Merged

Add support for bytemuck #336

merged 3 commits into from Apr 17, 2023

Conversation

KodrAus
Copy link
Member

@KodrAus KodrAus commented Apr 11, 2023

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
Copy link
Member Author

KodrAus commented Apr 11, 2023

cc @MarijnS95 do these look right to you? I haven't used bytemuck myself.

@@ -408,12 +409,6 @@ pub mod __private {
pub use crate::{external::*, traits::*};

pub use core;

#[cfg(feature = "serde")]
pub use serde;
Copy link
Member Author

@KodrAus KodrAus Apr 11, 2023

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

@@ -14,7 +14,7 @@ macro_rules! __declare_public_bitflags {
$vis:vis struct $BitFlags:ident;
) => {
$(#[$outer])*
$vis struct $BitFlags(<Self as $crate::__private::PublicFlags>::Internal);
$vis struct $BitFlags(<$BitFlags as $crate::__private::PublicFlags>::Internal);
Copy link
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
Copy link
Member Author

KodrAus commented Apr 17, 2023

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

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
Copy link
Member Author

KodrAus commented Apr 17, 2023

Awesome, thanks for checking them out! 🙇 I'll get this rolling through now.

@KodrAus KodrAus merged commit dc97104 into bitflags:main Apr 17, 2023
15 checks passed
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