Skip to content

Commit

Permalink
[derive] Improve IntoBytes error messages (#1712)
Browse files Browse the repository at this point in the history
On Rust toolchains 1.78.0 or later, use
`#[diagnostic::on_unimplemented]` to improve the error message emitted
when `IntoBytes` is derived on a type with inter-field padding.
  • Loading branch information
joshlf authored Sep 21, 2024
1 parent c57e3b9 commit 63b5e78
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 91 deletions.
19 changes: 10 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3852,25 +3852,26 @@ fn mut_from_prefix_suffix<T: FromBytes + KnownLayout + ?Sized>(
///
/// # Error Messages
///
/// Due to the way that the custom derive for `IntoBytes` is implemented, you
/// may get an error like this:
/// On Rust toolchains prior to 1.78.0, due to the way that the custom derive
/// for `IntoBytes` is implemented, you may get an error like this:
///
/// ```text
/// error[E0277]: the trait bound `HasPadding<Foo, true>: ShouldBe<false>` is not satisfied
/// error[E0277]: the trait bound `(): PaddingFree<Foo, true>` is not satisfied
/// --> lib.rs:23:10
/// |
/// 1 | #[derive(IntoBytes)]
/// | ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<Foo, true>`
/// | ^^^^^^^^^ the trait `PaddingFree<Foo, true>` is not implemented for `()`
/// |
/// = help: the trait `ShouldBe<VALUE>` is implemented for `HasPadding<T, VALUE>`
/// = help: the following implementations were found:
/// <() as PaddingFree<T, false>>
/// ```
///
/// This error indicates that the type being annotated has padding bytes, which
/// is illegal for `IntoBytes` types. Consider reducing the alignment of some
/// fields by using types in the [`byteorder`] module, adding explicit struct
/// fields where those padding bytes would be, or using `#[repr(packed)]`. See
/// the Rust Reference's page on [type layout] for more information about type
/// layout and padding.
/// fields by using types in the [`byteorder`] module, wrapping field types in
/// [`Unalign`], adding explicit struct fields where those padding bytes would
/// be, or using `#[repr(packed)]`. See the Rust Reference's page on [type
/// layout] for more information about type layout and padding.
///
/// [type layout]: https://doc.rust-lang.org/reference/type-layout.html
///
Expand Down
24 changes: 13 additions & 11 deletions src/util/macro_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@

#![allow(missing_debug_implementations)]

use core::{
marker::PhantomData,
mem::{self, ManuallyDrop},
};
use core::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 All @@ -35,13 +32,18 @@ use crate::{
Immutable, IntoBytes, Ptr, TryFromBytes, Unalign, ValidityError,
};

/// A compile-time check that should be one particular value.
pub trait ShouldBe<const VALUE: bool> {}

/// A struct for checking whether `T` contains padding.
pub struct HasPadding<T: ?Sized, const VALUE: bool>(PhantomData<T>);

impl<T: ?Sized, const VALUE: bool> ShouldBe<VALUE> for HasPadding<T, VALUE> {}
#[cfg_attr(
zerocopy_diagnostic_on_unimplemented,
diagnostic::on_unimplemented(
message = "`{T}` has inter-field padding",
label = "types with padding cannot implement `IntoBytes`",
note = "consider using `zerocopy::Unalign` to lower the alignment of individual fields",
note = "consider adding explicit fields where padding would be",
note = "consider using `#[repr(packed)]` to remove inter-field padding"
)
)]
pub trait PaddingFree<T: ?Sized, const HAS_PADDING: bool> {}
impl<T: ?Sized> PaddingFree<T, false> for () {}

/// A type whose size is equal to `align_of::<T>()`.
#[repr(C)]
Expand Down
2 changes: 1 addition & 1 deletion zerocopy-derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ proc-macro = true
[dependencies]
proc-macro2 = "1.0.1"
quote = "1.0.10"
syn = "2.0.46"
syn = { version = "2.0.46", features = ["full"] }

[dev-dependencies]
dissimilar = "1.0.9"
Expand Down
19 changes: 3 additions & 16 deletions zerocopy-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ mod repr;

use proc_macro2::{TokenStream, TokenTree};
use quote::ToTokens;
use syn::{punctuated::Punctuated, token::Colon, PredicateType};

use {
proc_macro2::Span,
Expand Down Expand Up @@ -1222,27 +1221,15 @@ fn impl_block<D: DataExt>(
let validator_context = check.validator_macro_context();
let validator_macro = check.validator_macro_ident();
let t = tag.iter();
// We have to manually construct a bound here because the validator
// context may include type definitions and syn cannot parse type
// definitions in const exprs without the `full` feature enabled.
// Constructing these bounds "verbatim" gets around this issue.
let bounded_ty = Type::Verbatim(quote! {
::zerocopy::util::macro_util::HasPadding<
parse_quote! {
(): ::zerocopy::util::macro_util::PaddingFree<
#type_ident,
{
#validator_context
::zerocopy::#validator_macro!(#type_ident, #(#t,)* #(#variant_types),*)
}
>
});
let mut bounds = Punctuated::new();
bounds.push(parse_quote! { ::zerocopy::util::macro_util::ShouldBe<false> });
WherePredicate::Type(PredicateType {
lifetimes: None,
bounded_ty,
colon_token: Colon::default(),
bounds,
})
}
});

let self_bounds: Option<WherePredicate> = match self_type_trait_bounds {
Expand Down
23 changes: 23 additions & 0 deletions zerocopy-derive/src/output_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,29 @@ fn test_into_bytes() {
}
} no_build
}

test! {
IntoBytes {
#[repr(C)]
struct Foo {
a: u8,
b: u8,
}
} expands to {
#[allow(deprecated)]
unsafe impl ::zerocopy::IntoBytes for Foo
where
u8: ::zerocopy::IntoBytes,
u8: ::zerocopy::IntoBytes,
(): ::zerocopy::util::macro_util::PaddingFree<
Foo,
{ ::zerocopy::struct_has_padding!(Foo, [u8, u8]) },
>,
{
fn only_derive_is_allowed_to_implement_this_trait() {}
}
} no_build
}
}

#[test]
Expand Down
18 changes: 9 additions & 9 deletions zerocopy-derive/tests/ui-msrv/enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -343,35 +343,35 @@ error[E0277]: the trait bound `bool: FromBytes` is not satisfied
= help: see issue #48214
= note: this error originates in the derive macro `FromBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes1, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes1, true>` is not satisfied
--> tests/ui-msrv/enum.rs:533:10
|
533 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes1, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes1, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes2, true>` is not satisfied
--> tests/ui-msrv/enum.rs:544:10
|
544 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes3, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes3, true>` is not satisfied
--> tests/ui-msrv/enum.rs:550:10
|
550 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes3, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes3, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
12 changes: 6 additions & 6 deletions zerocopy-derive/tests/ui-msrv/struct.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -110,25 +110,25 @@ error[E0277]: the trait bound `AU16: Unaligned` is not satisfied
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes2, true>` is not satisfied
--> tests/ui-msrv/struct.rs:107:10
|
107 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes3, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes3, true>` is not satisfied
--> tests/ui-msrv/struct.rs:114:10
|
114 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes3, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes3, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down
6 changes: 3 additions & 3 deletions zerocopy-derive/tests/ui-msrv/union.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ error[E0277]: the trait bound `UnsafeCell<()>: zerocopy::Immutable` is not satis
= help: see issue #48214
= note: this error originates in the derive macro `Immutable` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: the trait bound `(): PaddingFree<IntoBytes2, true>` is not satisfied
--> tests/ui-msrv/union.rs:39:10
|
39 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
|
= help: the following implementations were found:
<HasPadding<T, VALUE> as ShouldBe<VALUE>>
<() as PaddingFree<T, false>>
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
30 changes: 21 additions & 9 deletions zerocopy-derive/tests/ui-nightly/enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -413,41 +413,53 @@ help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes1, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes1` has inter-field padding
--> tests/ui-nightly/enum.rs:533:10
|
533 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes1, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes1, true>`
= help: the trait `PaddingFree<IntoBytes1, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes1, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
|
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes2` has inter-field padding
--> tests/ui-nightly/enum.rs:544:10
|
544 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes2, true>`
= help: the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes2, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
|
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes3, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes3` has inter-field padding
--> tests/ui-nightly/enum.rs:550:10
|
550 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes3, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes3, true>`
= help: the trait `PaddingFree<IntoBytes3, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes3, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
Expand Down
20 changes: 14 additions & 6 deletions zerocopy-derive/tests/ui-nightly/struct.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -241,27 +241,35 @@ help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes2` has inter-field padding
--> tests/ui-nightly/struct.rs:107:10
|
107 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes2, true>`
= help: the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes2, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
|
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes3, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes3` has inter-field padding
--> tests/ui-nightly/struct.rs:114:10
|
114 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes3, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes3, true>`
= help: the trait `PaddingFree<IntoBytes3, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes3, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
Expand Down
10 changes: 7 additions & 3 deletions zerocopy-derive/tests/ui-nightly/union.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,17 @@ help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
9 + #![feature(trivial_bounds)]
|

error[E0277]: the trait bound `HasPadding<IntoBytes2, true>: ShouldBe<false>` is not satisfied
error[E0277]: `IntoBytes2` has inter-field padding
--> tests/ui-nightly/union.rs:39:10
|
39 | #[derive(IntoBytes)]
| ^^^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<IntoBytes2, true>`
| ^^^^^^^^^ types with padding cannot implement `IntoBytes`
|
= help: the trait `ShouldBe<true>` is implemented for `HasPadding<IntoBytes2, true>`
= help: the trait `PaddingFree<IntoBytes2, true>` is not implemented for `()`
= note: consider using `zerocopy::Unalign` to lower the alignment of individual fields
= note: consider adding explicit fields where padding would be
= note: consider using `#[repr(packed)]` to remove inter-field padding
= help: the trait `PaddingFree<IntoBytes2, false>` is implemented for `()`
= help: see issue #48214
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
Expand Down
Loading

0 comments on commit 63b5e78

Please sign in to comment.