Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only put Debug-like bounds on type variables #371

Merged
merged 5 commits into from
Jul 19, 2024
Merged

Only put Debug-like bounds on type variables #371

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16713df
Only put Debug-like bounds on type variables
sornas Jun 18, 2024
6131444
Add more tests and support trait objects
JelteF Jul 5, 2024
0570291
Merge branch 'refs/heads/master' into debug-bounds-only-participating…
tyranron Jul 19, 2024
5424185
Corrections and improvements
tyranron Jul 19, 2024
022fdfb
Fix dat `sum` tests
tyranron Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 127 additions & 6 deletions impl/src/fmt/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,22 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result<TokenStream> {
.unwrap_or_default();
let ident = &input.ident;

let type_variables = input
.generics
.params
.iter()
.filter_map(|param| match param {
syn::GenericParam::Lifetime(_) => None,
syn::GenericParam::Type(ty) => Some(&ty.ident),
syn::GenericParam::Const(_) => None,
})
.collect();

let (bounds, body) = match &input.data {
syn::Data::Struct(s) => expand_struct(attrs, ident, s, &attr_name),
syn::Data::Enum(e) => expand_enum(attrs, e, &attr_name),
syn::Data::Struct(s) => {
expand_struct(attrs, ident, s, type_variables, &attr_name)
}
syn::Data::Enum(e) => expand_enum(attrs, e, type_variables, &attr_name),
syn::Data::Union(_) => {
return Err(syn::Error::new(
input.span(),
Expand Down Expand Up @@ -64,11 +77,13 @@ fn expand_struct(
attrs: ContainerAttributes,
ident: &Ident,
s: &syn::DataStruct,
type_variables: Vec<&syn::Ident>,
attr_name: &syn::Ident,
) -> syn::Result<(Vec<syn::WherePredicate>, TokenStream)> {
let s = Expansion {
attr: &attrs,
fields: &s.fields,
type_variables: &type_variables,
ident,
attr_name,
};
Expand Down Expand Up @@ -99,6 +114,7 @@ fn expand_struct(
fn expand_enum(
mut attrs: ContainerAttributes,
e: &syn::DataEnum,
type_variables: Vec<&syn::Ident>,
attr_name: &syn::Ident,
) -> syn::Result<(Vec<syn::WherePredicate>, TokenStream)> {
if let Some(enum_fmt) = attrs.fmt.as_ref() {
Expand Down Expand Up @@ -136,6 +152,7 @@ fn expand_enum(
let v = Expansion {
attr: &attrs,
fields: &variant.fields,
type_variables: &type_variables,
ident,
attr_name,
};
Expand Down Expand Up @@ -195,6 +212,9 @@ struct Expansion<'a> {
/// Struct or enum [`syn::Fields`].
fields: &'a syn::Fields,

/// Type variables in this struct or enum.
type_variables: &'a [&'a syn::Ident],

/// Name of the attributes, considered by this macro.
attr_name: &'a syn::Ident,
}
Expand Down Expand Up @@ -334,15 +354,26 @@ impl<'a> Expansion<'a> {
let mut out = self.attr.bounds.0.clone().into_iter().collect::<Vec<_>>();

if let Some(fmt) = self.attr.fmt.as_ref() {
out.extend(fmt.bounded_types(self.fields).map(|(ty, trait_name)| {
let trait_ident = format_ident!("{trait_name}");
out.extend(fmt.bounded_types(self.fields).filter_map(
|(ty, trait_name)| {
if !self.contains_type_variable(ty) {
return None;
}

parse_quote! { #ty: derive_more::core::fmt::#trait_ident }
}));
let trait_ident = format_ident!("{trait_name}");

Some(parse_quote! { #ty: derive_more::core::fmt::#trait_ident })
},
));
Ok(out)
} else {
self.fields.iter().try_fold(out, |mut out, field| {
let ty = &field.ty;

if !self.contains_type_variable(ty) {
return Ok(out);
}

match FieldAttribute::parse_attrs(&field.attrs, self.attr_name)?
.map(Spanning::into_inner)
{
Expand All @@ -362,4 +393,94 @@ impl<'a> Expansion<'a> {
})
}
}

fn contains_type_variable(&self, ty: &syn::Type) -> bool {
match ty {
syn::Type::Path(syn::TypePath { qself, path }) => {
if let Some(qself) = qself {
if self.contains_type_variable(&qself.ty) {
return true;
}
}

if let Some(ident) = path.get_ident() {
self.type_variables
.iter()
.any(|type_var| *type_var == ident)
} else {
path.segments.iter().any(|segment| {
self.type_variables
.iter()
.any(|type_var| *type_var == &segment.ident)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is outside my Rust knowledge, but I'm starting to think this is wrong. If the type is a path (like std::vec::Vec), then it cannot contain a type variable, correct? We only care if it is the only segment or itself a type variable (T in std::vec::Vec<T>) at some part of the path.

Copy link
Owner

@JelteF JelteF Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, correct. I think an easy way to test that would be:

#[derive(derive_more::Debug)]
struct Vec {
    next: std::vec::Vec<i32>,
}

|| match &segment.arguments {
syn::PathArguments::None => false,
syn::PathArguments::AngleBracketed(
syn::AngleBracketedGenericArguments {
args, ..
},
) => args.iter().any(|generic| match generic {
syn::GenericArgument::Type(ty)
| syn::GenericArgument::AssocType(
syn::AssocType { ty, .. },
) => self.contains_type_variable(ty),

syn::GenericArgument::Lifetime(_)
| syn::GenericArgument::Const(_)
| syn::GenericArgument::AssocConst(_)
| syn::GenericArgument::Constraint(_) => false,
_ => false,
}),
syn::PathArguments::Parenthesized(
syn::ParenthesizedGenericArguments {
inputs,
output,
..
},
) => {
inputs
.iter()
.any(|ty| self.contains_type_variable(ty))
|| match output {
syn::ReturnType::Default => false,
syn::ReturnType::Type(_, ty) => {
self.contains_type_variable(ty)
}
}
}
}
})
}
}

syn::Type::Array(syn::TypeArray { elem, .. })
| syn::Type::Group(syn::TypeGroup { elem, .. })
| syn::Type::Paren(syn::TypeParen { elem, .. })
| syn::Type::Ptr(syn::TypePtr { elem, .. })
| syn::Type::Reference(syn::TypeReference { elem, .. })
| syn::Type::Slice(syn::TypeSlice { elem, .. }) => {
self.contains_type_variable(elem)
}

syn::Type::BareFn(syn::TypeBareFn { inputs, output, .. }) => {
inputs
.iter()
.any(|arg| self.contains_type_variable(&arg.ty))
|| match output {
syn::ReturnType::Default => false,
syn::ReturnType::Type(_, ty) => self.contains_type_variable(ty),
}
}
syn::Type::Tuple(syn::TypeTuple { elems, .. }) => {
elems.iter().any(|ty| self.contains_type_variable(ty))
}

syn::Type::ImplTrait(_) => false,
syn::Type::Infer(_) => false,
syn::Type::Macro(_) => false,
syn::Type::Never(_) => false,
syn::Type::TraitObject(_) => false,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trait objects can contain type parameters:

trait MyTrait<T> {}

struct Test<T>
{
    t: Box<dyn MyTrait<T>>,
}

syn::Type::Verbatim(_) => false,
_ => false,
}
}
}
67 changes: 67 additions & 0 deletions tests/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1932,3 +1932,70 @@ mod complex_enum_syntax {
assert_eq!(format!("{:?}", Enum::A), "A");
}
}

// See: https://github.com/JelteF/derive_more/issues/363
mod type_variables {
#[cfg(not(feature = "std"))]
use alloc::{boxed::Box, format, vec, vec::Vec};

use derive_more::Debug;

#[derive(Debug)]
struct ItemStruct {
next: Option<Box<ItemStruct>>,
}

#[derive(Debug)]
struct ItemTuple(Option<Box<ItemTuple>>);

#[derive(Debug)]
#[debug("Item({_0:?})")]
struct ItemTupleContainerFmt(Option<Box<ItemTupleContainerFmt>>);

#[derive(Debug)]
enum ItemEnum {
Node { children: Vec<ItemEnum>, inner: i32 },
Leaf { inner: i32 },
}

#[test]
fn assert() {
assert_eq!(
format!(
"{:?}",
ItemStruct {
next: Some(Box::new(ItemStruct { next: None }))
}
),
"ItemStruct { next: Some(ItemStruct { next: None }) }"
);

assert_eq!(
format!("{:?}", ItemTuple(Some(Box::new(ItemTuple(None)))),),
"ItemTuple(Some(ItemTuple(None)))"
);

assert_eq!(
format!(
"{:?}",
ItemTupleContainerFmt(Some(Box::new(ItemTupleContainerFmt(None)))),
),
"Item(Some(Item(None)))"
);

let item = ItemEnum::Node {
children: vec![
ItemEnum::Node {
children: vec![],
inner: 0,
},
ItemEnum::Leaf { inner: 1 },
],
inner: 2,
};
assert_eq!(
format!("{item:?}"),
"Node { children: [Node { children: [], inner: 0 }, Leaf { inner: 1 }], inner: 2 }",
)
}
}