Repository: JelteF/derive_more (rust)

Issue:
'overflow evaluating the requirement' when deriving debug for item that contains itself
the following code fails to compile

```
#[derive(derive_more::Debug)]
struct Message {
    inner: Box<Message>,
}

fn main() {}
```

```
error[E0275]: overflow evaluating the requirement `Message: Debug`
 --> src/main.rs:1:10
  |
1 | #[derive(derive_more::Debug)]
  |          ^^^^^^^^^^^^^^^^^^
  |
  = note: required for `Box<Message>` to implement `Debug`
  = help: see issue #48214
  = note: this error originates in the derive macro `derive_more::Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
```

the same error is reported with stuff like `Vec<Message>` and with enum instead of struct.

```
$ cargo --version --verbose
cargo 1.78.0 (54d8815d0 2024-03-26)
release: 1.78.0
commit-hash: 54d8815d04fa3816edc207bbc4dd36bf18014dbc
commit-date: 2024-03-26
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Arch Linux Rolling Release [64-bit]
$ cat Cargo.toml
[package]
name = "derive-more-debug"
version = "0.1.0"
edition = "2021"

[dependencies]
derive_more = { version = "1.0.0-beta.6", features = ["debug"] }
```

Pull request:
Only put Debug-like bounds on type variables
Resolves #363

Well, at least it's a suggestion for a resolution :p

## Synopsis

The problem, as reported in the issue, is that code like the following

```rust
#[derive(derive_more::Debug)]
struct Item {
    next: Option<Box<Item>>,
}
```

expands into something like

```rust
impl std::fmt::Debug for Item where Item: Debug { /* ... */ }
```

which does not compile. This PR changes the Debug derive so it does not emit those bounds.

## Solution

My understanding of the current code is that we iterate over all fields of the struct/enum and add either a specific
format bound (e.g. `: fmt::Binary`), a default `: fmt::Debug` bound or skip it if either it is marked
as `#[debug(skip)]` or the entire container has a format attribute. 

The suggested solution in the issue (if I understood it correctly) was to only add bounds if the type is a type
variable, since rustc already knows if a concrete type is, say, `: fmt::Debug`. So, instead of adding the bound for
every type, we first check that the type contains one of the container's type variables. Since types can be nested, it
is an unfortunately long recursive function handling the different types of types. This part of Rust syntax is probably
not going to change, so perhaps it is feasible to shorten some of the branches into `_ => false`.

One drawback of this implementation is that we iterate over the list of type variables every time we find a "leaf type".
I chose `Vec` over `HashSet` because in my experience there are only a handful of type variables per container.

## Status

I should add more tests, probably? I've at least verified the case mentioned above (for tuple structs, data structs and
enums). I must admit I haven't really looked in the documentation, but I should probably edit that too.

## Checklist

- [x] ~~Documentation is updated~~ (not required)
- [x] Tests are added/updated (if required)
- [x] ~~[CHANGELOG entry][l:1] is added~~ (not required)

[l:1]: /CHANGELOG.md


Patch:
diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs
index 9b4e99f..524f021 100644
--- a/impl/src/fmt/debug.rs
+++ b/impl/src/fmt/debug.rs
@@ -24,9 +24,21 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result<TokenStream> {
         .unwrap_or_default();
     let ident = &input.ident;
 
+    let type_params: Vec<_> = input
+        .generics
+        .params
+        .iter()
+        .filter_map(|p| match p {
+            syn::GenericParam::Type(t) => Some(&t.ident),
+            syn::GenericParam::Const(..) | syn::GenericParam::Lifetime(..) => 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_params, &attr_name)
+        }
+        syn::Data::Enum(e) => expand_enum(attrs, e, &type_params, &attr_name),
         syn::Data::Union(_) => {
             return Err(syn::Error::new(
                 input.span(),
@@ -64,11 +76,13 @@ fn expand_struct(
     attrs: ContainerAttributes,
     ident: &Ident,
     s: &syn::DataStruct,
+    type_params: &[&syn::Ident],
     attr_name: &syn::Ident,
 ) -> syn::Result<(Vec<syn::WherePredicate>, TokenStream)> {
     let s = Expansion {
         attr: &attrs,
         fields: &s.fields,
+        type_params,
         ident,
         attr_name,
     };
@@ -99,6 +113,7 @@ fn expand_struct(
 fn expand_enum(
     mut attrs: ContainerAttributes,
     e: &syn::DataEnum,
+    type_params: &[&syn::Ident],
     attr_name: &syn::Ident,
 ) -> syn::Result<(Vec<syn::WherePredicate>, TokenStream)> {
     if let Some(enum_fmt) = attrs.fmt.as_ref() {
@@ -136,6 +151,7 @@ fn expand_enum(
             let v = Expansion {
                 attr: &attrs,
                 fields: &variant.fields,
+                type_params,
                 ident,
                 attr_name,
             };
@@ -195,6 +211,9 @@ struct Expansion<'a> {
     /// Struct or enum [`syn::Fields`].
     fields: &'a syn::Fields,
 
+    /// Type parameters in this struct or enum.
+    type_params: &'a [&'a syn::Ident],
+
     /// Name of the attributes, considered by this macro.
     attr_name: &'a syn::Ident,
 }
@@ -334,15 +353,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_generic_param(ty) {
+                        return None;
+                    }
+
+                    let trait_ident = format_ident!("{trait_name}");
 
-                parse_quote! { #ty: derive_more::core::fmt::#trait_ident }
-            }));
+                    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_generic_param(ty) {
+                    return Ok(out);
+                }
+
                 match FieldAttribute::parse_attrs(&field.attrs, self.attr_name)?
                     .map(Spanning::into_inner)
                 {
@@ -362,4 +392,105 @@ impl<'a> Expansion<'a> {
             })
         }
     }
+
+    /// Checks whether the provided [`syn::Path`] contains any of these [`Expansion::type_params`].
+    fn path_contains_generic_param(&self, path: &syn::Path) -> bool {
+        path.segments
+            .iter()
+            .any(|segment| 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_generic_param(ty)
+                    }
+
+                    syn::GenericArgument::Lifetime(_)
+                    | syn::GenericArgument::Const(_)
+                    | syn::GenericArgument::AssocConst(_)
+                    | syn::GenericArgument::Constraint(_) => false,
+                    _ => unimplemented!(
+                        "syntax is not supported by `derive_more`, please report a bug",
+                    ),
+                }),
+                syn::PathArguments::Parenthesized(
+                    syn::ParenthesizedGenericArguments { inputs, output, .. },
+                ) => {
+                    inputs.iter().any(|ty| self.contains_generic_param(ty))
+                        || match output {
+                            syn::ReturnType::Default => false,
+                            syn::ReturnType::Type(_, ty) => {
+                                self.contains_generic_param(ty)
+                            }
+                        }
+                }
+            })
+    }
+
+    /// Checks whether the provided [`syn::Type`] contains any of these [`Expansion::type_params`].
+    fn contains_generic_param(&self, ty: &syn::Type) -> bool {
+        if self.type_params.is_empty() {
+            return false;
+        }
+        match ty {
+            syn::Type::Path(syn::TypePath { qself, path }) => {
+                if let Some(qself) = qself {
+                    if self.contains_generic_param(&qself.ty) {
+                        return true;
+                    }
+                }
+
+                if let Some(ident) = path.get_ident() {
+                    self.type_params.iter().any(|param| *param == ident)
+                } else {
+                    self.path_contains_generic_param(path)
+                }
+            }
+
+            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_generic_param(elem)
+            }
+
+            syn::Type::BareFn(syn::TypeBareFn { inputs, output, .. }) => {
+                inputs
+                    .iter()
+                    .any(|arg| self.contains_generic_param(&arg.ty))
+                    || match output {
+                        syn::ReturnType::Default => false,
+                        syn::ReturnType::Type(_, ty) => self.contains_generic_param(ty),
+                    }
+            }
+            syn::Type::Tuple(syn::TypeTuple { elems, .. }) => {
+                elems.iter().any(|ty| self.contains_generic_param(ty))
+            }
+
+            syn::Type::ImplTrait(_) => false,
+            syn::Type::Infer(_) => false,
+            syn::Type::Macro(_) => false,
+            syn::Type::Never(_) => false,
+            syn::Type::TraitObject(syn::TypeTraitObject { bounds, .. }) => {
+                bounds.iter().any(|bound| match bound {
+                    syn::TypeParamBound::Trait(syn::TraitBound { path, .. }) => {
+                        self.path_contains_generic_param(path)
+                    }
+                    syn::TypeParamBound::Lifetime(_) => false,
+                    syn::TypeParamBound::Verbatim(_) => false,
+                    _ => unimplemented!(
+                        "syntax is not supported by `derive_more`, please report a bug",
+                    ),
+                })
+            }
+            syn::Type::Verbatim(_) => false,
+            _ => unimplemented!(
+                "syntax is not supported by `derive_more`, please report a bug",
+            ),
+        }
+    }
 }


Test patch:
diff --git a/tests/debug.rs b/tests/debug.rs
index bc6b613..0c4b180 100644
--- a/tests/debug.rs
+++ b/tests/debug.rs
@@ -1932,3 +1932,141 @@ mod complex_enum_syntax {
         assert_eq!(format!("{:?}", Enum::A), "A");
     }
 }
+
+// See: https://github.com/JelteF/derive_more/issues/363
+mod type_variables {
+    mod our_alloc {
+        #[cfg(not(feature = "std"))]
+        pub use alloc::{boxed::Box, format, vec, vec::Vec};
+        #[cfg(feature = "std")]
+        pub use std::{boxed::Box, format, vec, vec::Vec};
+    }
+
+    use our_alloc::{format, vec, Box, 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 },
+    }
+
+    #[derive(Debug)]
+    struct VecMeansDifferent<Vec> {
+        next: our_alloc::Vec<i32>,
+        real: Vec,
+    }
+
+    #[derive(Debug)]
+    struct Array<T> {
+        #[debug("{t}")]
+        t: [T; 10],
+    }
+
+    mod parens {
+        #![allow(unused_parens)] // test that type is found even in parentheses
+
+        use derive_more::Debug;
+
+        #[derive(Debug)]
+        struct Paren<T> {
+            t: (T),
+        }
+    }
+
+    #[derive(Debug)]
+    struct ParenthesizedGenericArgumentsInput<T> {
+        t: dyn Fn(T) -> i32,
+    }
+
+    #[derive(Debug)]
+    struct ParenthesizedGenericArgumentsOutput<T> {
+        t: dyn Fn(i32) -> T,
+    }
+
+    #[derive(Debug)]
+    struct Ptr<T> {
+        t: *const T,
+    }
+
+    #[derive(Debug)]
+    struct Reference<'a, T> {
+        t: &'a T,
+    }
+
+    #[derive(Debug)]
+    struct Slice<'a, T> {
+        t: &'a [T],
+    }
+
+    #[derive(Debug)]
+    struct BareFn<T> {
+        t: Box<fn(T) -> T>,
+    }
+
+    #[derive(Debug)]
+    struct Tuple<T> {
+        t: Box<(T, T)>,
+    }
+
+    trait MyTrait<T> {}
+
+    #[derive(Debug)]
+    struct TraitObject<T> {
+        t: Box<dyn MyTrait<T>>,
+    }
+
+    #[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 }",
+        )
+    }
+}
diff --git a/tests/sum.rs b/tests/sum.rs
index a273bf8..503d6b4 100644
--- a/tests/sum.rs
+++ b/tests/sum.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(not(feature = "std"), no_std)]
+#![allow(dead_code)] // some code is tested for type checking only
 
 use derive_more::Sum;