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.

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

Only put Display-like bounds on type variables #387

Merged
merged 10 commits into from
Jul 25, 2024
Merged

Conversation

JelteF
Copy link
Owner

@JelteF JelteF commented Jul 20, 2024

Only put Display-like bounds on type variables

Resolves #363
Requires #377
Related to #371

Synopsis

The problem is that the Display derive adds a bounds for all types that
are used in the format string. But this is not necessary for types that
don't contain a type variable. And adding those bounds can result in
errors like for the following code:

#[derive(Display, Debug)]
#[display("{inner:?}")]
#[display(bounds(T: Display))]
struct OptionalBox<T> {
    inner: Option<Box<T>>,
}

#[derive(Display, Debug)]
#[display("{next}")]
struct ItemStruct {
    next: OptionalBox<ItemStruct>,
}

That code would generate the following error:

error[E0275]: overflow evaluating the requirement `ItemStruct: derive_more::Display`

Solution

This makes sure we don't add unnecessary bounds for Display-like
derives. It does so in the same way as #371 did for the Debug derive: By
only adding bounds when the type contains a type variable.

Checklist

  • Tests are added/updated (if required)

Changelog is not updated because the 0.99 did not have this issue.

@JelteF JelteF force-pushed the fix-display-bounds branch 5 times, most recently from 01c60d2 to 5f602fe Compare July 20, 2024 11:46
@JelteF JelteF requested a review from tyranron July 20, 2024 11:57
@JelteF JelteF self-assigned this Jul 20, 2024
@JelteF JelteF added this to the 1.0.0 milestone Jul 20, 2024
@JelteF JelteF force-pushed the fix-display-bounds branch 2 times, most recently from b658620 to 4f92982 Compare July 24, 2024 22:11
Resolves #363

Related to #371

Requires #377

This makes sure we don't add unnecessary bounds for Display-like
derives. It does so in the same way as #371 did for the Debug derive: By
only adding bounds when the type contains a type variable.
# Conflicts:
#	impl/doc/display.md
#	impl/src/fmt/display.rs
#	tests/display.rs
Copy link
Collaborator

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

@JelteF this seems fine. The only change I've added is extracting the duplicate code of syn::Type/syn::Path traversal to the fmt module shared by both expansions.

@tyranron tyranron merged commit efbd8ed into master Jul 25, 2024
17 checks passed
@tyranron tyranron deleted the fix-display-bounds branch July 25, 2024 13:48
tyranron added a commit that referenced this pull request Aug 20, 2024
Related to #387

## Synopsis

After #387, the following snippet fails to compile:
```rust
#[derive(Debug)]
struct AssocType<I: Iterator> {
    iter: I,
    elem: Option<I::Item>,
}
```
This happens, because the implied bound `Option<I::Item>: Debug` is not
generated.

## Solution

Correct the `ContainsGenericsExt::contains_generics()` implementations
to consider associated types of the type parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'overflow evaluating the requirement' when deriving debug for item that contains itself
2 participants