From f840723c511100fb8817624b6aa927e5d78fc4b3 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Sat, 18 May 2024 09:53:18 -0700 Subject: [PATCH] [error] Implement std::error::Error on errors While we're here, also relax `Dispaly for AlignmentError` to permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an important relaxation since our APIs permit performing conversions into unsized destination types with runtime alignment checking. Also make all errors `Send + Sync` regardless of `Dst`, which only exists at the type level, but is never instantiated. Makes progress on #1297 --- Cargo.toml | 3 +- README.md | 5 +++ src/error.rs | 111 ++++++++++++++++++++++++++++++++++++++------------- src/lib.rs | 7 +++- src/util.rs | 26 ++++++++++++ 5 files changed, 123 insertions(+), 29 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 84b61e9cb7..449da6114c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,10 +60,11 @@ alloc = [] derive = ["zerocopy-derive"] simd = [] simd-nightly = ["simd"] +std = ["alloc"] # This feature depends on all other features that work on the stable compiler. # We make no stability guarantees about this feature; it may be modified or # removed at any time. -__internal_use_only_features_that_work_on_stable = ["alloc", "derive", "simd"] +__internal_use_only_features_that_work_on_stable = ["alloc", "derive", "simd", "std"] [dependencies] zerocopy-derive = { version = "=0.8.0-alpha.14", path = "zerocopy-derive", optional = true } diff --git a/README.md b/README.md index bf9974882a..0664c998cd 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,11 @@ for network parsing. the `alloc` crate is added as a dependency, and some allocation-related functionality is added. +- **`std`** + By default, `zerocopy` is `no_std`. When the `std` feature is enabled, the + `std` crate is added as a dependency (ie, `no_std` is disabled), and + support for some `std` types is added. `std` implies `alloc`. + - **`derive`** Provides derives for the core marker traits via the `zerocopy-derive` crate. These derives are re-exported from `zerocopy`, so it is not diff --git a/src/error.rs b/src/error.rs index fff9e4d36e..1eb8595213 100644 --- a/src/error.rs +++ b/src/error.rs @@ -23,9 +23,9 @@ //! All error types provide an `into_src` method that converts the error into //! the source value underlying the failed conversion. -use core::{convert::Infallible, fmt, marker::PhantomData, ops::Deref}; +use core::{convert::Infallible, fmt, ops::Deref}; -use crate::TryFromBytes; +use crate::{util::SendSyncPhantomData, KnownLayout, TryFromBytes}; #[cfg(doc)] use crate::{FromBytes, Ref}; @@ -82,18 +82,27 @@ impl fmt::Display for Convert } } +#[cfg(any(feature = "std", test))] +impl std::error::Error for ConvertError +where + A: fmt::Display + fmt::Debug, + S: fmt::Display + fmt::Debug, + V: fmt::Display + fmt::Debug, +{ +} + /// The error emitted if the conversion source is improperly aligned. #[derive(PartialEq, Eq)] pub struct AlignmentError { /// The source value involved in the conversion. src: Src, /// The inner destination type inolved in the conversion. - dst: PhantomData, + dst: SendSyncPhantomData, } impl AlignmentError { pub(crate) fn new(src: Src) -> Self { - Self { src, dst: PhantomData } + Self { src, dst: SendSyncPhantomData::default() } } /// Produces the source underlying the failed conversion. @@ -103,11 +112,11 @@ impl AlignmentError { } pub(crate) fn with_src(self, new_src: NewSrc) -> AlignmentError { - AlignmentError { src: new_src, dst: PhantomData } + AlignmentError { src: new_src, dst: SendSyncPhantomData::default() } } pub(crate) fn map_src(self, f: impl Fn(Src) -> NewSrc) -> AlignmentError { - AlignmentError { src: f(self.src), dst: PhantomData } + AlignmentError { src: f(self.src), dst: SendSyncPhantomData::default() } } pub(crate) fn into(self) -> ConvertError { @@ -126,9 +135,10 @@ impl fmt::Debug for AlignmentError { // The bounds on this impl are intentionally conservative, and can be relaxed // either once a `?Sized` alignment accessor is stabilized, or by storing the // alignment as a runtime value. -impl fmt::Display for AlignmentError +impl fmt::Display for AlignmentError where Src: Deref, + Dst: KnownLayout, { #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -139,14 +149,22 @@ where f.write_str("the conversion failed because the address of the source (a multiple of ")?; addr_align.fmt(f)?; f.write_str(") is not a multiple of the alignment (")?; - core::mem::align_of::().fmt(f)?; + ::LAYOUT.align.get().fmt(f)?; f.write_str(") of the destination type: ")?; f.write_str(core::any::type_name::())?; Ok(()) } } -impl From> +#[cfg(any(feature = "std", test))] +impl std::error::Error for AlignmentError +where + Src: Deref, + Dst: KnownLayout, +{ +} + +impl From> for ConvertError, S, V> { #[inline] @@ -161,12 +179,12 @@ pub struct SizeError { /// The source value involved in the conversion. src: Src, /// The inner destination type inolved in the conversion. - dst: PhantomData, + dst: SendSyncPhantomData, } impl SizeError { pub(crate) fn new(src: Src) -> Self { - Self { src, dst: PhantomData } + Self { src, dst: SendSyncPhantomData::default() } } /// Produces the source underlying the failed conversion. @@ -177,17 +195,17 @@ impl SizeError { /// Sets the source value associated with the conversion error. pub(crate) fn with_src(self, new_src: NewSrc) -> SizeError { - SizeError { src: new_src, dst: PhantomData } + SizeError { src: new_src, dst: SendSyncPhantomData::default() } } /// Maps the source value associated with the conversion error. pub(crate) fn map_src(self, f: impl Fn(Src) -> NewSrc) -> SizeError { - SizeError { src: f(self.src), dst: PhantomData } + SizeError { src: f(self.src), dst: SendSyncPhantomData::default() } } /// Sets the destination type associated with the conversion error. pub(crate) fn with_dst(self) -> SizeError { - SizeError { src: self.src, dst: PhantomData } + SizeError { src: self.src, dst: SendSyncPhantomData::default() } } /// Converts the error into a general [`ConvertError`]. @@ -204,10 +222,7 @@ impl fmt::Debug for SizeError { } /// Produces a human-readable error message. -impl fmt::Display for SizeError -where - Src: Deref, -{ +impl fmt::Display for SizeError { #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("the conversion failed because the source was incorrectly sized to complete the conversion into the destination type: ")?; @@ -216,7 +231,10 @@ where } } -impl From> for ConvertError, V> { +#[cfg(any(feature = "std", test))] +impl std::error::Error for SizeError {} + +impl From> for ConvertError, V> { #[inline] fn from(err: SizeError) -> Self { Self::Size(err) @@ -229,12 +247,12 @@ pub struct ValidityError { /// The source value involved in the conversion. pub(crate) src: Src, /// The inner destination type inolved in the conversion. - dst: PhantomData, + dst: SendSyncPhantomData, } impl ValidityError { pub(crate) fn new(src: Src) -> Self { - Self { src, dst: PhantomData } + Self { src, dst: SendSyncPhantomData::default() } } /// Produces the source underlying the failed conversion. @@ -245,7 +263,7 @@ impl ValidityError { /// Maps the source value associated with the conversion error. pub(crate) fn map_src(self, f: impl Fn(Src) -> NewSrc) -> ValidityError { - ValidityError { src: f(self.src), dst: PhantomData } + ValidityError { src: f(self.src), dst: SendSyncPhantomData::default() } } /// Converts the error into a general [`ConvertError`]. @@ -262,10 +280,7 @@ impl fmt::Debug for ValidityError { } /// Produces a human-readable error message. -impl fmt::Display for ValidityError -where - Src: Deref, -{ +impl fmt::Display for ValidityError { #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("the conversion failed because the source bytes are not a valid value of the destination type: ")?; @@ -274,6 +289,9 @@ where } } +#[cfg(any(feature = "std", test))] +impl std::error::Error for ValidityError {} + impl From> for ConvertError> { @@ -395,9 +413,46 @@ impl TryReadError { } #[cfg(test)] -mod test { +mod tests { use super::*; + #[test] + fn test_send_sync() { + // Test that all error types are `Send + Sync` even if `Dst: !Send + + // !Sync`. + + #[allow(dead_code)] + fn is_send_sync(_t: T) {} + + #[allow(dead_code)] + fn alignment_err_is_send_sync(err: AlignmentError) { + is_send_sync(err) + } + + #[allow(dead_code)] + fn size_err_is_send_sync(err: SizeError) { + is_send_sync(err) + } + + #[allow(dead_code)] + fn validity_err_is_send_sync( + err: ValidityError, + ) { + is_send_sync(err) + } + + #[allow(dead_code)] + fn convert_error_is_send_sync( + err: ConvertError< + AlignmentError, + SizeError, + ValidityError, + >, + ) { + is_send_sync(err) + } + } + #[test] fn alignment_display() { #[repr(C, align(128))] @@ -405,6 +460,8 @@ mod test { bytes: [u8; 128], } + impl_known_layout!(elain::Align::<8>); + let aligned = Aligned { bytes: [0; 128] }; assert_eq!( diff --git a/src/lib.rs b/src/lib.rs index 084d5fe743..9f42ec34a5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,6 +82,11 @@ //! the `alloc` crate is added as a dependency, and some allocation-related //! functionality is added. //! +//! - **`std`** +//! By default, `zerocopy` is `no_std`. When the `std` feature is enabled, the +//! `std` crate is added as a dependency (ie, `no_std` is disabled), and +//! support for some `std` types is added. `std` implies `alloc`. +//! //! - **`derive`** //! Provides derives for the core marker traits via the `zerocopy-derive` //! crate. These derives are re-exported from `zerocopy`, so it is not @@ -255,7 +260,7 @@ clippy::arithmetic_side_effects, clippy::indexing_slicing, ))] -#![cfg_attr(not(test), no_std)] +#![cfg_attr(not(any(test, feature = "std-error")), no_std)] #![cfg_attr( all(feature = "simd-nightly", any(target_arch = "x86", target_arch = "x86_64")), feature(stdarch_x86_avx512) diff --git a/src/util.rs b/src/util.rs index f91e9475f1..2f069fc872 100644 --- a/src/util.rs +++ b/src/util.rs @@ -8,6 +8,7 @@ use core::{ cell::UnsafeCell, + marker::PhantomData, mem::{self, ManuallyDrop, MaybeUninit}, num::{NonZeroUsize, Wrapping}, ptr::NonNull, @@ -431,6 +432,31 @@ safety_comment! { ); } +/// Like [`PhantomData`], but [`Send`] and [`Sync`] regardless of whether the +/// wrapped `T` is. +pub(crate) struct SendSyncPhantomData(PhantomData); + +// SAFETY: `SendSyncPhantomData` does not enable any behavior which isn't sound +// to be called from multiple threads. +unsafe impl Send for SendSyncPhantomData {} +// SAFETY: `SendSyncPhantomData` does not enable any behavior which isn't sound +// to be called from multiple threads. +unsafe impl Sync for SendSyncPhantomData {} + +impl Default for SendSyncPhantomData { + fn default() -> SendSyncPhantomData { + SendSyncPhantomData(PhantomData) + } +} + +impl PartialEq for SendSyncPhantomData { + fn eq(&self, other: &Self) -> bool { + self.0.eq(&other.0) + } +} + +impl Eq for SendSyncPhantomData {} + pub(crate) trait AsAddress { fn addr(self) -> usize; }