Skip to content

Commit

Permalink
Issue 374 (rkyv#385)
Browse files Browse the repository at this point in the history
* Added integration tests to rkyv_typename

This is primarily done for testing issue rkyv#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<F: FnMut(&str)>(...) 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 rkyv#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 rkyv#374 as well as rkyv#384 and adds a possible solution for
issue rkyv#383.

- Issue rkyv#384 is a minor fix so I took the liberty of fixing it here.
- Issue rkyv#383 is non orthogonal to rkyv#374. I.e. some code involving
issue rkyv#383 is necessary to make everything work. It may however
require further discussion so issue rkyv#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 rkyv#383
  • Loading branch information
edfraenkel authored May 3, 2023
1 parent 71919e0 commit 1cd147d
Show file tree
Hide file tree
Showing 2 changed files with 290 additions and 26 deletions.
253 changes: 253 additions & 0 deletions rkyv_typename/tests/test_derive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
#![cfg(test)]
use rkyv_typename::TypeName;

fn check_typename<T: TypeName + ?Sized>(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>("Struct");
}

#[test]
fn trivial_unit_struct() {
#[derive(TypeName)]
struct Struct;
check_typename::<Struct>("Struct");
}

#[test]
fn trivial_tuple_struct() {
#[derive(TypeName)]
struct Struct();
check_typename::<Struct>("Struct");
}

#[test]
fn trivial_enum() {
#[derive(TypeName)]
enum Enum {}
check_typename::<Enum>("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>("Struct");
}

#[test]
#[allow(dead_code)]
fn basic_enum() {
#[derive(TypeName)]
enum Enum {
CaseA(u32),
CaseB(u128),
}
check_typename::<Enum>("Enum");
}

// Test basic generic structs

#[test]
#[allow(dead_code)]
fn generic_one_param() {
#[derive(TypeName)]
struct Struct<C> {
a: u32,
b: u128,
c: C,
}
check_typename::<Struct<()>>("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<F: FnMut(&str)>(...) method.
#[derive(TypeName)]
struct Struct<F> {
a: u32,
b: u128,
c: F,
}
check_typename::<Struct<()>>("Struct<()>");
}

#[test]
#[allow(dead_code)]
fn generic_two_params() {
#[derive(TypeName)]
// More than one type parameter
struct Struct<C, D> {
a: u32,
b: u128,
c: (C, D),
}
check_typename::<Struct<(), u8>>("Struct<(), u8>");
}

// Test that trait bounds are generated correctly

#[test]
#[allow(dead_code)]
fn generic_with_trait_bounds() {
#[derive(TypeName)]
struct Struct<C: std::fmt::Debug + Clone, D>
where
D: std::fmt::Display + Copy,
{
a: u32,
b: u128,
c: (C, D),
}
check_typename::<Struct<(), u8>>("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>("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>("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<()>>("Struct<()>");
}

// Test const generics

#[test]
#[allow(dead_code)]
fn const_generic_num() {
#[derive(TypeName)]
struct Struct<const N: usize> {
a: u32,
b: u128,
}
check_typename::<Struct<77>>("Struct<77>");
}

#[test]
#[allow(dead_code)]
fn const_generic_bool() {
#[derive(TypeName)]
struct Struct<const T: bool> {
a: u32,
b: u128,
}
check_typename::<Struct<true>>("Struct<true>");
}

#[test]
#[allow(dead_code)]
fn const_generic_char() {
#[derive(TypeName)]
struct Struct<const N: char> {
a: u32,
b: u128,
}
check_typename::<Struct<'a'>>("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<String, [[String; 3]; 2], Vec<Box<i128>>, &(&Enum<(u8, i8), 99>,), [u16], 33, false>,
>(
format!(
"Struct<\
alloc::string::String, \
[[alloc::string::String; 3]; 2], \
alloc::vec::Vec<alloc::boxed::Box<i128>>, \
&(&{mod_path}::Enum<(u8, i8), 99>,), \
[u16], \
33, \
false\
>")
.as_str(),
);
}
63 changes: 37 additions & 26 deletions rkyv_typename_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 }
});
Expand All @@ -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 {
Expand All @@ -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<F: FnMut(&str)>(mut f: F) {
fn build_type_name<TYPENAME__F: FnMut(&str)>(mut f: TYPENAME__F) {
#build_args
}
}
Expand Down

0 comments on commit 1cd147d

Please sign in to comment.