Skip to content

Commit

Permalink
[error] Implement std::error::Error on errors
Browse files Browse the repository at this point in the history
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` 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
  • Loading branch information
joshlf authored and jswrenn committed May 18, 2024
1 parent 1c77a9d commit 235cd25
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 27 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
121 changes: 96 additions & 25 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -82,18 +82,27 @@ impl<A: fmt::Display, S: fmt::Display, V: fmt::Display> fmt::Display for Convert
}
}

#[cfg(any(feature = "std", test))]
impl<A, S, V> std::error::Error for ConvertError<A, S, V>
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<Src, Dst: ?Sized> {
/// The source value involved in the conversion.
src: Src,
/// The inner destination type inolved in the conversion.
dst: PhantomData<Dst>,
dst: SendSyncPhantomData<Dst>,
}

impl<Src, Dst: ?Sized> AlignmentError<Src, Dst> {
pub(crate) fn new(src: Src) -> Self {
Self { src, dst: PhantomData }
Self { src, dst: SendSyncPhantomData::default() }
}

/// Produces the source underlying the failed conversion.
Expand All @@ -103,11 +112,11 @@ impl<Src, Dst: ?Sized> AlignmentError<Src, Dst> {
}

pub(crate) fn with_src<NewSrc>(self, new_src: NewSrc) -> AlignmentError<NewSrc, Dst> {
AlignmentError { src: new_src, dst: PhantomData }
AlignmentError { src: new_src, dst: SendSyncPhantomData::default() }
}

pub(crate) fn map_src<NewSrc>(self, f: impl Fn(Src) -> NewSrc) -> AlignmentError<NewSrc, Dst> {
AlignmentError { src: f(self.src), dst: PhantomData }
AlignmentError { src: f(self.src), dst: SendSyncPhantomData::default() }
}

pub(crate) fn into<S, V>(self) -> ConvertError<Self, S, V> {
Expand All @@ -126,9 +135,10 @@ impl<Src, Dst: ?Sized> fmt::Debug for AlignmentError<Src, Dst> {
// 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<Src, Dst> fmt::Display for AlignmentError<Src, Dst>
impl<Src, Dst: ?Sized> fmt::Display for AlignmentError<Src, Dst>
where
Src: Deref,
Dst: KnownLayout,
{
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -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::<Dst>().fmt(f)?;
<Dst as KnownLayout>::LAYOUT.align.get().fmt(f)?;
f.write_str(") of the destination type: ")?;
f.write_str(core::any::type_name::<Dst>())?;
Ok(())
}
}

impl<Src, Dst, S, V> From<AlignmentError<Src, Dst>>
#[cfg(any(feature = "std", test))]
impl<Src, Dst: ?Sized> std::error::Error for AlignmentError<Src, Dst>
where
Src: Deref,
Dst: KnownLayout,
{
}

impl<Src, Dst: ?Sized, S, V> From<AlignmentError<Src, Dst>>
for ConvertError<AlignmentError<Src, Dst>, S, V>
{
#[inline]
Expand All @@ -161,12 +179,12 @@ pub struct SizeError<Src, Dst: ?Sized> {
/// The source value involved in the conversion.
src: Src,
/// The inner destination type inolved in the conversion.
dst: PhantomData<Dst>,
dst: SendSyncPhantomData<Dst>,
}

impl<Src, Dst: ?Sized> SizeError<Src, Dst> {
pub(crate) fn new(src: Src) -> Self {
Self { src, dst: PhantomData }
Self { src, dst: SendSyncPhantomData::default() }
}

/// Produces the source underlying the failed conversion.
Expand All @@ -177,17 +195,17 @@ impl<Src, Dst: ?Sized> SizeError<Src, Dst> {

/// Sets the source value associated with the conversion error.
pub(crate) fn with_src<NewSrc>(self, new_src: NewSrc) -> SizeError<NewSrc, Dst> {
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<NewSrc>(self, f: impl Fn(Src) -> NewSrc) -> SizeError<NewSrc, Dst> {
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<NewDst: ?Sized>(self) -> SizeError<Src, NewDst> {
SizeError { src: self.src, dst: PhantomData }
SizeError { src: self.src, dst: SendSyncPhantomData::default() }
}

/// Converts the error into a general [`ConvertError`].
Expand All @@ -206,17 +224,26 @@ impl<Src, Dst: ?Sized> fmt::Debug for SizeError<Src, Dst> {
/// Produces a human-readable error message.
impl<Src, Dst: ?Sized> fmt::Display for SizeError<Src, Dst>
where
Src: Deref,
Dst: KnownLayout,
{
#[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: ")?;
f.write_str("the conversion failed because the source was incorrectly sized to complete the conversion into the destination type")?;
if let crate::SizeInfo::Sized { size } = Dst::LAYOUT.size_info {
f.write_str(" (")?;
size.fmt(f)?;
f.write_str(" bytes)")?;
}
f.write_str(": ")?;
f.write_str(core::any::type_name::<Dst>())?;
Ok(())
}
}

impl<Src, Dst, A, V> From<SizeError<Src, Dst>> for ConvertError<A, SizeError<Src, Dst>, V> {
#[cfg(any(feature = "std", test))]
impl<Src, Dst: ?Sized> std::error::Error for SizeError<Src, Dst> where Dst: KnownLayout {}

impl<Src, Dst: ?Sized, A, V> From<SizeError<Src, Dst>> for ConvertError<A, SizeError<Src, Dst>, V> {
#[inline]
fn from(err: SizeError<Src, Dst>) -> Self {
Self::Size(err)
Expand All @@ -229,12 +256,12 @@ pub struct ValidityError<Src, Dst: ?Sized + TryFromBytes> {
/// The source value involved in the conversion.
pub(crate) src: Src,
/// The inner destination type inolved in the conversion.
dst: PhantomData<Dst>,
dst: SendSyncPhantomData<Dst>,
}

impl<Src, Dst: ?Sized + TryFromBytes> ValidityError<Src, Dst> {
pub(crate) fn new(src: Src) -> Self {
Self { src, dst: PhantomData }
Self { src, dst: SendSyncPhantomData::default() }
}

/// Produces the source underlying the failed conversion.
Expand All @@ -245,7 +272,7 @@ impl<Src, Dst: ?Sized + TryFromBytes> ValidityError<Src, Dst> {

/// Maps the source value associated with the conversion error.
pub(crate) fn map_src<NewSrc>(self, f: impl Fn(Src) -> NewSrc) -> ValidityError<NewSrc, Dst> {
ValidityError { src: f(self.src), dst: PhantomData }
ValidityError { src: f(self.src), dst: SendSyncPhantomData::default() }
}

/// Converts the error into a general [`ConvertError`].
Expand All @@ -262,10 +289,7 @@ impl<Src, Dst: ?Sized + TryFromBytes> fmt::Debug for ValidityError<Src, Dst> {
}

/// Produces a human-readable error message.
impl<Src, Dst: ?Sized + TryFromBytes> fmt::Display for ValidityError<Src, Dst>
where
Src: Deref,
{
impl<Src, Dst: ?Sized + TryFromBytes> fmt::Display for ValidityError<Src, Dst> {
#[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: ")?;
Expand All @@ -274,6 +298,9 @@ where
}
}

#[cfg(any(feature = "std", test))]
impl<Src, Dst: ?Sized + TryFromBytes> std::error::Error for ValidityError<Src, Dst> {}

impl<Src, Dst: ?Sized + TryFromBytes, A, S> From<ValidityError<Src, Dst>>
for ConvertError<A, S, ValidityError<Src, Dst>>
{
Expand Down Expand Up @@ -395,16 +422,55 @@ impl<Src, Dst: ?Sized + TryFromBytes> TryReadError<Src, Dst> {
}

#[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: Send + Sync>(_t: T) {}

#[allow(dead_code)]
fn alignment_err_is_send_sync<Src: Send + Sync, Dst>(err: AlignmentError<Src, Dst>) {
is_send_sync(err)
}

#[allow(dead_code)]
fn size_err_is_send_sync<Src: Send + Sync, Dst>(err: SizeError<Src, Dst>) {
is_send_sync(err)
}

#[allow(dead_code)]
fn validity_err_is_send_sync<Src: Send + Sync, Dst: TryFromBytes>(
err: ValidityError<Src, Dst>,
) {
is_send_sync(err)
}

#[allow(dead_code)]
fn convert_error_is_send_sync<Src: Send + Sync, Dst: TryFromBytes>(
err: ConvertError<
AlignmentError<Src, Dst>,
SizeError<Src, Dst>,
ValidityError<Src, Dst>,
>,
) {
is_send_sync(err)
}
}

#[test]
fn alignment_display() {
#[repr(C, align(128))]
struct Aligned {
bytes: [u8; 128],
}

impl_known_layout!(elain::Align::<8>);

let aligned = Aligned { bytes: [0; 128] };

assert_eq!(
Expand Down Expand Up @@ -434,6 +500,11 @@ mod test {
SizeError::<_, [u8]>::new(&[0u8; 1][..]).to_string(),
"the conversion failed because the source was incorrectly sized to complete the conversion into the destination type: [u8]"
);

assert_eq!(
SizeError::<_, [u8; 2]>::new(&[0u8; 1][..]).to_string(),
"the conversion failed because the source was incorrectly sized to complete the conversion into the destination type (2 bytes): [u8; 2]"
);
}

#[test]
Expand Down
7 changes: 6 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -255,7 +260,7 @@
clippy::arithmetic_side_effects,
clippy::indexing_slicing,
))]
#![cfg_attr(not(test), no_std)]
#![cfg_attr(not(any(test, feature = "std")), no_std)]
#![cfg_attr(
all(feature = "simd-nightly", any(target_arch = "x86", target_arch = "x86_64")),
feature(stdarch_x86_avx512)
Expand Down
1 change: 1 addition & 0 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ macro_rules! impl_known_layout {
const _: () = {
use core::ptr::NonNull;

#[allow(non_local_definitions)]
// SAFETY: Delegates safety to `DstLayout::for_type`.
unsafe impl<$($tyvar $(: ?$optbound)?)? $(, const $constvar : $constty)?> KnownLayout for $ty {
#[allow(clippy::missing_inline_in_public_items)]
Expand Down
26 changes: 26 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use core::{
cell::UnsafeCell,
marker::PhantomData,
mem::{self, ManuallyDrop, MaybeUninit},
num::{NonZeroUsize, Wrapping},
ptr::NonNull,
Expand Down Expand Up @@ -431,6 +432,31 @@ safety_comment! {
);
}

/// Like [`PhantomData`], but [`Send`] and [`Sync`] regardless of whether the
/// wrapped `T` is.
pub(crate) struct SendSyncPhantomData<T: ?Sized>(PhantomData<T>);

// SAFETY: `SendSyncPhantomData` does not enable any behavior which isn't sound
// to be called from multiple threads.
unsafe impl<T: ?Sized> Send for SendSyncPhantomData<T> {}
// SAFETY: `SendSyncPhantomData` does not enable any behavior which isn't sound
// to be called from multiple threads.
unsafe impl<T: ?Sized> Sync for SendSyncPhantomData<T> {}

impl<T: ?Sized> Default for SendSyncPhantomData<T> {
fn default() -> SendSyncPhantomData<T> {
SendSyncPhantomData(PhantomData)
}
}

impl<T: ?Sized> PartialEq for SendSyncPhantomData<T> {
fn eq(&self, other: &Self) -> bool {
self.0.eq(&other.0)
}
}

impl<T: ?Sized> Eq for SendSyncPhantomData<T> {}

pub(crate) trait AsAddress {
fn addr(self) -> usize;
}
Expand Down

0 comments on commit 235cd25

Please sign in to comment.