From 3e44e058ec26842bc3d6b8af76eda7f90e3fb4dc Mon Sep 17 00:00:00 2001 From: Cai Bear Date: Fri, 26 Jun 2026 19:59:28 -0700 Subject: [PATCH] Prevent duplicate attributes. --- bitcode_derive/src/attribute.rs | 157 ++++++++++++++++++++------------ bitcode_derive/src/bound.rs | 6 +- bitcode_derive/src/decode.rs | 30 +++--- bitcode_derive/src/encode.rs | 24 +++-- bitcode_derive/src/lib.rs | 5 - bitcode_derive/src/shared.rs | 74 ++++++++------- src/derive/mod.rs | 22 +++++ 7 files changed, 190 insertions(+), 128 deletions(-) diff --git a/bitcode_derive/src/attribute.rs b/bitcode_derive/src/attribute.rs index a6a63bf..fb10acd 100644 --- a/bitcode_derive/src/attribute.rs +++ b/bitcode_derive/src/attribute.rs @@ -1,5 +1,6 @@ use crate::{err, error}; use proc_macro2::TokenStream; +use quote::quote; use std::str::FromStr; use syn::punctuated::Punctuated; use syn::spanned::Spanned; @@ -7,7 +8,7 @@ use syn::{parse2, Attribute, Expr, ExprLit, Lit, Meta, Path, Result, Token, Type enum BitcodeAttr { BoundType(Type), - CrateAlias(Path), + CrateName(Path), Skip, } @@ -49,7 +50,7 @@ impl BitcodeAttr { // removed: https://github.com/SoftbearStudios/bitcode/pull/28#issuecomment-2227465515 // path.leading_colon = Some(Token![::](str_lit.span())); - Ok(Self::CrateAlias(path)) + Ok(Self::CrateName(path)) } _ => err(&nested, "expected name value"), }, @@ -58,89 +59,133 @@ impl BitcodeAttr { } } - fn apply(self, attrs: &mut BitcodeAttrs, nested: &Meta) -> Result<()> { + fn apply(self, attrs: &mut BitcodeAnyAttrs<'_, '_>, nested: &Meta) -> Result<()> { + fn set_if_not_duplicate( + v: &mut T, + new_value: T, + nested: &Meta, + ) -> Result<()> { + if new_value == Default::default() { + return err(nested, "cannot set to default value"); + } + if &*v != &Default::default() { + return err(nested, "duplicate"); + } + *v = new_value; + Ok(()) + } + match self { Self::BoundType(bound_type) => { - if let AttrType::Field { bound_type: b, .. } = &mut attrs.attr_type { - if b.is_some() { - return err(nested, "duplicate"); - } - *b = Some(bound_type); - Ok(()) + if let BitcodeAnyAttrs::Field(field) = attrs { + set_if_not_duplicate(&mut field.bound_type, Some(bound_type), nested) } else { - err(nested, "can only apply bound to fields") + err(nested, r#"can only apply to fields"#) } } - Self::CrateAlias(crate_name) => { - if let AttrType::Derive = attrs.attr_type { - attrs.crate_name = crate_name; - Ok(()) + Self::CrateName(crate_name) => { + if let BitcodeAnyAttrs::Derive(derive) = attrs { + set_if_not_duplicate(&mut derive.crate_name, Some(crate_name), nested) } else { - err(nested, "can only apply crate rename to derives") + err(nested, r#"can only apply to struct/enum definition"#) } } Self::Skip => { - if let AttrType::Field { .. } = &attrs.attr_type { - attrs.skip = true; - Ok(()) + if let BitcodeAnyAttrs::Field(field) = attrs { + set_if_not_duplicate(&mut field.skip, true, nested) } else { - err(nested, "can only apply skip to fields") + err(nested, "can only apply to fields") } } } } } -#[derive(Clone)] -pub struct BitcodeAttrs { - attr_type: AttrType, - /// The crate name to use for the generated code, defaults to "bitcode". - pub crate_name: Path, - /// Whether to skip this field during (de)serialisation. - pub skip: bool, -} - -#[derive(Clone)] -enum AttrType { - Derive, - Variant, - Field { bound_type: Option }, +pub struct BitcodeDeriveAttrs { + crate_name: Option, + pub private: TokenStream, } - -impl BitcodeAttrs { - fn new(attr_type: AttrType) -> Self { - Self { - attr_type, - crate_name: syn::parse_str("bitcode").expect("invalid crate name"), - skip: false, - } +impl BitcodeDeriveAttrs { + pub fn parse(attrs: &[Attribute]) -> Result { + let mut ret = Self { + crate_name: Default::default(), + private: quote! {}, + }; + BitcodeAnyAttrs::Derive(&mut ret).parse_inner(attrs)?; + let crate_name = ret + .crate_name + .clone() + .unwrap_or_else(|| syn::parse_str("bitcode").unwrap()); + + ret.private = quote! { #crate_name::__private }; + Ok(ret) } +} - pub fn bound_type(&self) -> Option { - match &self.attr_type { - AttrType::Field { bound_type, .. } => bound_type.as_ref().cloned(), - _ => unreachable!(), - } +pub struct BitcodeVariantAttrs<'a> { + parent: &'a BitcodeDeriveAttrs, +} +impl std::ops::Deref for BitcodeVariantAttrs<'_> { + type Target = BitcodeDeriveAttrs; + fn deref(&self) -> &Self::Target { + self.parent } +} - pub fn parse_derive(attrs: &[Attribute]) -> Result { - let mut ret = Self::new(AttrType::Derive); - ret.parse_inner(attrs)?; +impl<'a> BitcodeVariantAttrs<'a> { + pub fn parse(attrs: &[Attribute], parent: &'a BitcodeDeriveAttrs) -> Result { + let mut ret = Self { parent }; + BitcodeAnyAttrs::Variant { _unused: &mut ret }.parse_inner(attrs)?; Ok(ret) } +} - pub fn parse_variant(attrs: &[Attribute], _derive_attrs: &Self) -> Result { - let mut ret = Self::new(AttrType::Variant); - ret.parse_inner(attrs)?; - Ok(ret) +#[derive(Copy, Clone)] +pub enum BitcodeDeriveOrVariantAttrs<'a> { + Derive(&'a BitcodeDeriveAttrs), + Variant(&'a BitcodeVariantAttrs<'a>), +} +impl std::ops::Deref for BitcodeDeriveOrVariantAttrs<'_> { + type Target = BitcodeDeriveAttrs; + fn deref(&self) -> &Self::Target { + match self { + Self::Derive(v) => v, + Self::Variant(v) => v.parent, + } } +} - pub fn parse_field(attrs: &[Attribute], _parent_attrs: &Self) -> Result { - let mut ret = Self::new(AttrType::Field { bound_type: None }); - ret.parse_inner(attrs)?; +pub struct BitcodeFieldAttrs<'a> { + parent: BitcodeDeriveOrVariantAttrs<'a>, + pub bound_type: Option, + pub skip: bool, +} +impl<'a> std::ops::Deref for BitcodeFieldAttrs<'a> { + type Target = BitcodeDeriveOrVariantAttrs<'a>; + fn deref(&self) -> &Self::Target { + &self.parent + } +} +impl<'a> BitcodeFieldAttrs<'a> { + pub fn parse(attrs: &[Attribute], parent: BitcodeDeriveOrVariantAttrs<'a>) -> Result { + let mut ret = Self { + parent, + bound_type: Default::default(), + skip: Default::default(), + }; + BitcodeAnyAttrs::Field(&mut ret).parse_inner(attrs)?; Ok(ret) } +} +enum BitcodeAnyAttrs<'a, 'b> { + Derive(&'a mut BitcodeDeriveAttrs), + Variant { + _unused: &'a mut BitcodeVariantAttrs<'b>, + }, + Field(&'a mut BitcodeFieldAttrs<'b>), +} +impl BitcodeAnyAttrs<'_, '_> { fn parse_inner(&mut self, attrs: &[Attribute]) -> Result<()> { for attr in attrs { let path = path_ident_string(attr.path(), attr)?; diff --git a/bitcode_derive/src/bound.rs b/bitcode_derive/src/bound.rs index f3edbf4..0908345 100644 --- a/bitcode_derive/src/bound.rs +++ b/bitcode_derive/src/bound.rs @@ -1,4 +1,4 @@ -use crate::attribute::BitcodeAttrs; +use crate::attribute::BitcodeFieldAttrs; use std::collections::{HashMap, HashSet}; use syn::punctuated::Pair; use syn::Token; @@ -12,11 +12,11 @@ impl FieldBounds { pub fn add_bound_type( &mut self, field: syn::Field, - field_attrs: &BitcodeAttrs, + field_attrs: &BitcodeFieldAttrs, bound: syn::Path, ) { let bounds = self.bounds.entry(bound).or_default(); - if let Some(bound_type) = field_attrs.bound_type() { + if let Some(bound_type) = field_attrs.bound_type.clone() { bounds.1.push(bound_type); } else { bounds.0.push(field); diff --git a/bitcode_derive/src/decode.rs b/bitcode_derive/src/decode.rs index 0421537..fedf2c8 100644 --- a/bitcode_derive/src/decode.rs +++ b/bitcode_derive/src/decode.rs @@ -1,5 +1,4 @@ -use crate::attribute::BitcodeAttrs; -use crate::private; +use crate::attribute::{BitcodeDeriveAttrs, BitcodeFieldAttrs}; use crate::shared::{remove_lifetimes, replace_lifetimes, VariantIndexType}; use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote, ToTokens}; @@ -36,20 +35,19 @@ impl Item { impl crate::shared::Item for Item { fn field_impl( self, - crate_name: &Path, + attrs: &BitcodeFieldAttrs, field_name: TokenStream, global_field_name: TokenStream, real_field_name: TokenStream, field_type: &Type, - field_attrs: &BitcodeAttrs, ) -> TokenStream { match self { Self::Type => { let mut de_type = replace_lifetimes(field_type, DE_LIFETIME).to_token_stream(); - if field_attrs.skip { + if attrs.skip { de_type = quote! { ::core::marker::PhantomData<#de_type> }; } - let private = private(crate_name); + let private = &attrs.private; let de = de_lifetime(); quote! { #global_field_name: <#de_type as #private::Decode<#de>>::Decoder, @@ -63,7 +61,7 @@ impl crate::shared::Item for Item { }, // Only used by enum variants. Self::Decode => { - let value = if field_attrs.skip { + let value = if attrs.skip { quote! { Default::default() } @@ -78,11 +76,11 @@ impl crate::shared::Item for Item { } Self::DecodeInPlace => { let de_type = replace_lifetimes(field_type, DE_LIFETIME); - let private = private(crate_name); + let private = &attrs.private; let target = quote! { #private::uninit_field!(out.#real_field_name: #de_type) }; - if field_attrs.skip { + if attrs.skip { quote! {{ (#target).write(Default::default()); }} @@ -109,7 +107,7 @@ impl crate::shared::Item for Item { fn enum_impl( self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, variant_count: usize, variant_index_type: VariantIndexType, pattern: impl Fn(usize) -> TokenStream, @@ -125,7 +123,7 @@ impl crate::shared::Item for Item { let inners: TokenStream = (0..variant_count).map(|i| inner(self, i)).collect(); let variants = decode_variants .then(|| { - let private = private(crate_name); + let private = &attrs.private; let c_style = inners.is_empty(); let histogram = if c_style { 0 @@ -152,7 +150,7 @@ impl crate::shared::Item for Item { } Self::Populate => { if never { - let private = private(crate_name); + let private = &attrs.private; return quote! { if __length != 0 { return #private::invalid_enum_variant(); @@ -253,8 +251,8 @@ impl crate::shared::Derive<{ Item::COUNT }> for Decode { type Item = Item; const ALL: [Self::Item; Item::COUNT] = Item::ALL; - fn bound(&self, crate_name: &Path) -> Path { - let private = private(crate_name); + fn bound(&self, attrs: &BitcodeDeriveAttrs) -> Path { + let private = &attrs.private; let de = de_lifetime(); parse_quote!(#private::Decode<#de>) } @@ -265,7 +263,7 @@ impl crate::shared::Derive<{ Item::COUNT }> for Decode { fn derive_impl( &self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, output: [TokenStream; Item::COUNT], ident: Ident, mut generics: Generics, @@ -321,7 +319,7 @@ impl crate::shared::Derive<{ Item::COUNT }> for Decode { let decoder_ident = Ident::new(&format!("{ident}Decoder"), Span::call_site()); let decoder_ty = quote! { #decoder_ident #decoder_generics }; - let private = private(crate_name); + let private = &attrs.private; quote! { #[allow(clippy::pedantic)] diff --git a/bitcode_derive/src/encode.rs b/bitcode_derive/src/encode.rs index d134a37..26e0c58 100644 --- a/bitcode_derive/src/encode.rs +++ b/bitcode_derive/src/encode.rs @@ -1,5 +1,4 @@ -use crate::attribute::BitcodeAttrs; -use crate::private; +use crate::attribute::{BitcodeDeriveAttrs, BitcodeFieldAttrs}; use crate::shared::{remove_lifetimes, replace_lifetimes, VariantIndexType}; use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote, ToTokens}; @@ -28,20 +27,19 @@ impl Item { impl crate::shared::Item for Item { fn field_impl( self, - crate_name: &Path, + attrs: &BitcodeFieldAttrs, field_name: TokenStream, global_field_name: TokenStream, real_field_name: TokenStream, field_type: &Type, - field_attrs: &BitcodeAttrs, ) -> TokenStream { match self { Self::Type => { let mut static_type = replace_lifetimes(field_type, "static").to_token_stream(); - if field_attrs.skip { + if attrs.skip { static_type = quote! { ::core::marker::PhantomData<#static_type> }; } - let private = private(crate_name); + let private = &attrs.private; quote! { #global_field_name: <#static_type as #private::Encode>::Encoder, } @@ -51,7 +49,7 @@ impl crate::shared::Item for Item { }, Self::Encode | Self::EncodeVectored => { let static_type = replace_lifetimes(field_type, "static"); - let value = if field_attrs.skip { + let value = if attrs.skip { quote! { { let _ = #field_name; @@ -112,7 +110,7 @@ impl crate::shared::Item for Item { fn enum_impl( self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, variant_count: usize, variant_index_type: VariantIndexType, pattern: impl Fn(usize) -> TokenStream, @@ -124,7 +122,7 @@ impl crate::shared::Item for Item { Self::Type => { let variants = encode_variants .then(|| { - let private = private(crate_name); + let private = &attrs.private; quote! { variants: #private::VariantEncoder<#variant_index_type, #variant_count>, } }) .unwrap_or_default(); @@ -239,8 +237,8 @@ impl crate::shared::Derive<{ Item::COUNT }> for Encode { type Item = Item; const ALL: [Self::Item; Item::COUNT] = Item::ALL; - fn bound(&self, crate_name: &Path) -> Path { - let private = private(crate_name); + fn bound(&self, attrs: &BitcodeDeriveAttrs) -> Path { + let private = &attrs.private; parse_quote!(#private::Encode) } @@ -250,7 +248,7 @@ impl crate::shared::Derive<{ Item::COUNT }> for Encode { fn derive_impl( &self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, output: [TokenStream; Item::COUNT], ident: Ident, mut generics: Generics, @@ -269,7 +267,7 @@ impl crate::shared::Derive<{ Item::COUNT }> for Encode { output; let encoder_ident = Ident::new(&format!("{ident}Encoder"), Span::call_site()); let encoder_ty = quote! { #encoder_ident #encoder_generics }; - let private = private(crate_name); + let private = &attrs.private; quote! { #[allow(clippy::pedantic)] diff --git a/bitcode_derive/src/lib.rs b/bitcode_derive/src/lib.rs index d8e50c2..0a572cd 100644 --- a/bitcode_derive/src/lib.rs +++ b/bitcode_derive/src/lib.rs @@ -2,7 +2,6 @@ use crate::decode::Decode; use crate::encode::Encode; use crate::shared::Derive; use proc_macro::TokenStream; -use quote::quote; use syn::spanned::Spanned; use syn::{parse_macro_input, DeriveInput, Error}; @@ -33,7 +32,3 @@ pub(crate) fn error(spanned: &impl Spanned, s: &str) -> Error { pub(crate) fn err(spanned: &impl Spanned, s: &str) -> Result { Err(error(spanned, s)) } - -pub(crate) fn private(crate_name: &syn::Path) -> proc_macro2::TokenStream { - quote! { #crate_name::__private } -} diff --git a/bitcode_derive/src/shared.rs b/bitcode_derive/src/shared.rs index 879e114..5444e94 100644 --- a/bitcode_derive/src/shared.rs +++ b/bitcode_derive/src/shared.rs @@ -1,4 +1,6 @@ -use crate::attribute::BitcodeAttrs; +use crate::attribute::{ + BitcodeDeriveAttrs, BitcodeDeriveOrVariantAttrs, BitcodeFieldAttrs, BitcodeVariantAttrs, +}; use crate::bound::FieldBounds; use crate::err; use proc_macro2::{Ident, Span, TokenStream}; @@ -68,12 +70,11 @@ impl ToTokens for VariantIndexType { pub trait Item: Copy + Sized { fn field_impl( self, - crate_name: &Path, + attrs: &BitcodeFieldAttrs, field_name: TokenStream, global_field_name: TokenStream, real_field_name: TokenStream, field_type: &Type, - field_attrs: &BitcodeAttrs, ) -> TokenStream; fn struct_impl( @@ -85,7 +86,7 @@ pub trait Item: Copy + Sized { fn enum_impl( self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, variant_count: usize, variant_index_type: VariantIndexType, pattern: impl Fn(usize) -> TokenStream, @@ -94,10 +95,9 @@ pub trait Item: Copy + Sized { fn field_impls( self, - crate_name: &Path, + attrs: &[BitcodeFieldAttrs], global_prefix: Option<&str>, fields: &Fields, - attrs: &Vec, ) -> TokenStream { fields .iter() @@ -114,7 +114,7 @@ pub trait Item: Copy + Sized { }) .unwrap_or_else(|| name.clone()); - self.field_impl(crate_name, name, global_name, real_name, &field.ty, attrs) + self.field_impl(attrs, name, global_name, real_name, &field.ty) }) .collect() } @@ -125,7 +125,7 @@ pub trait Derive { const ALL: [Self::Item; ITEM_COUNT]; /// `Encode` in `T: Encode`. - fn bound(&self, crate_name: &Path) -> Path; + fn bound(&self, attrs: &BitcodeDeriveAttrs) -> Path; /// Bound for skipped fields, e.g. `Default` fn skip_bound(&self) -> Option; @@ -133,28 +133,27 @@ pub trait Derive { /// Generates the derive implementation. fn derive_impl( &self, - crate_name: &Path, + attrs: &BitcodeDeriveAttrs, output: [TokenStream; ITEM_COUNT], ident: Ident, generics: Generics, any_static_borrow: bool, ) -> TokenStream; - fn field_attrs( + fn field_attrs<'attr>( &self, - crate_name: &Path, + attrs: BitcodeDeriveOrVariantAttrs<'attr>, fields: &Fields, - attrs: &BitcodeAttrs, bounds: &mut FieldBounds, - ) -> Result> { + ) -> Result>> { fields .iter() - .map(|field| { - let field_attrs = BitcodeAttrs::parse_field(&field.attrs, attrs)?; + .map(move |field| { + let field_attrs = BitcodeFieldAttrs::parse(&field.attrs, attrs)?; let bound = if field_attrs.skip { self.skip_bound() } else { - Some(self.bound(crate_name)) + Some(self.bound(&attrs)) }; if let Some(bound) = bound { bounds.add_bound_type(field.clone(), &field_attrs, bound); @@ -165,7 +164,7 @@ pub trait Derive { } fn derive(&self, mut input: DeriveInput) -> Result { - let attrs = BitcodeAttrs::parse_derive(&input.attrs)?; + let attrs = BitcodeDeriveAttrs::parse(&input.attrs)?; let ident = input.ident; syn::visit_mut::visit_data_mut(&mut ReplaceSelves(&ident), &mut input.data); let mut bounds = FieldBounds::default(); @@ -173,13 +172,15 @@ pub trait Derive { let output = match input.data { Data::Struct(DataStruct { ref fields, .. }) => { // Used for adding `bounds` and skipping fields. Would be used by `#[bitcode(with_serde)]`. - let field_attrs = - self.field_attrs(&attrs.crate_name, fields, &attrs, &mut bounds)?; + let field_attrs = self.field_attrs( + BitcodeDeriveOrVariantAttrs::Derive(&attrs), + fields, + &mut bounds, + )?; let destructure_fields = &destructure_fields(fields); Self::ALL.map(|item| { - let field_impls = - item.field_impls(&attrs.crate_name, None, fields, &field_attrs); + let field_impls = item.field_impls(&field_attrs, None, fields); item.struct_impl(&ident, destructure_fields, &field_impls) }) } @@ -201,18 +202,28 @@ pub trait Derive { } // Used for adding `bounds` and skipping fields. Would be used by `#[bitcode(with_serde)]`. - let variant_attrs = data_enum + let variants_with_attrs = data_enum .variants .iter() .map(|variant| { - let attrs = BitcodeAttrs::parse_variant(&variant.attrs, &attrs)?; - self.field_attrs(&attrs.crate_name, &variant.fields, &attrs, &mut bounds) + let variant_attrs = BitcodeVariantAttrs::parse(&variant.attrs, &attrs)?; + Ok((variant, variant_attrs)) + }) + .collect::>>()?; + let variant_field_attrs = variants_with_attrs + .iter() + .map(|(variant, variant_attrs)| { + self.field_attrs( + BitcodeDeriveOrVariantAttrs::Variant(&variant_attrs), + &variant.fields, + &mut bounds, + ) }) .collect::>>()?; Self::ALL.map(|item| { item.enum_impl( - &attrs.crate_name, + &attrs, data_enum.variants.len(), variant_index_type, |i| { @@ -225,13 +236,12 @@ pub trait Derive { }, |item, i| { let variant = &data_enum.variants[i]; - let variant_attrs = &variant_attrs[i]; + let variant_field_attrs = &variant_field_attrs[i]; let global_prefix = format!("{}_", &variant.ident); item.field_impls( - &attrs.crate_name, + variant_field_attrs, Some(&global_prefix), &variant.fields, - variant_attrs, ) }, ) @@ -240,13 +250,7 @@ pub trait Derive { Data::Union(_) => err(&ident, "unions are not supported")?, }; let (generics, any_static_borrow) = bounds.added_to(input.generics); - Ok(self.derive_impl( - &attrs.crate_name, - output, - ident, - generics, - any_static_borrow, - )) + Ok(self.derive_impl(&attrs, output, ident, generics, any_static_borrow)) } } diff --git a/src/derive/mod.rs b/src/derive/mod.rs index 7f31584..e0eed37 100644 --- a/src/derive/mod.rs +++ b/src/derive/mod.rs @@ -114,6 +114,28 @@ impl crate::buffer::Buffer { } } +/// ``` +/// use bitcode::{Encode, Decode}; +/// #[derive(Encode, Decode)] +/// struct Test { +/// #[bitcode(skip)] +/// x: u32, +/// y: u32, +/// } +/// ``` +/// ```compile_fail +/// use bitcode::{Encode, Decode}; +/// #[derive(Encode, Decode)] +/// struct Test { +/// #[bitcode(skip)] +/// #[bitcode(skip)] +/// x: u32, +/// y: u32, +/// } +/// ``` +#[doc(hidden)] +pub fn _cant_duplicate_skip() {} + #[cfg(test)] mod tests { use crate::{Decode, Encode};