Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion tools/cargo-zerocopy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn install_toolchain_or_exit(versions: &Versions, name: &str) -> Result<(), Erro
fn get_rustflags(name: &str) -> String {
// See #1792 for context on zerocopy_derive_union_into_bytes.
let mut flags =
"--cfg zerocopy_derive_union_into_bytes --cfg __ZEROCOPY_INTERNAL_USE_ONLY_DEV_MODE"
"--cfg zerocopy_unstable_derive_on_error --cfg zerocopy_derive_union_into_bytes --cfg __ZEROCOPY_INTERNAL_USE_ONLY_DEV_MODE"
.to_string();
flags += &format!(" --cfg __ZEROCOPY_INTERNAL_USE_ONLY_TOOLCHAIN=\"{name}\"");

Expand Down
5 changes: 4 additions & 1 deletion zerocopy-derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ exclude = [".*", "tests/enum_from_bytes.rs", "tests/ui-nightly/enum_from_bytes_u

[lints.rust]
# See #1792 for more context.
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(zerocopy_derive_union_into_bytes)'] }
unexpected_cfgs = { level = "warn", check-cfg = [
'cfg(zerocopy_derive_union_into_bytes)',
'cfg(zerocopy_unstable_derive_on_error)',
] }

[lib]
proc-macro = true
Expand Down
16 changes: 9 additions & 7 deletions zerocopy-derive/src/derive/from_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,20 @@ fn derive_from_zeros_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Erro
Repr::Compound(Spanned { t: CompoundRepr::C | CompoundRepr::Primitive(_), span: _ }, _) => {
}
Repr::Transparent(_) | Repr::Compound(Spanned { t: CompoundRepr::Rust, span: _ }, _) => {
return Err(Error::new(
Span::call_site(),
"must have #[repr(C)] or #[repr(Int)] attribute in order to guarantee this type's memory layout",
));
return ctx.error_or_skip(
Error::new(
Span::call_site(),
"must have #[repr(C)] or #[repr(Int)] attribute in order to guarantee this type's memory layout",
),
);
}
}

let zero_variant = match find_zero_variant(enm) {
Ok(index) => enm.variants.iter().nth(index).unwrap(),
// Has unknown variants
Err(true) => {
return Err(Error::new_spanned(
return ctx.error_or_skip(Error::new_spanned(
&ctx.ast,
"FromZeros only supported on enums with a variant that has a discriminant of `0`\n\
help: This enum has discriminants which are not literal integers. One of those may \
Expand All @@ -138,7 +140,7 @@ fn derive_from_zeros_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Erro
}
// Does not have unknown variants
Err(false) => {
return Err(Error::new_spanned(
return ctx.error_or_skip(Error::new_spanned(
&ctx.ast,
"FromZeros only supported on enums with a variant that has a discriminant of `0`",
));
Expand Down Expand Up @@ -170,7 +172,7 @@ fn derive_from_bytes_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Erro

let variants_required = 1usize << enum_size_from_repr(&repr)?;
if enm.variants.len() != variants_required {
return Err(Error::new_spanned(
return ctx.error_or_skip(Error::new_spanned(
&ctx.ast,
format!(
"FromBytes only supported on {} enum with {} variants",
Expand Down
12 changes: 8 additions & 4 deletions zerocopy-derive/src/derive/into_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn derive_into_bytes_struct(ctx: &Ctx, strct: &DataStruct) -> Result<TokenStream
// structs without requiring `Unaligned`.
(None, true)
} else {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout",
));
Expand All @@ -99,7 +99,7 @@ fn derive_into_bytes_struct(ctx: &Ctx, strct: &DataStruct) -> Result<TokenStream
fn derive_into_bytes_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Error> {
let repr = EnumRepr::from_attrs(&ctx.ast.attrs)?;
if !repr.is_c() && !repr.is_primitive() {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must have #[repr(C)] or #[repr(Int)] attribute in order to guarantee this type's memory layout",
));
Expand Down Expand Up @@ -127,6 +127,7 @@ fn derive_into_bytes_union(ctx: &Ctx, unn: &DataUnion) -> Result<TokenStream, Er
let error_message = "requires --cfg zerocopy_derive_union_into_bytes;
please let us know you use this feature: https://github.com/google/zerocopy/discussions/1802";
quote!(
#[allow(unused_attributes, unexpected_cfgs)]
const _: () = {
#[cfg(not(zerocopy_derive_union_into_bytes))]
#core::compile_error!(#error_message);
Expand All @@ -136,7 +137,10 @@ please let us know you use this feature: https://github.com/google/zerocopy/disc

// FIXME(#10): Support type parameters.
if !ctx.ast.generics.params.is_empty() {
return Err(Error::new(Span::call_site(), "unsupported on types with type parameters"));
return ctx.error_or_skip(Error::new(
Span::call_site(),
"unsupported on types with type parameters",
));
}

// Because we don't support generics, we don't need to worry about
Expand All @@ -145,7 +149,7 @@ please let us know you use this feature: https://github.com/google/zerocopy/disc
// no padding.
let repr = StructUnionRepr::from_attrs(&ctx.ast.attrs)?;
if !repr.is_c() && !repr.is_transparent() && !repr.is_packed_1() {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must be #[repr(C)], #[repr(packed)], or #[repr(transparent)]",
));
Expand Down
18 changes: 16 additions & 2 deletions zerocopy-derive/src/derive/try_from_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,11 @@ fn derive_try_from_bytes_enum(
// SAFETY: It would be sound for the enum to implement `FromBytes`, as
// required by `gen_trivial_is_bit_valid_unchecked`.
(None, true) => unsafe { gen_trivial_is_bit_valid_unchecked(ctx) },
(None, false) => derive_is_bit_valid(ctx, enm, &repr)?,
(None, false) => match derive_is_bit_valid(ctx, enm, &repr) {
Ok(extra) => extra,
Err(_) if ctx.skip_on_error => return Ok(TokenStream::new()),
Err(e) => return Err(e),
},
};

Ok(ImplBlockBuilder::new(ctx, enm, Trait::TryFromBytes, FieldBounds::ALL_SELF)
Expand All @@ -697,7 +701,13 @@ fn try_gen_trivial_is_bit_valid(ctx: &Ctx, top_level: Trait) -> Option<proc_macr
// make this no longer true. To hedge against these, we include an explicit
// `Self: FromBytes` check in the generated `is_bit_valid`, which is
// bulletproof.
if matches!(top_level, Trait::FromBytes) && ctx.ast.generics.params.is_empty() {
//
// If `ctx.skip_on_error` is true, we can't rely on the `FromBytes` derive
// to fail compilation if `Self` is not actually soundly `FromBytes`.
if matches!(top_level, Trait::FromBytes)
&& ctx.ast.generics.params.is_empty()
&& !ctx.skip_on_error
{
let zerocopy_crate = &ctx.zerocopy_crate;
let core = ctx.core_path();
Some(quote!(
Expand Down Expand Up @@ -730,6 +740,10 @@ fn try_gen_trivial_is_bit_valid(ctx: &Ctx, top_level: Trait) -> Option<proc_macr
None
}
}

/// # Safety
///
/// All initialized bit patterns must be valid for `Self`.
unsafe fn gen_trivial_is_bit_valid_unchecked(ctx: &Ctx) -> proc_macro2::TokenStream {
let zerocopy_crate = &ctx.zerocopy_crate;
let core = ctx.core_path();
Expand Down
6 changes: 3 additions & 3 deletions zerocopy-derive/src/derive/unaligned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn derive_unaligned_struct(ctx: &Ctx, strct: &DataStruct) -> Result<TokenStream,
} else if repr.is_c() || repr.is_transparent() {
FieldBounds::ALL_SELF
} else {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment",
));
Expand All @@ -45,7 +45,7 @@ fn derive_unaligned_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Error
repr.unaligned_validate_no_align_gt_1()?;

if !repr.is_u8() && !repr.is_i8() {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must have #[repr(u8)] or #[repr(i8)] attribute in order to guarantee this type's alignment",
));
Expand All @@ -68,7 +68,7 @@ fn derive_unaligned_union(ctx: &Ctx, unn: &DataUnion) -> Result<TokenStream, Err
} else if repr.is_c() || repr.is_transparent() {
FieldBounds::ALL_SELF
} else {
return Err(Error::new(
return ctx.error_or_skip(Error::new(
Span::call_site(),
"must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment",
));
Expand Down
77 changes: 72 additions & 5 deletions zerocopy-derive/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,32 @@
use proc_macro2::{Span, TokenStream};
use quote::{quote, ToTokens};
use syn::{
parse_quote, Data, DataEnum, DataStruct, DataUnion, DeriveInput, Error, Expr, ExprLit, Field,
GenericParam, Ident, Index, Lit, Meta, Path, Type, Variant, Visibility, WherePredicate,
parse_quote, spanned::Spanned as _, Data, DataEnum, DataStruct, DataUnion, DeriveInput, Error,
Expr, ExprLit, Field, GenericParam, Ident, Index, Lit, LitStr, Meta, Path, Type, Variant,
Visibility, WherePredicate,
};

use crate::repr::{CompoundRepr, EnumRepr, PrimitiveRepr, Repr, Spanned};

pub(crate) struct Ctx {
pub(crate) ast: DeriveInput,
pub(crate) zerocopy_crate: Path,

// The value of the last `#[zerocopy(on_error = ...)]` attribute, or `false`
// if none is provided.
pub(crate) skip_on_error: bool,

// The span of the last `#[zerocopy(on_error = ...)]` attribute, if any.
pub(crate) on_error_span: Option<proc_macro2::Span>,
}

impl Ctx {
/// Attempt to extract a crate path from the provided attributes. Defaults to
/// `::zerocopy` if not found.
pub(crate) fn try_from_derive_input(ast: DeriveInput) -> Result<Self, Error> {
let mut path = parse_quote!(::zerocopy);
let mut skip_on_error = false;
let mut on_error_span = None;

for attr in &ast.attrs {
if let Meta::List(ref meta_list) = attr.meta {
Expand All @@ -45,6 +55,21 @@ impl Ctx {
));
}

if meta.path.is_ident("on_error") {
on_error_span = Some(meta.path.span());
let value = meta.value()?;
let s: LitStr = value.parse()?;
match s.value().as_str() {
"skip" => skip_on_error = true,
"fail" => skip_on_error = false,
_ => return Err(Error::new(
s.span(),
"unrecognized value for `on_error` attribute from `zerocopy`; expected `skip` or `fail`",
)),
}
return Ok(());
}

Err(Error::new(
Span::call_site(),
format!(
Expand All @@ -57,17 +82,55 @@ impl Ctx {
}
}

Ok(Self { ast, zerocopy_crate: path })
Ok(Self { ast, zerocopy_crate: path, skip_on_error, on_error_span })
}

pub(crate) fn with_input(&self, input: &DeriveInput) -> Self {
Self { ast: input.clone(), zerocopy_crate: self.zerocopy_crate.clone() }
Self {
ast: input.clone(),
zerocopy_crate: self.zerocopy_crate.clone(),
skip_on_error: self.skip_on_error,
on_error_span: self.on_error_span,
}
}

pub(crate) fn core_path(&self) -> TokenStream {
let zerocopy_crate = &self.zerocopy_crate;
quote!(#zerocopy_crate::util::macro_util::core_reexport)
}

pub(crate) fn cfg_compile_error(&self) -> TokenStream {
// By checking both during the compilation of the proc macro *and* in
// the generated code, we ensure that `--cfg
// zerocopy_unstable_derive_on_error` need only be passed *either* when
// compiling this crate *or* when compiling the user's crate. The former
// is preferable, but in some situations (such as when cross-compiling
// using `cargo build --target`), it doesn't get propagated to this
// crate's build by default.
if cfg!(zerocopy_unstable_derive_on_error) {
quote!()
} else if let Some(span) = self.on_error_span {
let core = self.core_path();
let error_message = "`on_error` is experimental; pass '--cfg zerocopy_unstable_derive_on_error' to enable";
quote::quote_spanned! {span=>
#[allow(unused_attributes, unexpected_cfgs)]
const _: () = {
#[cfg(not(zerocopy_unstable_derive_on_error))]
#core::compile_error!(#error_message);
};
}
} else {
quote!()
}
}

pub(crate) fn error_or_skip<E>(&self, error: E) -> Result<TokenStream, E> {
if self.skip_on_error {
Ok(self.cfg_compile_error())
} else {
Err(error)
}
}
}

pub(crate) trait DataExt {
Expand Down Expand Up @@ -586,7 +649,10 @@ impl<'a> ImplBlockBuilder<'a> {
});

let inner_extras = self.inner_extras;
let allow_trivial_bounds =
if self.ctx.skip_on_error { quote!(#[allow(trivial_bounds)]) } else { quote!() };
let impl_tokens = quote! {
#allow_trivial_bounds
unsafe impl < #(#params),* > #trait_path for #type_ident < #(#param_idents),* >
where
#(#bounds,)*
Expand All @@ -598,7 +664,8 @@ impl<'a> ImplBlockBuilder<'a> {
};

let outer_extras = self.outer_extras.filter(|e| !e.is_empty());
const_block([Some(impl_tokens), outer_extras])
let cfg_compile_error = self.ctx.cfg_compile_error();
const_block([Some(cfg_compile_error), Some(impl_tokens), outer_extras])
}
}

Expand Down
Loading
Loading