Skip to content

Commit

Permalink
Improve UX of postmonomorphization errors (#1268)
Browse files Browse the repository at this point in the history 
By asserting in a macro, rather than a helper function, rustc's
diagnostics point to the user's callsite, rather than our own.
  • Loading branch information
@jswrenn
jswrenn authored May 16, 2024
1 parent c8ea4b3 commit 264619b
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 75 deletions.
24 changes: 12 additions & 12 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
match Ptr::from_ref(candidate).try_cast_into_no_leftover::<Self, BecauseImmutable>(None) {
Ok(candidate) => {
// This call may panic. If that happens, it doesn't cause any soundness
Expand Down Expand Up @@ -1274,7 +1274,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
try_ref_from_prefix_suffix(candidate, CastType::Prefix, None)
}

Expand Down Expand Up @@ -1354,7 +1354,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
try_ref_from_prefix_suffix(candidate, CastType::Suffix, None).map(swap)
}

Expand Down Expand Up @@ -1431,7 +1431,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
match Ptr::from_mut(bytes).try_cast_into_no_leftover::<Self, BecauseExclusive>(None) {
Ok(candidate) => {
// This call may panic. If that happens, it doesn't cause any soundness
Expand Down Expand Up @@ -1536,7 +1536,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
try_mut_from_prefix_suffix(candidate, CastType::Prefix, None)
}

Expand Down Expand Up @@ -1624,7 +1624,7 @@ pub unsafe trait TryFromBytes {
where
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
try_mut_from_prefix_suffix(candidate, CastType::Suffix, None).map(swap)
}

Expand Down Expand Up @@ -2339,7 +2339,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
match Ptr::from_ref(source).try_cast_into_no_leftover::<_, BecauseImmutable>(None) {
Ok(ptr) => Ok(ptr.bikeshed_recall_valid().as_ref()),
Err(err) => Err(err.map_src(|src| src.as_ref())),
Expand Down Expand Up @@ -2417,7 +2417,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
ref_from_prefix_suffix(source, None, CastType::Prefix)
}

Expand Down Expand Up @@ -2478,7 +2478,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: Immutable + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
ref_from_prefix_suffix(source, None, CastType::Suffix).map(swap)
}

Expand Down Expand Up @@ -2547,7 +2547,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
match Ptr::from_mut(source).try_cast_into_no_leftover::<_, BecauseExclusive>(None) {
Ok(ptr) => Ok(ptr.bikeshed_recall_valid().as_mut()),
Err(err) => Err(err.map_src(|src| src.as_mut())),
Expand Down Expand Up @@ -2626,7 +2626,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
mut_from_prefix_suffix(source, None, CastType::Prefix)
}

Expand Down Expand Up @@ -2693,7 +2693,7 @@ pub unsafe trait FromBytes: FromZeros {
where
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
static_assert_dst_is_not_zst!(Self);
mut_from_prefix_suffix(source, None, CastType::Suffix).map(swap)
}

Expand Down
13 changes: 7 additions & 6 deletions src/macro_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

#![allow(missing_debug_implementations)]

use core::{marker::PhantomData, mem::ManuallyDrop};
use core::{
marker::PhantomData,
mem::{self, ManuallyDrop},
};

// TODO(#29), TODO(https://github.com/rust-lang/rust/issues/69835): Remove this
// `cfg` when `size_of_val_raw` is stabilized.
Expand Down Expand Up @@ -384,7 +387,7 @@ pub const unsafe fn transmute_ref<'dst, 'src: 'dst, Src: 'src, Dst: 'dst>(
// TODO(#67): Once our MSRV is 1.58, replace this `transmute` with `&*dst`.
#[allow(clippy::transmute_ptr_to_ref)]
unsafe {
core::mem::transmute(dst)
mem::transmute(dst)
}
}

Expand Down Expand Up @@ -442,7 +445,7 @@ where
I::Aliasing: AtLeast<invariant::Shared>,
R: AliasingSafeReason,
{
crate::util::assert_dst_not_bigger_than_src::<Src, Dst>();
static_assert!(Src, Dst => mem::size_of::<Dst>() >= mem::size_of::<Src>());

// SAFETY: This is a pointer cast, satisfying the following properties:
// - `p as *mut Dst` addresses a subset of the `bytes` addressed by `src`,
Expand Down Expand Up @@ -494,7 +497,7 @@ where
// inner value [1].
//
// [1]: https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html
Ok(unsafe { core::mem::transmute_copy(&*src) })
Ok(unsafe { mem::transmute_copy(&*src) })
}

/// A function which emits a warning if its return value is not used.
Expand All @@ -519,8 +522,6 @@ pub mod core_reexport {

#[cfg(test)]
mod tests {
use core::mem;

use super::*;
use crate::util::testutil::*;

Expand Down
55 changes: 54 additions & 1 deletion src/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,8 @@ macro_rules! const_panic {

/// Either assert (if the current Rust toolchain supports panicking in `const
/// fn`) or evaluate the expression and, if it evaluates to `false`, call
/// `const_panic!`.
/// `const_panic!`. This is used in place of `assert!` in const contexts to
/// accommodate old toolchains.
macro_rules! const_assert {
($e:expr) => {{
#[cfg(zerocopy_panic_in_const)]
Expand Down Expand Up @@ -720,3 +721,55 @@ macro_rules! const_unreachable {
loop {}
}};
}

/// Asserts at compile time that `$condition` is true for `Self` or the given
/// `$tyvar`s. Unlike `const_assert`, this is *strictly* a compile-time check;
/// it cannot be evaluated in a runtime context. The condition is checked after
/// monomorphization and, upon failure, emits a compile error.
macro_rules! static_assert {
(Self $(: $(? $optbound:ident $(+)?)* $($bound:ident $(+)?)* )? => $condition:expr) => {{
trait StaticAssert {
const ASSERT: bool;
}

impl<T $(: $(? $optbound +)* $($bound +)*)?> StaticAssert for T {
const ASSERT: bool = {
const_assert!($condition);
$condition
};
}

const_assert!(<Self as StaticAssert>::ASSERT);
}};
($($tyvar:ident $(: $(? $optbound:ident $(+)?)* $($bound:ident $(+)?)* )?),* => $condition:expr) => {{
trait StaticAssert {
const ASSERT: bool;
}

impl<$($tyvar $(: $(? $optbound +)* $($bound +)*)?,)*> StaticAssert for ($($tyvar,)*) {
const ASSERT: bool = {
const_assert!($condition);
$condition
};
}

const_assert!(<($($tyvar,)*) as StaticAssert>::ASSERT);
}};
}

/// Assert at compile time that `tyvar` does not have a zero-sized DST
/// component.
macro_rules! static_assert_dst_is_not_zst {
($tyvar:ident) => {{
use crate::KnownLayout;
static_assert!($tyvar: ?Sized + KnownLayout => {
let dst_is_zst = match $tyvar::LAYOUT.size_info {
crate::SizeInfo::Sized { .. } => false,
crate::SizeInfo::SliceDst(TrailingSliceLayout { elem_size, .. }) => {
elem_size == 0
}
};
!dst_is_zst
});
}}
}
32 changes: 16 additions & 16 deletions src/ref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ where
#[must_use = "has no side effects"]
#[inline]
pub fn from_bytes(source: B) -> Result<Ref<B, T>, CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
if let Err(e) =
Ptr::from_ref(source.deref()).try_cast_into_no_leftover::<T, BecauseImmutable>(None)
{
Expand Down Expand Up @@ -346,7 +346,7 @@ where
#[must_use = "has no side effects"]
#[inline]
pub fn from_prefix(source: B) -> Result<(Ref<B, T>, B), CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
let remainder = match Ptr::from_ref(source.deref())
.try_cast_into::<T, BecauseImmutable>(CastType::Prefix, None)
{
Expand Down Expand Up @@ -404,7 +404,7 @@ where
#[must_use = "has no side effects"]
#[inline]
pub fn from_suffix(source: B) -> Result<(B, Ref<B, T>), CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
let remainder = match Ptr::from_ref(source.deref())
.try_cast_into::<T, BecauseImmutable>(CastType::Suffix, None)
{
Expand Down Expand Up @@ -443,7 +443,7 @@ where
/// aligned, this returns `Err`.
#[inline]
pub fn from_bytes_with_elems(source: B, count: usize) -> Result<Ref<B, T>, CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
let expected_len = match count.size_for_metadata(T::LAYOUT) {
Some(len) => len,
None => return Err(SizeError::new(source).into()),
Expand Down Expand Up @@ -472,7 +472,7 @@ where
source: B,
count: usize,
) -> Result<(Ref<B, T>, B), CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
let expected_len = match count.size_for_metadata(T::LAYOUT) {
Some(len) => len,
None => return Err(SizeError::new(source).into()),
Expand All @@ -496,7 +496,7 @@ where
source: B,
count: usize,
) -> Result<(B, Ref<B, T>), CastError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
let expected_len = match count.size_for_metadata(T::LAYOUT) {
Some(len) => len,
None => return Err(SizeError::new(source).into()),
Expand Down Expand Up @@ -544,7 +544,7 @@ where
#[must_use = "has no side effects"]
#[inline(always)]
pub fn unaligned_from(source: B) -> Result<Ref<B, T>, SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
match Ref::from_bytes(source) {
Ok(dst) => Ok(dst),
Err(CastError::Size(e)) => Err(e),
Expand Down Expand Up @@ -589,7 +589,7 @@ where
#[must_use = "has no side effects"]
#[inline(always)]
pub fn unaligned_from_prefix(source: B) -> Result<(Ref<B, T>, B), SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
Ref::from_prefix(source).map_err(|e| match e {
CastError::Size(e) => e,
CastError::Alignment(_) => unreachable!(),
Expand Down Expand Up @@ -627,7 +627,7 @@ where
#[must_use = "has no side effects"]
#[inline(always)]
pub fn unaligned_from_suffix(source: B) -> Result<(B, Ref<B, T>), SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
Ref::from_suffix(source).map_err(|e| match e {
CastError::Size(e) => e,
CastError::Alignment(_) => unreachable!(),
Expand All @@ -653,7 +653,7 @@ where
source: B,
count: usize,
) -> Result<Ref<B, T>, SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
Self::from_bytes_with_elems(source, count).map_err(|e| match e {
CastError::Size(e) => e,
CastError::Alignment(_) => unreachable!(),
Expand All @@ -679,7 +679,7 @@ where
source: B,
count: usize,
) -> Result<(Ref<B, T>, B), SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
Self::from_prefix_with_elems(source, count).map_err(|e| match e {
CastError::Size(e) => e,
CastError::Alignment(_) => unreachable!(),
Expand All @@ -699,7 +699,7 @@ where
source: B,
count: usize,
) -> Result<(B, Ref<B, T>), SizeError<B, T>> {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);
Self::from_suffix_with_elems(source, count).map_err(|e| match e {
CastError::Size(e) => e,
CastError::Alignment(_) => unreachable!(),
Expand All @@ -720,7 +720,7 @@ where
#[inline(always)]
pub fn into_ref(self) -> &'a T {
// Presumably unreachable, since we've guarded each constructor of `Ref`.
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);

// SAFETY: We don't call any methods on `b` other than those provided by
// `IntoByteSlice`.
Expand Down Expand Up @@ -750,7 +750,7 @@ where
#[inline(always)]
pub fn into_mut(self) -> &'a mut T {
// Presumably unreachable, since we've guarded each constructor of `Ref`.
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);

// SAFETY: We don't call any methods on `b` other than those provided by
// `IntoByteSliceMut`.
Expand Down Expand Up @@ -846,7 +846,7 @@ where
type Target = T;
#[inline]
fn deref(&self) -> &T {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);

// SAFETY: We don't call any methods on `b` other than those provided by
// `ByteSlice`.
Expand All @@ -873,7 +873,7 @@ where
{
#[inline]
fn deref_mut(&mut self) -> &mut T {
util::assert_dst_is_not_zst::<T>();
static_assert_dst_is_not_zst!(T);

// SAFETY: We don't call any methods on `b` other than those provided by
// `ByteSliceMut`.
Expand Down
Loading

0 comments on commit 264619b

Please sign in to comment.