From 503fa470f5b64d6e2ec86cf39171799685cb617a Mon Sep 17 00:00:00 2001 From: Kitlith Date: Sun, 20 Aug 2023 17:08:31 -0700 Subject: [PATCH 01/16] WIP: bitsize derive on generic structs --- bilge-impl/src/bitsize.rs | 10 ++++-- bilge-impl/src/bitsize_internal.rs | 51 ++++++++++++++++++++++++------ bilge-impl/src/default_bits.rs | 2 +- bilge-impl/src/from_bits.rs | 2 +- bilge-impl/src/try_from_bits.rs | 2 +- tests/struct.rs | 6 ++++ 6 files changed, 59 insertions(+), 14 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 6c8d306..22b6bb4 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -121,7 +121,13 @@ fn analyze_enum(bitsize: BitSize, variants: Iter) { } fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { - let ItemStruct { vis, ident, fields, .. } = item; + let ItemStruct { + vis, + ident, + fields, + generics, + .. + } = item; let declared_bitsize = declared_bitsize as usize; let computed_bitsize = fields.iter().fold(quote!(0), |acc, next| { @@ -144,7 +150,7 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { }; quote! { - #vis struct #ident #fields_def + #vis struct #ident #generics #fields_def // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize const _: () = assert!( diff --git a/bilge-impl/src/bitsize_internal.rs b/bilge-impl/src/bitsize_internal.rs index e6061a7..94e3713 100644 --- a/bilge-impl/src/bitsize_internal.rs +++ b/bilge-impl/src/bitsize_internal.rs @@ -1,6 +1,6 @@ use proc_macro2::{Ident, TokenStream}; use quote::quote; -use syn::{Attribute, Field, Item, ItemEnum, ItemStruct, Type}; +use syn::{Attribute, Field, Generics, Item, ItemEnum, ItemStruct, Type}; use crate::shared::{self, unreachable}; @@ -12,6 +12,7 @@ struct ItemIr<'a> { name: &'a Ident, /// generated item (and setters, getters, constructor, impl Bitsized) expanded: TokenStream, + generics: &'a Generics, } pub(super) fn bitsize_internal(args: TokenStream, item: TokenStream) -> TokenStream { @@ -21,13 +22,25 @@ pub(super) fn bitsize_internal(args: TokenStream, item: TokenStream) -> TokenStr let expanded = generate_struct(item, &arb_int); let attrs = &item.attrs; let name = &item.ident; - ItemIr { attrs, name, expanded } + let generics = &item.generics; + ItemIr { + attrs, + name, + expanded, + generics, + } } Item::Enum(ref item) => { let expanded = generate_enum(item); let attrs = &item.attrs; let name = &item.ident; - ItemIr { attrs, name, expanded } + let generics = &item.generics; + ItemIr { + attrs, + name, + expanded, + generics, + } } _ => unreachable(()), }; @@ -41,7 +54,19 @@ fn parse(item: TokenStream, args: TokenStream) -> (Item, TokenStream) { } fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStream { - let ItemStruct { vis, ident, fields, .. } = struct_data; + let ItemStruct { + vis, + ident, + fields, + generics, + .. + } = struct_data; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + + let phantom_ty = generics.type_params().map(|e| &e.ident).map(|ident| quote!(#ident)); + let phantom_lt = generics.lifetimes().map(|l| &l.lifetime).map(|lifetime| quote!(& #lifetime ())); + // TODO: integrate user-provided PhantomData somehow? (so that the user can set the variance) + let phantom = phantom_ty.chain(phantom_lt); let mut fieldless_next_int = 0; let mut previous_field_sizes = vec![]; @@ -67,11 +92,12 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre let const_ = if cfg!(feature = "nightly") { quote!(const) } else { quote!() }; quote! { - #vis struct #ident { + #vis struct #ident #generics { /// WARNING: modifying this value directly can break invariants value: #arb_int, + _phantom: ::core::marker::PhantomData<(#(#phantom),*)> } - impl #ident { + impl #impl_generics #ident #ty_generics #where_clause { // #[inline] #[allow(clippy::too_many_arguments, clippy::type_complexity, unused_parens)] pub #const_ fn new(#( #constructor_args )*) -> Self { @@ -81,7 +107,7 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre let mut offset = 0; let raw_value = #( #constructor_parts )|*; let value = #arb_int::new(raw_value); - Self { value } + Self { value, _phantom: ::core::marker::PhantomData } } #( #accessors )* } @@ -221,12 +247,19 @@ fn generate_enum(enum_data: &ItemEnum) -> TokenStream { /// We have _one_ `generate_common` function, which holds everything struct and enum have _in common_. /// Everything else has its own `generate_` functions. fn generate_common(ir: ItemIr, arb_int: &TokenStream) -> TokenStream { - let ItemIr { attrs, name, expanded } = ir; + let ItemIr { + attrs, + name, + expanded, + generics, + } = ir; + + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { #(#attrs)* #expanded - impl ::bilge::Bitsized for #name { + impl #impl_generics ::bilge::Bitsized for #name #ty_generics #where_clause { type ArbitraryInt = #arb_int; const BITS: usize = ::BITS; const MAX: Self::ArbitraryInt = ::MAX; diff --git a/bilge-impl/src/default_bits.rs b/bilge-impl/src/default_bits.rs index f8ce9df..ad1b6d2 100644 --- a/bilge-impl/src/default_bits.rs +++ b/bilge-impl/src/default_bits.rs @@ -29,7 +29,7 @@ fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields) -> TokenSt let mut offset = 0; let value = #default_value; let value = <#struct_name as Bitsized>::ArbitraryInt::new(value); - Self { value } + Self { value, _phantom: ::core::marker::PhantomData } } } } diff --git a/bilge-impl/src/from_bits.rs b/bilge-impl/src/from_bits.rs index 39bbe5d..08d1da3 100644 --- a/bilge-impl/src/from_bits.rs +++ b/bilge-impl/src/from_bits.rs @@ -156,7 +156,7 @@ fn generate_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) - impl #const_ ::core::convert::From<#arb_int> for #struct_type { fn from(value: #arb_int) -> Self { #( #assumes )* - Self { value } + Self { value, _phantom: ::core::marker::PhantomData } } } impl #const_ ::core::convert::From<#struct_type> for #arb_int { diff --git a/bilge-impl/src/try_from_bits.rs b/bilge-impl/src/try_from_bits.rs index 3c8bf08..d77b37b 100644 --- a/bilge-impl/src/try_from_bits.rs +++ b/bilge-impl/src/try_from_bits.rs @@ -119,7 +119,7 @@ fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) -> let is_ok: bool = {#is_ok}; if is_ok { - Ok(Self { value }) + Ok(Self { value, _phantom: ::core::marker::PhantomData }) } else { Err(::bilge::give_me_error()) } diff --git a/tests/struct.rs b/tests/struct.rs index a948ad9..58008b7 100644 --- a/tests/struct.rs +++ b/tests/struct.rs @@ -510,6 +510,12 @@ impl_from!(T; Generic => u2; |val| val.0); #[derive(DefaultBits, PartialEq, DebugBits, FromBits)] struct UsingGeneric(Generic<()>); +// TODO: bitsize doesn't work here yet because of the size check: it's trying to get the size of Generic in a non-generic context +// #[bitsize(2)] +// FromBits, DefaultBits, and DebugBits don't work yet because they need the generics threaded through to the correct spot. +// #[derive(PartialEq)] // TODO: FromBits, DefaultBits, DebugBits +// struct IsGeneric(Generic); + #[bitsize(2)] #[derive(DefaultBits, PartialEq, DebugBits, TryFromBits)] struct UsingGenericUnfilled(Generic<()>); From 8c44e5be090b7160226e2423366f0caea9f0c354 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 29 Aug 2023 19:57:28 -0700 Subject: [PATCH 02/16] Potential fix for asserting bitsize on generic structs --- bilge-impl/src/bitsize.rs | 20 ++++++++++++-------- tests/struct.rs | 7 +++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 22b6bb4..477855d 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -149,17 +149,21 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { } }; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + quote! { #vis struct #ident #generics #fields_def - // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize - const _: () = assert!( - (#computed_bitsize) == (#declared_bitsize), - concat!("struct size and declared bit size differ: ", - // stringify!(#computed_bitsize), - " != ", - stringify!(#declared_bitsize)) - ); + impl #impl_generics #ident #ty_generics #where_clause { + // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize + const _bitsize_check: () = assert!( + (#computed_bitsize) == (#declared_bitsize), + concat!("struct size and declared bit size differ: ", + // stringify!(#computed_bitsize), + " != ", + stringify!(#declared_bitsize)) + ); + } } } diff --git a/tests/struct.rs b/tests/struct.rs index 58008b7..bc52dad 100644 --- a/tests/struct.rs +++ b/tests/struct.rs @@ -510,11 +510,10 @@ impl_from!(T; Generic => u2; |val| val.0); #[derive(DefaultBits, PartialEq, DebugBits, FromBits)] struct UsingGeneric(Generic<()>); -// TODO: bitsize doesn't work here yet because of the size check: it's trying to get the size of Generic in a non-generic context -// #[bitsize(2)] +#[bitsize(2)] // FromBits, DefaultBits, and DebugBits don't work yet because they need the generics threaded through to the correct spot. -// #[derive(PartialEq)] // TODO: FromBits, DefaultBits, DebugBits -// struct IsGeneric(Generic); +#[derive(PartialEq)] // TODO: FromBits, DefaultBits, DebugBits +struct IsGeneric(Generic); #[bitsize(2)] #[derive(DefaultBits, PartialEq, DebugBits, TryFromBits)] From 453f7d93af83fbf8ab18645f93ea7020b3b5b267 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 29 Aug 2023 21:15:11 -0700 Subject: [PATCH 03/16] Potentially fix the rest of the derive macros for generic structs (Drawing the rest of the owl) The workaround for DebugBits and DefaultBits is a little overkill, but much more trivial than trying to figure out whether we need to emit the where clause for each field. We should be able to filter it in the future if necessary. Perhaps this should also be refactored into a shared helper function, since it was copy-pasted into two locations. --- bilge-impl/src/debug_bits.rs | 22 ++++++++++++++++++---- bilge-impl/src/default_bits.rs | 28 +++++++++++++++++++++------- bilge-impl/src/fmt_bits.rs | 14 ++++++++------ bilge-impl/src/from_bits.rs | 18 ++++++++++-------- bilge-impl/src/shared.rs | 8 ++++---- bilge-impl/src/try_from_bits.rs | 17 ++++++++++------- tests/struct.rs | 7 +++++-- 7 files changed, 76 insertions(+), 38 deletions(-) diff --git a/bilge-impl/src/debug_bits.rs b/bilge-impl/src/debug_bits.rs index 6921f6b..61a89d8 100644 --- a/bilge-impl/src/debug_bits.rs +++ b/bilge-impl/src/debug_bits.rs @@ -1,7 +1,7 @@ use proc_macro2::{Ident, TokenStream}; use proc_macro_error::abort_call_site; use quote::quote; -use syn::{Data, Fields}; +use syn::{Data, Fields, WhereClause, WherePredicate}; use crate::shared::{self, unreachable}; @@ -17,7 +17,7 @@ pub(super) fn debug_bits(item: TokenStream) -> TokenStream { }; let fmt_impl = match struct_data.fields { - Fields::Named(fields) => { + Fields::Named(ref fields) => { let calls = fields.named.iter().map(|f| { // We can unwrap since this is a named field let call = f.ident.as_ref().unwrap(); @@ -30,7 +30,7 @@ pub(super) fn debug_bits(item: TokenStream) -> TokenStream { #(#calls)*.finish() } } - Fields::Unnamed(fields) => { + Fields::Unnamed(ref fields) => { let calls = fields.unnamed.iter().map(|_| { let call: Ident = syn::parse_str(&format!("val_{}", fieldless_next_int)).unwrap_or_else(unreachable); fieldless_next_int += 1; @@ -45,8 +45,22 @@ pub(super) fn debug_bits(item: TokenStream) -> TokenStream { Fields::Unit => todo!("this is a unit struct, which is not supported right now"), }; + let (impl_generics, ty_generics, where_clause) = derive_input.generics.split_for_impl(); + let mut where_clause = where_clause.map(<_>::clone).unwrap_or_else(|| WhereClause { + where_token: <_>::default(), + predicates: <_>::default(), + }); + + // NOTE: This is a little overkill, as it adds where clauses for concrete types as well + // but this is easier than trying to figure out exactly what types we need to add clauses for. + where_clause.predicates.extend(struct_data.fields.iter().map(|e| { + let ty = &e.ty; + let res: WherePredicate = syn::parse_quote!(#ty : ::core::fmt::Debug); + res + })); + quote! { - impl ::core::fmt::Debug for #name { + impl #impl_generics ::core::fmt::Debug for #name #ty_generics #where_clause { fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { #fmt_impl } diff --git a/bilge-impl/src/default_bits.rs b/bilge-impl/src/default_bits.rs index ad1b6d2..662ae82 100644 --- a/bilge-impl/src/default_bits.rs +++ b/bilge-impl/src/default_bits.rs @@ -1,34 +1,48 @@ use proc_macro2::{Ident, TokenStream}; use proc_macro_error::abort_call_site; use quote::quote; -use syn::{Data, DeriveInput, Fields, Type}; +use syn::{Data, DeriveInput, Fields, Generics, Type, WhereClause, WherePredicate}; use crate::shared::{self, fallback::Fallback, unreachable, BitSize}; pub(crate) fn default_bits(item: TokenStream) -> TokenStream { let derive_input = parse(item); //TODO: does fallback need handling? - let (derive_data, _, name, ..) = analyze(&derive_input); + let (derive_data, _, name, generics, ..) = analyze(&derive_input); match derive_data { - Data::Struct(data) => generate_struct_default_impl(name, &data.fields), + Data::Struct(data) => generate_struct_default_impl(name, &data.fields, generics), Data::Enum(_) => abort_call_site!("use derive(Default) for enums"), _ => unreachable(()), } } -fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields) -> TokenStream { +fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields, generics: &Generics) -> TokenStream { let default_value = fields .iter() .map(|field| generate_default_inner(&field.ty)) .reduce(|acc, next| quote!(#acc | #next)); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let mut where_clause = where_clause.map(<_>::clone).unwrap_or_else(|| WhereClause { + where_token: <_>::default(), + predicates: <_>::default(), + }); + + // NOTE: This is a little overkill, as it adds where clauses for concrete types as well + // but this is easier than trying to figure out exactly what types we need to add clauses for. + where_clause.predicates.extend(fields.iter().map(|e| { + let ty = &e.ty; + let res: WherePredicate = syn::parse_quote!(#ty : ::core::default::Default); + res + })); + quote! { - impl ::core::default::Default for #struct_name { + impl #impl_generics ::core::default::Default for #struct_name #ty_generics #where_clause { fn default() -> Self { let mut offset = 0; let value = #default_value; - let value = <#struct_name as Bitsized>::ArbitraryInt::new(value); + let value = <#struct_name #ty_generics as Bitsized>::ArbitraryInt::new(value); Self { value, _phantom: ::core::marker::PhantomData } } } @@ -87,6 +101,6 @@ fn parse(item: TokenStream) -> DeriveInput { shared::parse_derive(item) } -fn analyze(derive_input: &DeriveInput) -> (&Data, TokenStream, &Ident, BitSize, Option) { +fn analyze(derive_input: &DeriveInput) -> (&Data, TokenStream, &Ident, &Generics, BitSize, Option) { shared::analyze_derive(derive_input, false) } diff --git a/bilge-impl/src/fmt_bits.rs b/bilge-impl/src/fmt_bits.rs index 1a4e6a7..934fbf6 100644 --- a/bilge-impl/src/fmt_bits.rs +++ b/bilge-impl/src/fmt_bits.rs @@ -1,21 +1,21 @@ use proc_macro2::{Ident, TokenStream}; use quote::quote; -use syn::{punctuated::Iter, Data, DeriveInput, Fields, Variant}; +use syn::{punctuated::Iter, Data, DeriveInput, Fields, Generics, Variant}; use crate::shared::{self, discriminant_assigner::DiscriminantAssigner, fallback::Fallback, unreachable, BitSize}; pub(crate) fn binary(item: TokenStream) -> TokenStream { let derive_input = parse(item); - let (derive_data, arb_int, name, bitsize, fallback) = analyze(&derive_input); + let (derive_data, arb_int, name, generics, bitsize, fallback) = analyze(&derive_input); match derive_data { - Data::Struct(data) => generate_struct_binary_impl(name, &data.fields), + Data::Struct(data) => generate_struct_binary_impl(name, &data.fields, generics), Data::Enum(data) => generate_enum_binary_impl(name, data.variants.iter(), arb_int, bitsize, fallback), _ => unreachable(()), } } -fn generate_struct_binary_impl(struct_name: &Ident, fields: &Fields) -> TokenStream { +fn generate_struct_binary_impl(struct_name: &Ident, fields: &Fields, generics: &Generics) -> TokenStream { let write_underscore = quote! { write!(f, "_")?; }; // fields are printed from most significant to least significant, separated by an underscore @@ -37,8 +37,10 @@ fn generate_struct_binary_impl(struct_name: &Ident, fields: &Fields) -> TokenStr }) .reduce(|acc, next| quote!(#acc #write_underscore #next)); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + quote! { - impl ::core::fmt::Binary for #struct_name { + impl #impl_generics ::core::fmt::Binary for #struct_name #ty_generics #where_clause { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let struct_size = <#struct_name as Bitsized>::BITS; let mut last_bit_pos = struct_size; @@ -107,6 +109,6 @@ fn parse(item: TokenStream) -> DeriveInput { shared::parse_derive(item) } -fn analyze(derive_input: &DeriveInput) -> (&Data, TokenStream, &Ident, BitSize, Option) { +fn analyze(derive_input: &DeriveInput) -> (&Data, TokenStream, &Ident, &Generics, BitSize, Option) { shared::analyze_derive(derive_input, false) } diff --git a/bilge-impl/src/from_bits.rs b/bilge-impl/src/from_bits.rs index 08d1da3..89e3ca7 100644 --- a/bilge-impl/src/from_bits.rs +++ b/bilge-impl/src/from_bits.rs @@ -2,15 +2,15 @@ use itertools::Itertools; use proc_macro2::{Ident, TokenStream}; use proc_macro_error::{abort, abort_call_site}; use quote::quote; -use syn::{punctuated::Iter, Data, DeriveInput, Fields, Type, Variant}; +use syn::{punctuated::Iter, Data, DeriveInput, Fields, Generics, Type, Variant}; use crate::shared::{self, discriminant_assigner::DiscriminantAssigner, enum_fills_bitsize, fallback::Fallback, unreachable, BitSize}; pub(super) fn from_bits(item: TokenStream) -> TokenStream { let derive_input = parse(item); - let (derive_data, arb_int, name, internal_bitsize, fallback) = analyze(&derive_input); + let (derive_data, arb_int, name, generics, internal_bitsize, fallback) = analyze(&derive_input); let expanded = match &derive_data { - Data::Struct(struct_data) => generate_struct(arb_int, name, &struct_data.fields), + Data::Struct(struct_data) => generate_struct(arb_int, name, &struct_data.fields, generics), Data::Enum(enum_data) => { let variants = enum_data.variants.iter(); let match_arms = analyze_enum(variants, name, internal_bitsize, fallback.as_ref(), &arb_int); @@ -25,7 +25,7 @@ fn parse(item: TokenStream) -> DeriveInput { shared::parse_derive(item) } -fn analyze(derive_input: &DeriveInput) -> (&syn::Data, TokenStream, &Ident, BitSize, Option) { +fn analyze(derive_input: &DeriveInput) -> (&syn::Data, TokenStream, &Ident, &Generics, BitSize, Option) { shared::analyze_derive(derive_input, false) } @@ -141,7 +141,7 @@ fn generate_filled_check_for(ty: &Type, vec: &mut Vec) { } } -fn generate_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) -> TokenStream { +fn generate_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields, generics: &Generics) -> TokenStream { let const_ = if cfg!(feature = "nightly") { quote!(const) } else { quote!() }; let mut assumes = Vec::new(); @@ -152,15 +152,17 @@ fn generate_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) - // a single check per type is enough, so the checks can be deduped let assumes = assumes.into_iter().unique_by(TokenStream::to_string); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + quote! { - impl #const_ ::core::convert::From<#arb_int> for #struct_type { + impl #impl_generics #const_ ::core::convert::From<#arb_int> for #struct_type #ty_generics #where_clause { fn from(value: #arb_int) -> Self { #( #assumes )* Self { value, _phantom: ::core::marker::PhantomData } } } - impl #const_ ::core::convert::From<#struct_type> for #arb_int { - fn from(value: #struct_type) -> Self { + impl #impl_generics #const_ ::core::convert::From<#struct_type #ty_generics> for #arb_int #where_clause { + fn from(value: #struct_type #ty_generics) -> Self { value.value } } diff --git a/bilge-impl/src/shared.rs b/bilge-impl/src/shared.rs index 8f490b8..1b48df2 100644 --- a/bilge-impl/src/shared.rs +++ b/bilge-impl/src/shared.rs @@ -6,7 +6,7 @@ use fallback::{fallback_variant, Fallback}; use proc_macro2::{Ident, Literal, TokenStream}; use proc_macro_error::{abort, abort_call_site}; use quote::quote; -use syn::{Attribute, DeriveInput, LitInt, Meta, Type}; +use syn::{Attribute, DeriveInput, Generics, LitInt, Meta, Type}; use util::PathExt; /// As arbitrary_int is limited to basic rust primitives, the maximum is u128. @@ -23,11 +23,11 @@ pub(crate) fn parse_derive(item: TokenStream) -> DeriveInput { // allow since we want `if try_from` blocks to stand out #[allow(clippy::collapsible_if)] -pub(crate) fn analyze_derive(derive_input: &DeriveInput, try_from: bool) -> (&syn::Data, TokenStream, &Ident, BitSize, Option) { +pub(crate) fn analyze_derive(derive_input: &DeriveInput, try_from: bool) -> (&syn::Data, TokenStream, &Ident, &Generics, BitSize, Option) { let DeriveInput { attrs, ident, - // generics, + generics, data, .. } = derive_input; @@ -57,7 +57,7 @@ pub(crate) fn analyze_derive(derive_input: &DeriveInput, try_from: bool) -> (&sy abort_call_site!("fallback is not allowed with `TryFromBits`"; help = "use `#[derive(FromBits)]` or remove this `#[fallback]`") } - (data, arb_int, ident, bitsize, fallback) + (data, arb_int, ident, generics, bitsize, fallback) } // If we want to support bitsize(u4) besides bitsize(4), do that here. diff --git a/bilge-impl/src/try_from_bits.rs b/bilge-impl/src/try_from_bits.rs index d77b37b..797961a 100644 --- a/bilge-impl/src/try_from_bits.rs +++ b/bilge-impl/src/try_from_bits.rs @@ -1,6 +1,7 @@ use proc_macro2::{Ident, TokenStream}; use proc_macro_error::{abort, emit_call_site_warning}; use quote::quote; +use syn::Generics; use syn::{punctuated::Iter, Data, DeriveInput, Fields, Type, Variant}; use crate::shared::{self, discriminant_assigner::DiscriminantAssigner, enum_fills_bitsize, fallback::Fallback, unreachable, BitSize}; @@ -8,9 +9,9 @@ use crate::shared::{bitsize_from_type_ident, last_ident_of_path}; pub(super) fn try_from_bits(item: TokenStream) -> TokenStream { let derive_input = parse(item); - let (derive_data, arb_int, name, internal_bitsize, ..) = analyze(&derive_input); + let (derive_data, arb_int, name, generics, internal_bitsize, ..) = analyze(&derive_input); match derive_data { - Data::Struct(ref data) => codegen_struct(arb_int, name, &data.fields), + Data::Struct(ref data) => codegen_struct(arb_int, name, &data.fields, generics), Data::Enum(ref enum_data) => { let variants = enum_data.variants.iter(); let match_arms = analyze_enum(variants, name, internal_bitsize, &arb_int); @@ -24,7 +25,7 @@ fn parse(item: TokenStream) -> DeriveInput { shared::parse_derive(item) } -fn analyze(derive_input: &DeriveInput) -> (&syn::Data, TokenStream, &Ident, BitSize, Option) { +fn analyze(derive_input: &DeriveInput) -> (&syn::Data, TokenStream, &Ident, &Generics, BitSize, Option) { shared::analyze_derive(derive_input, true) } @@ -81,7 +82,7 @@ fn generate_field_check(ty: &Type) -> TokenStream { crate::bitsize_internal::struct_gen::generate_getter_inner(ty, false) } -fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) -> TokenStream { +fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields, generics: &Generics) -> TokenStream { let is_ok: TokenStream = fields .iter() .map(|field| { @@ -104,8 +105,10 @@ fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) -> let const_ = if cfg!(feature = "nightly") { quote!(const) } else { quote!() }; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + quote! { - impl #const_ ::core::convert::TryFrom<#arb_int> for #struct_type { + impl #impl_generics #const_ ::core::convert::TryFrom<#arb_int> for #struct_type #ty_generics #where_clause { type Error = ::bilge::BitsError; // validates all values, which means enums, even in inner structs (TODO: and reserved fields?) @@ -126,8 +129,8 @@ fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields) -> } } - impl #const_ ::core::convert::From<#struct_type> for #arb_int { - fn from(struct_value: #struct_type) -> Self { + impl #impl_generics #const_ ::core::convert::From<#struct_type #ty_generics> for #arb_int #where_clause { + fn from(struct_value: #struct_type #ty_generics) -> Self { struct_value.value } } diff --git a/tests/struct.rs b/tests/struct.rs index bc52dad..76385cf 100644 --- a/tests/struct.rs +++ b/tests/struct.rs @@ -511,10 +511,13 @@ impl_from!(T; Generic => u2; |val| val.0); struct UsingGeneric(Generic<()>); #[bitsize(2)] -// FromBits, DefaultBits, and DebugBits don't work yet because they need the generics threaded through to the correct spot. -#[derive(PartialEq)] // TODO: FromBits, DefaultBits, DebugBits +#[derive(DefaultBits, PartialEq, DebugBits, FromBits)] struct IsGeneric(Generic); +#[bitsize(2)] +#[derive(DefaultBits, PartialEq, DebugBits, TryFromBits)] +struct IsGenericUnfilled(Generic); + #[bitsize(2)] #[derive(DefaultBits, PartialEq, DebugBits, TryFromBits)] struct UsingGenericUnfilled(Generic<()>); From f912e2978ee8a9c72cbdc50f2e88cd863d240c27 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Sun, 3 Sep 2023 18:57:06 -0700 Subject: [PATCH 04/16] fixup! switch to uppercase to avoid warning --- bilge-impl/src/bitsize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 477855d..734712f 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -156,7 +156,7 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { impl #impl_generics #ident #ty_generics #where_clause { // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize - const _bitsize_check: () = assert!( + const _BITSIZE_CHECK: () = assert!( (#computed_bitsize) == (#declared_bitsize), concat!("struct size and declared bit size differ: ", // stringify!(#computed_bitsize), From 9e1c5419eb67f907c6542d1957303e980db03afb Mon Sep 17 00:00:00 2001 From: Kitlith Date: Sun, 3 Sep 2023 18:58:13 -0700 Subject: [PATCH 05/16] Switch from bounding on field type to bounding on generics. In some respects, this is a downgrade, but it matches the behaviour of the standard library derives, syn, serde, etc. --- bilge-impl/src/debug_bits.rs | 8 ++++---- bilge-impl/src/default_bits.rs | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bilge-impl/src/debug_bits.rs b/bilge-impl/src/debug_bits.rs index 61a89d8..ceeda48 100644 --- a/bilge-impl/src/debug_bits.rs +++ b/bilge-impl/src/debug_bits.rs @@ -51,10 +51,10 @@ pub(super) fn debug_bits(item: TokenStream) -> TokenStream { predicates: <_>::default(), }); - // NOTE: This is a little overkill, as it adds where clauses for concrete types as well - // but this is easier than trying to figure out exactly what types we need to add clauses for. - where_clause.predicates.extend(struct_data.fields.iter().map(|e| { - let ty = &e.ty; + // NOTE: This is not *ideal*, but it's approximately what the standard library does, + // for various reasons. see https://github.com/rust-lang/rust/issues/26925 + where_clause.predicates.extend(derive_input.generics.type_params().map(|t| { + let ty = &t.ident; let res: WherePredicate = syn::parse_quote!(#ty : ::core::fmt::Debug); res })); diff --git a/bilge-impl/src/default_bits.rs b/bilge-impl/src/default_bits.rs index 662ae82..6883f50 100644 --- a/bilge-impl/src/default_bits.rs +++ b/bilge-impl/src/default_bits.rs @@ -29,11 +29,11 @@ fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields, generics: predicates: <_>::default(), }); - // NOTE: This is a little overkill, as it adds where clauses for concrete types as well - // but this is easier than trying to figure out exactly what types we need to add clauses for. - where_clause.predicates.extend(fields.iter().map(|e| { - let ty = &e.ty; - let res: WherePredicate = syn::parse_quote!(#ty : ::core::default::Default); + // NOTE: This is not *ideal*, but it's approximately what the standard library does, + // for various reasons. see https://github.com/rust-lang/rust/issues/26925 + where_clause.predicates.extend(generics.type_params().map(|t| { + let ty = &t.ident; + let res: WherePredicate = syn::parse_quote!(#ty : ::core::fmt::Debug); res })); From 123ee67661140bd6170bbf476fff533bb6aa0200 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Sun, 3 Sep 2023 19:06:21 -0700 Subject: [PATCH 06/16] fixup! default impl needs to bound on default, not debug. This was a copy-paste error. --- bilge-impl/src/default_bits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bilge-impl/src/default_bits.rs b/bilge-impl/src/default_bits.rs index 6883f50..0939225 100644 --- a/bilge-impl/src/default_bits.rs +++ b/bilge-impl/src/default_bits.rs @@ -33,7 +33,7 @@ fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields, generics: // for various reasons. see https://github.com/rust-lang/rust/issues/26925 where_clause.predicates.extend(generics.type_params().map(|t| { let ty = &t.ident; - let res: WherePredicate = syn::parse_quote!(#ty : ::core::fmt::Debug); + let res: WherePredicate = syn::parse_quote!(#ty : ::core::default::Default); res })); From c3cc057d0091eebe80ef2dc1af9304f475e3cde4 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 4 Sep 2023 22:05:08 -0700 Subject: [PATCH 07/16] Pass where clause on struct through bitsize macros --- bilge-impl/src/bitsize.rs | 26 +++++--------------------- bilge-impl/src/bitsize_internal.rs | 2 +- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 734712f..9f218e0 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -121,13 +121,7 @@ fn analyze_enum(bitsize: BitSize, variants: Iter) { } fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { - let ItemStruct { - vis, - ident, - fields, - generics, - .. - } = item; + let ItemStruct { ident, fields, generics, .. } = item; let declared_bitsize = declared_bitsize as usize; let computed_bitsize = fields.iter().fold(quote!(0), |acc, next| { @@ -135,24 +129,14 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { quote!(#acc + #field_size) }); - // we could remove this if the whole struct gets passed - let is_tuple_struct = fields.iter().any(|field| field.ident.is_none()); - let fields_def = if is_tuple_struct { - let fields = fields.iter(); - quote! { - ( #(#fields,)* ); - } - } else { - let fields = fields.iter(); - quote! { - { #(#fields,)* } - } - }; + // The only part of the struct we don't want to pass through are the attributes + let mut item = item.clone(); + item.attrs = Vec::new(); let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { - #vis struct #ident #generics #fields_def + #item impl #impl_generics #ident #ty_generics #where_clause { // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize diff --git a/bilge-impl/src/bitsize_internal.rs b/bilge-impl/src/bitsize_internal.rs index 94e3713..795ae38 100644 --- a/bilge-impl/src/bitsize_internal.rs +++ b/bilge-impl/src/bitsize_internal.rs @@ -92,7 +92,7 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre let const_ = if cfg!(feature = "nightly") { quote!(const) } else { quote!() }; quote! { - #vis struct #ident #generics { + #vis struct #ident #generics #where_clause { /// WARNING: modifying this value directly can break invariants value: #arb_int, _phantom: ::core::marker::PhantomData<(#(#phantom),*)> From 62441dfba18a91fd52a28a943bb2d457765582bc Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 12 Sep 2023 16:46:49 -0700 Subject: [PATCH 08/16] Use struct update syntax for ItemStruct Co-authored-by: pickx --- bilge-impl/src/bitsize.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 9f218e0..db214ee 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -129,9 +129,10 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { quote!(#acc + #field_size) }); - // The only part of the struct we don't want to pass through are the attributes - let mut item = item.clone(); - item.attrs = Vec::new(); + let item = ItemStruct { + attrs: Vec::new(), + ..item.clone() + }; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); From dd670831c44c9ed18cdf3e180d016d1e7b5960be Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 12 Sep 2023 16:58:14 -0700 Subject: [PATCH 09/16] Seperate phantom type generation into a function. --- bilge-impl/src/bitsize_internal.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/bilge-impl/src/bitsize_internal.rs b/bilge-impl/src/bitsize_internal.rs index 795ae38..ec50146 100644 --- a/bilge-impl/src/bitsize_internal.rs +++ b/bilge-impl/src/bitsize_internal.rs @@ -63,11 +63,6 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre } = struct_data; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let phantom_ty = generics.type_params().map(|e| &e.ident).map(|ident| quote!(#ident)); - let phantom_lt = generics.lifetimes().map(|l| &l.lifetime).map(|lifetime| quote!(& #lifetime ())); - // TODO: integrate user-provided PhantomData somehow? (so that the user can set the variance) - let phantom = phantom_ty.chain(phantom_lt); - let mut fieldless_next_int = 0; let mut previous_field_sizes = vec![]; let (accessors, (constructor_args, constructor_parts)): (Vec, (Vec, Vec)) = fields @@ -91,11 +86,13 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre let const_ = if cfg!(feature = "nightly") { quote!(const) } else { quote!() }; + let phantom_type = generate_phantom_type(generics); + quote! { #vis struct #ident #generics #where_clause { /// WARNING: modifying this value directly can break invariants value: #arb_int, - _phantom: ::core::marker::PhantomData<(#(#phantom),*)> + _phantom: ::core::marker::PhantomData<#phantom_type> } impl #impl_generics #ident #ty_generics #where_clause { // #[inline] @@ -114,6 +111,21 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre } } +/// Returns a tuple with the following types, in order: +/// - The original struct's type parameters +/// - References to `()` bound by the original struct's lifetime parameters +/// If there are 0 generics, the type will simply be `()`. +/// If there is a single generic type or lifetime, the type will not wrapped in a tuple. +fn generate_phantom_type(generics: &Generics) -> TokenStream { + let phantom_ty = generics.type_params().map(|e| &e.ident).map(|ident| quote!(#ident)); + let phantom_lt = generics.lifetimes().map(|l| &l.lifetime).map(|lifetime| quote!(& #lifetime ())); + // TODO: integrate user-provided PhantomData somehow? (so that the user can set the variance) + let phantom = phantom_ty.chain(phantom_lt); + quote! { + (#(#phantom),*) + } +} + fn generate_field(field: &Field, field_offset: &TokenStream, fieldless_next_int: &mut usize) -> (TokenStream, (TokenStream, TokenStream)) { let Field { ident, ty, .. } = field; let name = if let Some(ident) = ident { From 5f501fe127576041746a9d671682849d7eb32767 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 12 Sep 2023 18:33:19 -0600 Subject: [PATCH 10/16] Refactor where clause modification into shared function --- bilge-impl/src/debug_bits.rs | 14 ++------------ bilge-impl/src/default_bits.rs | 14 ++------------ bilge-impl/src/shared.rs | 31 ++++++++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/bilge-impl/src/debug_bits.rs b/bilge-impl/src/debug_bits.rs index ceeda48..43f350f 100644 --- a/bilge-impl/src/debug_bits.rs +++ b/bilge-impl/src/debug_bits.rs @@ -1,7 +1,7 @@ use proc_macro2::{Ident, TokenStream}; use proc_macro_error::abort_call_site; use quote::quote; -use syn::{Data, Fields, WhereClause, WherePredicate}; +use syn::{Data, Fields}; use crate::shared::{self, unreachable}; @@ -46,18 +46,8 @@ pub(super) fn debug_bits(item: TokenStream) -> TokenStream { }; let (impl_generics, ty_generics, where_clause) = derive_input.generics.split_for_impl(); - let mut where_clause = where_clause.map(<_>::clone).unwrap_or_else(|| WhereClause { - where_token: <_>::default(), - predicates: <_>::default(), - }); - // NOTE: This is not *ideal*, but it's approximately what the standard library does, - // for various reasons. see https://github.com/rust-lang/rust/issues/26925 - where_clause.predicates.extend(derive_input.generics.type_params().map(|t| { - let ty = &t.ident; - let res: WherePredicate = syn::parse_quote!(#ty : ::core::fmt::Debug); - res - })); + let where_clause = shared::generate_trait_where_clause(&derive_input.generics, where_clause, quote!(::core::fmt::Debug)); quote! { impl #impl_generics ::core::fmt::Debug for #name #ty_generics #where_clause { diff --git a/bilge-impl/src/default_bits.rs b/bilge-impl/src/default_bits.rs index 0939225..b0f3073 100644 --- a/bilge-impl/src/default_bits.rs +++ b/bilge-impl/src/default_bits.rs @@ -1,7 +1,7 @@ use proc_macro2::{Ident, TokenStream}; use proc_macro_error::abort_call_site; use quote::quote; -use syn::{Data, DeriveInput, Fields, Generics, Type, WhereClause, WherePredicate}; +use syn::{Data, DeriveInput, Fields, Generics, Type}; use crate::shared::{self, fallback::Fallback, unreachable, BitSize}; @@ -24,18 +24,8 @@ fn generate_struct_default_impl(struct_name: &Ident, fields: &Fields, generics: .reduce(|acc, next| quote!(#acc | #next)); let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let mut where_clause = where_clause.map(<_>::clone).unwrap_or_else(|| WhereClause { - where_token: <_>::default(), - predicates: <_>::default(), - }); - // NOTE: This is not *ideal*, but it's approximately what the standard library does, - // for various reasons. see https://github.com/rust-lang/rust/issues/26925 - where_clause.predicates.extend(generics.type_params().map(|t| { - let ty = &t.ident; - let res: WherePredicate = syn::parse_quote!(#ty : ::core::default::Default); - res - })); + let where_clause = shared::generate_trait_where_clause(generics, where_clause, quote!(::core::default::Default)); quote! { impl #impl_generics ::core::default::Default for #struct_name #ty_generics #where_clause { diff --git a/bilge-impl/src/shared.rs b/bilge-impl/src/shared.rs index 1b48df2..a88a7e5 100644 --- a/bilge-impl/src/shared.rs +++ b/bilge-impl/src/shared.rs @@ -2,11 +2,13 @@ pub mod discriminant_assigner; pub mod fallback; pub mod util; +use std::borrow::Cow; + use fallback::{fallback_variant, Fallback}; use proc_macro2::{Ident, Literal, TokenStream}; use proc_macro_error::{abort, abort_call_site}; use quote::quote; -use syn::{Attribute, DeriveInput, Generics, LitInt, Meta, Type}; +use syn::{Attribute, DeriveInput, Generics, LitInt, Meta, Type, WhereClause, WherePredicate}; use util::PathExt; /// As arbitrary_int is limited to basic rust primitives, the maximum is u128. @@ -194,3 +196,30 @@ pub(crate) fn bitsize_internal_arg(attr: &Attribute) -> Option { None } + +pub(crate) fn generate_trait_where_clause<'a>( + generics: &Generics, existing_clause: Option<&'a WhereClause>, req_trait: TokenStream, +) -> Option> { + // NOTE: This is not *ideal*, but it's approximately what the standard library does, + // for various reasons. see https://github.com/rust-lang/rust/issues/26925 + let mut extra_predicates = generics + .type_params() + .map(|t| { + let ty = &t.ident; + let res: WherePredicate = syn::parse_quote!(#ty : #req_trait); + res + }) + .peekable(); + + let predicates = match (existing_clause, extra_predicates.peek()) { + (Some(clause), Some(_)) => clause.predicates.iter().cloned().chain(extra_predicates).collect(), + (Some(clause), None) => return Some(Cow::Borrowed(clause)), + (None, Some(_)) => extra_predicates.collect(), + (None, None) => return None, + }; + + Some(Cow::Owned(WhereClause { + where_token: existing_clause.map(|c| c.where_token).unwrap_or(<_>::default()), + predicates, + })) +} From 3071cf968888c8ea6a4c9973ad407ff337f91fe3 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 12 Sep 2023 18:55:13 -0600 Subject: [PATCH 11/16] fixup! use unwrap_or_default() --- bilge-impl/src/shared.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bilge-impl/src/shared.rs b/bilge-impl/src/shared.rs index a88a7e5..6ed2411 100644 --- a/bilge-impl/src/shared.rs +++ b/bilge-impl/src/shared.rs @@ -219,7 +219,7 @@ pub(crate) fn generate_trait_where_clause<'a>( }; Some(Cow::Owned(WhereClause { - where_token: existing_clause.map(|c| c.where_token).unwrap_or(<_>::default()), + where_token: existing_clause.map(|c| c.where_token).unwrap_or_default(), predicates, })) } From d6e3b124b107491cba40d9e42974c3d6afd779eb Mon Sep 17 00:00:00 2001 From: Kitlith Date: Tue, 12 Sep 2023 18:55:40 -0600 Subject: [PATCH 12/16] Rejigger the assert generation to match current idea Still needs some methods to explicitly reference the constant in order to produce a compile error; --- bilge-impl/src/bitsize.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index db214ee..2e2bc40 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -139,16 +139,24 @@ fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { quote! { #item - impl #impl_generics #ident #ty_generics #where_clause { - // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize - const _BITSIZE_CHECK: () = assert!( - (#computed_bitsize) == (#declared_bitsize), - concat!("struct size and declared bit size differ: ", - // stringify!(#computed_bitsize), - " != ", - stringify!(#declared_bitsize)) - ); - } + const _: () = { + // TODO: This is useless without methods that reference it, and this is un-namable outside of this block. + // Move or add some method implementations inside this block? + trait Assertion { + const SIZE_CHECK: (); + } + + impl #impl_generics Assertion for #ident #ty_generics #where_clause { + // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize + const SIZE_CHECK: () = assert!( + (#computed_bitsize) == (#declared_bitsize), + concat!("struct size and declared bit size differ: ", + // stringify!(#computed_bitsize), + " != ", + stringify!(#declared_bitsize)) + ); + } + }; } } From 3e489c55c244e0b04c9902d27876367634108fca Mon Sep 17 00:00:00 2001 From: Kitlith Date: Wed, 13 Sep 2023 12:07:51 -0600 Subject: [PATCH 13/16] Experimental: move assert generation into bitsize_internal This allows me to make accesses to the consts on the Bitsize trait error. We could nix the Assertion trait here and just use the check expression directly. --- bilge-impl/src/bitsize.rs | 31 +-------- bilge-impl/src/bitsize_internal.rs | 104 ++++++++++++++++++----------- 2 files changed, 67 insertions(+), 68 deletions(-) diff --git a/bilge-impl/src/bitsize.rs b/bilge-impl/src/bitsize.rs index 2e2bc40..5773f8f 100644 --- a/bilge-impl/src/bitsize.rs +++ b/bilge-impl/src/bitsize.rs @@ -120,43 +120,14 @@ fn analyze_enum(bitsize: BitSize, variants: Iter) { } } -fn generate_struct(item: &ItemStruct, declared_bitsize: u8) -> TokenStream { - let ItemStruct { ident, fields, generics, .. } = item; - let declared_bitsize = declared_bitsize as usize; - - let computed_bitsize = fields.iter().fold(quote!(0), |acc, next| { - let field_size = shared::generate_type_bitsize(&next.ty); - quote!(#acc + #field_size) - }); - +fn generate_struct(item: &ItemStruct, _declared_bitsize: u8) -> TokenStream { let item = ItemStruct { attrs: Vec::new(), ..item.clone() }; - let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - quote! { #item - - const _: () = { - // TODO: This is useless without methods that reference it, and this is un-namable outside of this block. - // Move or add some method implementations inside this block? - trait Assertion { - const SIZE_CHECK: (); - } - - impl #impl_generics Assertion for #ident #ty_generics #where_clause { - // constness: when we get const blocks evaluated at compile time, add a const computed_bitsize - const SIZE_CHECK: () = assert!( - (#computed_bitsize) == (#declared_bitsize), - concat!("struct size and declared bit size differ: ", - // stringify!(#computed_bitsize), - " != ", - stringify!(#declared_bitsize)) - ); - } - }; } } diff --git a/bilge-impl/src/bitsize_internal.rs b/bilge-impl/src/bitsize_internal.rs index ec50146..81ffdbc 100644 --- a/bilge-impl/src/bitsize_internal.rs +++ b/bilge-impl/src/bitsize_internal.rs @@ -13,52 +13,32 @@ struct ItemIr<'a> { /// generated item (and setters, getters, constructor, impl Bitsized) expanded: TokenStream, generics: &'a Generics, + check_expr: TokenStream, } pub(super) fn bitsize_internal(args: TokenStream, item: TokenStream) -> TokenStream { - let (item, arb_int) = parse(item, args); + let (item, arb_int, declared_bitsize) = parse(item, args); let ir = match item { - Item::Struct(ref item) => { - let expanded = generate_struct(item, &arb_int); - let attrs = &item.attrs; - let name = &item.ident; - let generics = &item.generics; - ItemIr { - attrs, - name, - expanded, - generics, - } - } - Item::Enum(ref item) => { - let expanded = generate_enum(item); - let attrs = &item.attrs; - let name = &item.ident; - let generics = &item.generics; - ItemIr { - attrs, - name, - expanded, - generics, - } - } + Item::Struct(ref item) => generate_struct(item, &arb_int, declared_bitsize), + Item::Enum(ref item) => generate_enum(item), _ => unreachable(()), }; generate_common(ir, &arb_int) } -fn parse(item: TokenStream, args: TokenStream) -> (Item, TokenStream) { +fn parse(item: TokenStream, args: TokenStream) -> (Item, TokenStream, u8) { let item = syn::parse2(item).unwrap_or_else(unreachable); - let (_declared_bitsize, arb_int) = shared::bitsize_and_arbitrary_int_from(args); - (item, arb_int) + let (declared_bitsize, arb_int) = shared::bitsize_and_arbitrary_int_from(args); + (item, arb_int, declared_bitsize) } -fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStream { +fn generate_struct<'a>(struct_data: &'a ItemStruct, arb_int: &TokenStream, declared_bitsize: u8) -> ItemIr<'a> { let ItemStruct { vis, ident, fields, generics, + attrs, .. } = struct_data; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); @@ -88,7 +68,7 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre let phantom_type = generate_phantom_type(generics); - quote! { + let expanded = quote! { #vis struct #ident #generics #where_clause { /// WARNING: modifying this value directly can break invariants value: #arb_int, @@ -108,6 +88,29 @@ fn generate_struct(struct_data: &ItemStruct, arb_int: &TokenStream) -> TokenStre } #( #accessors )* } + }; + + let computed_bitsize = fields.iter().fold(quote!(0), |acc, next| { + let field_size = shared::generate_type_bitsize(&next.ty); + quote!(#acc + #field_size) + }); + + let declared_bitsize = declared_bitsize as usize; + + let check_expr = quote!(assert!( + (#computed_bitsize) == (#declared_bitsize), + concat!("struct size and declared bit size differ: ", + // stringify!(#computed_bitsize), + " != ", + stringify!(#declared_bitsize)) + )); + + ItemIr { + attrs, + name: ident, + expanded, + generics, + check_expr, } } @@ -247,12 +250,26 @@ fn generate_constructor_stuff(ty: &Type, name: &Ident) -> (TokenStream, TokenStr (constructor_arg, constructor_part) } -fn generate_enum(enum_data: &ItemEnum) -> TokenStream { - let ItemEnum { vis, ident, variants, .. } = enum_data; - quote! { +fn generate_enum(enum_data: &ItemEnum) -> ItemIr { + let ItemEnum { + vis, + ident, + variants, + generics, + attrs, + .. + } = enum_data; + let expanded = quote! { #vis enum #ident { #variants } + }; + ItemIr { + attrs, + name: ident, + expanded, + generics, + check_expr: quote! { () }, } } @@ -264,6 +281,7 @@ fn generate_common(ir: ItemIr, arb_int: &TokenStream) -> TokenStream { name, expanded, generics, + check_expr, } = ir; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); @@ -271,10 +289,20 @@ fn generate_common(ir: ItemIr, arb_int: &TokenStream) -> TokenStream { quote! { #(#attrs)* #expanded - impl #impl_generics ::bilge::Bitsized for #name #ty_generics #where_clause { - type ArbitraryInt = #arb_int; - const BITS: usize = ::BITS; - const MAX: Self::ArbitraryInt = ::MAX; - } + const _: () = { + trait Assertion { + const SIZE_CHECK: (); + } + + impl #impl_generics Assertion for #name #ty_generics #where_clause { + const SIZE_CHECK: () = #check_expr; + } + + impl #impl_generics ::bilge::Bitsized for #name #ty_generics #where_clause { + type ArbitraryInt = #arb_int; + const BITS: usize = (::BITS, ::SIZE_CHECK).0; + const MAX: Self::ArbitraryInt = (::MAX, ::SIZE_CHECK).0; + } + }; } } From fa2c3d26625cb0107a0e48199df5b159f387268e Mon Sep 17 00:00:00 2001 From: Kitlith Date: Fri, 15 Sep 2023 09:42:13 -0600 Subject: [PATCH 14/16] Gate FromBits and TryFromBits on Bitsized::BITS not panicing. --- bilge-impl/src/from_bits.rs | 2 ++ bilge-impl/src/try_from_bits.rs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/bilge-impl/src/from_bits.rs b/bilge-impl/src/from_bits.rs index 89e3ca7..f5098ef 100644 --- a/bilge-impl/src/from_bits.rs +++ b/bilge-impl/src/from_bits.rs @@ -157,12 +157,14 @@ fn generate_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields, g quote! { impl #impl_generics #const_ ::core::convert::From<#arb_int> for #struct_type #ty_generics #where_clause { fn from(value: #arb_int) -> Self { + ::BITS; #( #assumes )* Self { value, _phantom: ::core::marker::PhantomData } } } impl #impl_generics #const_ ::core::convert::From<#struct_type #ty_generics> for #arb_int #where_clause { fn from(value: #struct_type #ty_generics) -> Self { + <#struct_type #ty_generics as Bitsized>::BITS; value.value } } diff --git a/bilge-impl/src/try_from_bits.rs b/bilge-impl/src/try_from_bits.rs index 797961a..88f1e4f 100644 --- a/bilge-impl/src/try_from_bits.rs +++ b/bilge-impl/src/try_from_bits.rs @@ -65,6 +65,7 @@ fn codegen_enum(arb_int: TokenStream, enum_type: &Ident, match_arms: (Vec ::core::result::Result { + ::BITS; match number.value() { #( #from_int_match_arms )* i => Err(::bilge::give_me_error()), @@ -116,6 +117,8 @@ fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields, ge type ArbIntOf = ::ArbitraryInt; type BaseIntOf = as Number>::UnderlyingType; + ::BITS; + // cursor starts at value's first field let mut cursor = value.value(); @@ -131,6 +134,7 @@ fn codegen_struct(arb_int: TokenStream, struct_type: &Ident, fields: &Fields, ge impl #impl_generics #const_ ::core::convert::From<#struct_type #ty_generics> for #arb_int #where_clause { fn from(struct_value: #struct_type #ty_generics) -> Self { + <#struct_type #ty_generics as Bitsized>::BITS; struct_value.value } } From 853f418fec1505cb057f87927f7d0cd400a170af Mon Sep 17 00:00:00 2001 From: Kitlith Date: Fri, 15 Sep 2023 11:28:06 -0600 Subject: [PATCH 15/16] fixup! s/T/Self/ --- bilge-impl/src/try_from_bits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bilge-impl/src/try_from_bits.rs b/bilge-impl/src/try_from_bits.rs index 88f1e4f..ad970e2 100644 --- a/bilge-impl/src/try_from_bits.rs +++ b/bilge-impl/src/try_from_bits.rs @@ -65,7 +65,7 @@ fn codegen_enum(arb_int: TokenStream, enum_type: &Ident, match_arms: (Vec ::core::result::Result { - ::BITS; + ::BITS; match number.value() { #( #from_int_match_arms )* i => Err(::bilge::give_me_error()), From d58ce821bbad3d5bf124e2aa77f2d6de5cc0cf93 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Fri, 15 Sep 2023 16:21:53 -0600 Subject: [PATCH 16/16] Add test to check that where clauses are preserved by the derive macro --- tests/ui/where-clause-is-preserved.rs | 40 +++++++++++++++++++++ tests/ui/where-clause-is-preserved.stderr | 44 +++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/ui/where-clause-is-preserved.rs create mode 100644 tests/ui/where-clause-is-preserved.stderr diff --git a/tests/ui/where-clause-is-preserved.rs b/tests/ui/where-clause-is-preserved.rs new file mode 100644 index 0000000..3383119 --- /dev/null +++ b/tests/ui/where-clause-is-preserved.rs @@ -0,0 +1,40 @@ +#![feature(const_trait_impl)] + +use bilge::prelude::*; +use core::marker::PhantomData; + +#[derive(Default)] +struct Generic(u2, PhantomData); + +impl Bitsized for Generic { + type ArbitraryInt = u2; + const BITS: usize = 2; + const MAX: Self::ArbitraryInt = ::MAX; +} + +impl const From for Generic { + fn from(val: u2) -> Self { + Self(val, PhantomData) + } +} + +impl const From> for u2 { + fn from(val: Generic) -> u2 { + val.0 + } +} + +#[bitsize(5)] +struct BoundedGeneric(Generic, u3) +where + T: Iterator; + +fn main() { + // kinda ick, but the only way I could find to check that the clause was on the struct directly + BoundedGeneric::<()> { + value: <_>::default(), + _phantom: PhantomData, + }; + // check that where clause is present on inherent impl + BoundedGeneric::<()>::new(<_>::default(), u3::new(0)); +} diff --git a/tests/ui/where-clause-is-preserved.stderr b/tests/ui/where-clause-is-preserved.stderr new file mode 100644 index 0000000..2ac8671 --- /dev/null +++ b/tests/ui/where-clause-is-preserved.stderr @@ -0,0 +1,44 @@ +error[E0277]: `()` is not an iterator + --> tests/ui/where-clause-is-preserved.rs:34:22 + | +34 | BoundedGeneric::<()> { + | ^^ `()` is not an iterator + | + = help: the trait `Iterator` is not implemented for `()` +note: required by a bound in `BoundedGeneric` + --> tests/ui/where-clause-is-preserved.rs:30:8 + | +28 | struct BoundedGeneric(Generic, u3) + | -------------- required by a bound in this +29 | where +30 | T: Iterator; + | ^^^^^^^^^^^^^^^^^^^ required by this bound in `BoundedGeneric` + +error[E0599]: the function or associated item `new` exists for struct `BoundedGeneric<()>`, but its trait bounds were not satisfied + --> tests/ui/where-clause-is-preserved.rs:39:27 + | +27 | #[bitsize(5)] + | ------------- function or associated item `new` not found for this struct +... +39 | BoundedGeneric::<()>::new(<_>::default(), u3::new(0)); + | ^^^ function or associated item cannot be called on `BoundedGeneric<()>` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `<() as Iterator>::Item = ()` + `(): Iterator` + +error[E0277]: `()` is not an iterator + --> tests/ui/where-clause-is-preserved.rs:39:5 + | +39 | BoundedGeneric::<()>::new(<_>::default(), u3::new(0)); + | ^^^^^^^^^^^^^^^^^^^^ `()` is not an iterator + | + = help: the trait `Iterator` is not implemented for `()` +note: required by a bound in `BoundedGeneric` + --> tests/ui/where-clause-is-preserved.rs:30:8 + | +28 | struct BoundedGeneric(Generic, u3) + | -------------- required by a bound in this +29 | where +30 | T: Iterator; + | ^^^^^^^^^^^^^^^^^^^ required by this bound in `BoundedGeneric`