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 optional zerocopy support #62

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,18 @@ version = "^1.0.0"
default-features = false
optional = true

[dependencies.zerocopy]
version = "0.8.9"
default-features = false
optional = true
[dependencies.zerocopy-derive]
version = "0.8.9"
default-features = false
optional = true

[features]
std = []
zerocopy = ["dep:zerocopy", "dep:zerocopy-derive"]

[workspace]
members = [
Expand Down
76 changes: 76 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
//!
//! - [`serde`](https://serde.rs/) implements `Serialize` and `Deserialize`
//! for `BitFlags<T>`.
//! - [`zerocopy`](https://github.com/google/zerocopy/) implements `Immutable`, `IntoBytes`,
//! `FromZeros`, `TryFromBytes`, and `KnownLayout` for all `BitFlags<T>` and `Unaligned` if the value type is unaligned.
//! - `std` implements `std::error::Error` for `FromBitsError`.
//!
//! ## `const fn`-compatible APIs
Expand Down Expand Up @@ -523,6 +525,14 @@ pub use crate::const_api::ConstToken;
/// `BitFlags` value where that isn't the case is only possible with
/// incorrect unsafe code.
#[derive(Copy, Clone)]
#[cfg_attr(
feature = "zerocopy",
derive(
zerocopy_derive::Immutable,
zerocopy_derive::KnownLayout,
zerocopy_derive::IntoBytes,
)
)]
#[repr(transparent)]
pub struct BitFlags<T, N = <T as _internal::RawBitFlags>::Numeric> {
val: N,
Expand Down Expand Up @@ -1032,3 +1042,69 @@ mod impl_serde {
}
}
}

#[cfg(feature = "zerocopy")]
mod impl_zerocopy {
use super::{BitFlag, BitFlags};
use zerocopy::{FromZeros, Immutable, TryFromBytes, Unaligned};

// All zeros is always valid
unsafe impl<T> FromZeros for BitFlags<T>
where
T: BitFlag,
T::Numeric: Immutable,
T::Numeric: FromZeros,
{
fn only_derive_is_allowed_to_implement_this_trait() {}
Copy link
Owner

Choose a reason for hiding this comment

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

I think this could use a comment linking to google/zerocopy#287, so that this doesn't scream "crimes! damn crimes!" at anyone who looks at this later.

}

// Mark all BitFlags as Unaligned if the underlying number type is unaligned
unsafe impl<T> Unaligned for BitFlags<T>
where
T: BitFlag,
T::Numeric: Unaligned,
{
fn only_derive_is_allowed_to_implement_this_trait() {}
}

// Assert that there are no invalid bytes set
unsafe impl<T> TryFromBytes for BitFlags<T>
where
T: BitFlag,
T::Numeric: Immutable,
T::Numeric: TryFromBytes,
{
fn only_derive_is_allowed_to_implement_this_trait()
where
Self: Sized,
{
}

#[inline]
fn is_bit_valid<
ZerocopyAliasing: zerocopy::pointer::invariant::Aliasing
+ zerocopy::pointer::invariant::AtLeast<zerocopy::pointer::invariant::Shared>,
>(
candidate: zerocopy::Maybe<'_, Self, ZerocopyAliasing>,
) -> bool {
// SAFETY:
// - The cast preserves address. The caller has promised that the
// cast results in an object of equal or lesser size, and so the
// cast returns a pointer which references a subset of the bytes
// of `p`.
// - The cast preserves provenance.
// - The caller has promised that the destination type has
// `UnsafeCell`s at the same byte ranges as the source type.
let candidate = unsafe { candidate.cast_unsized::<T::Numeric, _>(|p| p as *mut _) };

// SAFETY: The caller has promised that the referenced memory region
// will contain a valid `$repr`.
let my_candidate =
unsafe { candidate.assume_validity::<zerocopy::pointer::invariant::Valid>() };
{
(my_candidate.read_unaligned::<zerocopy::pointer::BecauseImmutable>() ^ T::ALL_BITS)
== T::EMPTY
}
Copy link
Owner

Choose a reason for hiding this comment

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

This bit manipulation looks suspicious. Please add tests that exercise this. Or, maybe something like BitFlags::from_bits_truncate(x).bits() == x would be better, in terms of code reuse.

}
}
}
11 changes: 10 additions & 1 deletion test_suite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ edition = "2018"

[dependencies.enumflags2]
path = "../"
features = ["serde"]
features = ["serde", "zerocopy"]

[dependencies.serde]
version = "1"
features = ["derive"]

[dependencies.zerocopy]
version = "0.8.9"
features = ["derive"]

[dev-dependencies]
trybuild = "1.0"
glob = "0.3"
Expand Down Expand Up @@ -65,3 +69,8 @@ edition = "2018"
name = "not_literal"
path = "tests/not_literal.rs"
edition = "2018"

[[test]]
name = "zerocopy"
path = "tests/zerocopy.rs"
edition = "2018"
32 changes: 32 additions & 0 deletions test_suite/tests/zerocopy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use enumflags2::{bitflags, BitFlags};
use zerocopy::{Immutable, IntoBytes, KnownLayout, TryFromBytes};

#[test]
fn zerocopy_compile() {
#[bitflags]
#[derive(Copy, Clone, Debug, KnownLayout)]
#[repr(u8)]
enum TestU8 {
A,
B,
C,
D,
}

#[bitflags]
#[derive(Copy, Clone, Debug, KnownLayout)]
#[repr(u16)]
enum TestU16 {
A,
B,
C,
D,
}

#[derive(Clone, Debug, Immutable, TryFromBytes, IntoBytes, KnownLayout)]
#[repr(packed)]
struct Other {
flags2: BitFlags<TestU8>,
flags: BitFlags<TestU16>,
}
}