From 1cd147d55f597bcffb7781f94d33e81930923fef Mon Sep 17 00:00:00 2001 From: Daniel Fraenkel Date: Wed, 3 May 2023 04:29:30 +0200 Subject: [PATCH] Issue 374 (#385) * Added integration tests to rkyv_typename This is primarily done for testing issue #374. However, in addition to the original issue some other errors in the generated code from the TypeName derive macro were encountered: 1) const generics suffer from essentially the same issue 2) A name collision in the generic parameter F which occurs in the build_type_name(...) method. The same name F is used in the generated code. In stead it should be given a name which is unlikely to clash with code that invokes the macro. In addition to failing tests some trivial tests were added which do not fail: - basic_enum - generic_one_param - generic_with_trait_bounds - generic_two_params - basic_struct - trivial_enum - trivial_struct - trivial_tuple_struct - trivial_unit_struct The following test fails to compile due to the name collision with the generic parameter F: - generic_name_colisions The following tests fail to compile due to the original bug in issue #374: - lifetimes - lifetimes_two_lifetimes_with_bounds - combined_generic_type_and_lifetime The following test fails to compile due to the analogous related const-generic issue. - const_generics The following test checks checks some more potential errors and fails to compile due to combinations of these issues. - composite_cases * Fix failing integration tests for rkyv_typename Fixes issue #374 as well as #384 and adds a possible solution for issue #383. - Issue #384 is a minor fix so I took the liberty of fixing it here. - Issue #383 is non orthogonal to #374. I.e. some code involving issue #383 is necessary to make everything work. It may however require further discussion so issue #383 should probably remain open. * additional test cases for const generics - one of these fails because the const char generic creates output which does not surround the char with the expected quotes. Also: cosmetics * fixes const_generic_char test Fixed by formatting the const generic value with Debug in stead of Display. Formatting with Debug creates the expected quotes in opposition to Display. This implementation should be sound for rust 1.68 but this code may break in subtle or less subtle ways when new const-generic features are added to the compiler. Added comments about this in the code. Related issue #383 --- rkyv_typename/tests/test_derive.rs | 253 +++++++++++++++++++++++++++++ rkyv_typename_derive/src/lib.rs | 63 ++++--- 2 files changed, 290 insertions(+), 26 deletions(-) create mode 100644 rkyv_typename/tests/test_derive.rs diff --git a/rkyv_typename/tests/test_derive.rs b/rkyv_typename/tests/test_derive.rs new file mode 100644 index 00000000..c7865824 --- /dev/null +++ b/rkyv_typename/tests/test_derive.rs @@ -0,0 +1,253 @@ +#![cfg(test)] +use rkyv_typename::TypeName; + +fn check_typename(type_name_check: &str) { + let mut type_name = String::new(); + let mod_path = module_path!(); + let type_name_check = format!("{mod_path}::{type_name_check}"); + T::build_type_name(|piece| type_name += piece); + assert_eq!(type_name, type_name_check) +} + +// Test that the TypeName derive works for the most trivial cases + +#[test] +fn trivial_struct() { + #[derive(TypeName)] + struct Struct {} + check_typename::("Struct"); +} + +#[test] +fn trivial_unit_struct() { + #[derive(TypeName)] + struct Struct; + check_typename::("Struct"); +} + +#[test] +fn trivial_tuple_struct() { + #[derive(TypeName)] + struct Struct(); + check_typename::("Struct"); +} + +#[test] +fn trivial_enum() { + #[derive(TypeName)] + enum Enum {} + check_typename::("Enum"); +} + +// Test non generic structs and enums + +#[test] +#[allow(dead_code)] +fn basic_struct() { + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + } + check_typename::("Struct"); +} + +#[test] +#[allow(dead_code)] +fn basic_enum() { + #[derive(TypeName)] + enum Enum { + CaseA(u32), + CaseB(u128), + } + check_typename::("Enum"); +} + +// Test basic generic structs + +#[test] +#[allow(dead_code)] +fn generic_one_param() { + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + c: C, + } + check_typename::>("Struct<()>"); +} + +#[test] +#[allow(dead_code)] +fn generic_name_collisions() { + // The only difference with generic_one_param() is the type parameter F + // which could cause a name collision with the generic parameter in + // build_type_name(...) method. + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + c: F, + } + check_typename::>("Struct<()>"); +} + +#[test] +#[allow(dead_code)] +fn generic_two_params() { + #[derive(TypeName)] + // More than one type parameter + struct Struct { + a: u32, + b: u128, + c: (C, D), + } + check_typename::>("Struct<(), u8>"); +} + +// Test that trait bounds are generated correctly + +#[test] +#[allow(dead_code)] +fn generic_with_trait_bounds() { + #[derive(TypeName)] + struct Struct + where + D: std::fmt::Display + Copy, + { + a: u32, + b: u128, + c: (C, D), + } + check_typename::>("Struct<(), u8>"); +} + +// Test types with lifetimes + +#[test] +#[allow(dead_code)] +fn lifetimes() { + #[derive(TypeName)] + struct Struct<'b> { + a: u32, + b: &'b u128, + } + check_typename::("Struct"); +} + +#[test] +#[allow(dead_code)] +fn two_lifetimes_with_bounds() { + #[derive(TypeName)] + struct Struct<'b, 'c> + where + 'b: 'c, + { + a: u32, + b: &'b u128, + c: &'c i128, + } + check_typename::("Struct"); +} + +#[test] +#[allow(dead_code)] +fn combined_generic_type_and_lifetime() { + #[derive(TypeName)] + struct Struct<'a, C> { + a: u32, + b: &'a u128, + c: C, + } + check_typename::>("Struct<()>"); +} + +// Test const generics + +#[test] +#[allow(dead_code)] +fn const_generic_num() { + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + } + check_typename::>("Struct<77>"); +} + +#[test] +#[allow(dead_code)] +fn const_generic_bool() { + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + } + check_typename::>("Struct"); +} + +#[test] +#[allow(dead_code)] +fn const_generic_char() { + #[derive(TypeName)] + struct Struct { + a: u32, + b: u128, + } + check_typename::>("Struct<'a'>"); +} + +// Test more composite cases which could potentially cause problems such as: +// - Nested types +// - Compex trait bounds +// - Combination of lifetimes, generic types and const generics + +#[test] +#[allow(dead_code)] +fn composite_cases() { + #[derive(TypeName)] + struct Sub<'a, D, const N: usize>(u32, &'a u128, D) + where + D: 'a; + + #[derive(TypeName)] + enum Enum<'a, D, const N: usize> + where + D: std::fmt::Debug + ?Sized, + { + Variant1(&'a D), + Variant2(Sub<'a, &'a D, 77>), + } + + #[derive(TypeName)] + struct Struct<'a, C: std::fmt::Debug, D: Clone, E, F, G: ?Sized, const N: usize, const B: bool> + where + 'a: 'static, + C: Clone + 'a, + D: 'static + ?Sized, + { + a: u32, + b: &'a u128, + c: C, + d: Sub<'a, D, 44>, + e: E, + f: F, + g: G, + } + let mod_path = module_path!(); + check_typename::< + Struct>, &(&Enum<(u8, i8), 99>,), [u16], 33, false>, + >( + format!( + "Struct<\ + alloc::string::String, \ + [[alloc::string::String; 3]; 2], \ + alloc::vec::Vec>, \ + &(&{mod_path}::Enum<(u8, i8), 99>,), \ + [u16], \ + 33, \ + false\ + >") + .as_str(), + ); +} diff --git a/rkyv_typename_derive/src/lib.rs b/rkyv_typename_derive/src/lib.rs index 9628dacd..83dc2947 100644 --- a/rkyv_typename_derive/src/lib.rs +++ b/rkyv_typename_derive/src/lib.rs @@ -10,7 +10,9 @@ extern crate proc_macro; use proc_macro2::TokenStream; use quote::quote; -use syn::{parse_macro_input, spanned::Spanned, AttrStyle, DeriveInput, Error, Lit, Meta}; +use syn::{ + parse_macro_input, spanned::Spanned, AttrStyle, DeriveInput, Error, Lit, Meta, GenericParam +}; #[derive(Default)] struct Attributes { @@ -65,20 +67,7 @@ fn derive_type_name_impl(input: &DeriveInput) -> TokenStream { Err(error) => return error, }; - let generic_params = input.generics.params.iter().map(|p| quote! { #p }); - let generic_args = input.generics.type_params().map(|p| { - let name = &p.ident; - quote! { #name } - }); - let generic_predicates = match input.generics.where_clause { - Some(ref clause) => { - let predicates = clause.predicates.iter().map(|p| quote! { #p }); - quote! { #(#predicates,)* } - } - None => quote! {}, - }; - - let type_wheres = input.generics.type_params().map(|p| { + let typename_where_predicates = input.generics.type_params().map(|p| { let name = &p.ident; quote! { #name: rkyv_typename::TypeName } }); @@ -95,18 +84,35 @@ fn derive_type_name_impl(input: &DeriveInput) -> TokenStream { .typename .unwrap_or_else(|| input.ident.to_string()); - let build_args = if !input.generics.params.is_empty() { - let mut results = input.generics.type_params().map(|p| { - let name = &p.ident; - quote! { #name::build_type_name(&mut f) } - }); - let first = results.next().unwrap(); + let mut generics = input.generics.params.iter().filter_map(|p| { + match p { + GenericParam::Type(t) => { + let name = &t.ident; + Some(quote! { #name::build_type_name(&mut f) }) + } + GenericParam::Const(c) => { + let value = &c.ident; + Some(quote!{ + // This works for all const generic types that are supported by rust 1.68 or + // below. It happens to be, for these types, that the Debug trait is + // implemented in such a way that it generates the correct output for the + // expected behavior. + // However newer versions of the compiler may break this code in subtle and/or + // less subtle ways. + let const_val = &format!("{:?}", #value); + f(const_val); + }) + } + GenericParam::Lifetime(_) => None, + } + }); + let build_args = if let Some(first) = generics.next() { let name_str = format!("{}<", name_str); quote! { #module_path f(#name_str); #first; - #(f(", "); #results;)* + #(f(", "); #generics;)* f(">"); } } else { @@ -116,16 +122,21 @@ fn derive_type_name_impl(input: &DeriveInput) -> TokenStream { } }; + let (impl_generics, ty_generics, where_clause) = + input.generics.split_for_impl(); + let standard_derive_where_predicates = where_clause.map( + |w|&w.predicates + ); quote! { const _: () = { use rkyv_typename::TypeName; - impl<#(#generic_params,)*> TypeName for #name<#(#generic_args,)*> + impl #impl_generics TypeName for #name #ty_generics where - #generic_predicates - #(#type_wheres,)* + #(#typename_where_predicates,)* + #standard_derive_where_predicates { - fn build_type_name(mut f: F) { + fn build_type_name(mut f: TYPENAME__F) { #build_args } }