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

Suggest blanket impls when implementing a trait for a trait #96076

Closed
withoutboats opened this issue Apr 15, 2022 · 2 comments · Fixed by #97488
Closed

Suggest blanket impls when implementing a trait for a trait #96076

withoutboats opened this issue Apr 15, 2022 · 2 comments · Fixed by #97488
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@withoutboats
Copy link
Contributor

withoutboats commented Apr 15, 2022

One of the big problems with the 2015-style trait object syntax (no dyn) was that users often write things like impl Iterator for MyTrait { } when what they really meant was impl<T: MyTrait> Iterator for T { }. Back then, the error message would more often than not be some inscrutable message about Sized, which would send them on a wild goose chase of adding Sized bounds everywhere they could think of, leading to even more inscrutable messages.

Now, the error message is clearer: it tells users they need to add dyn for a trait object type. However, I find in many cases (most cases?) this is not what users actually want - what they actually want is the blanket impl. It would be helpful to have a note suggesting the correct syntax for a blanket impl, or better yet just presenting both correct syntaxes (impl for trait object and blanket impl) on equal footing and making it clear the difference between them.

@withoutboats withoutboats added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 15, 2022
@compiler-errors compiler-errors added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Apr 15, 2022
@vincenzopalazzo
Copy link
Member

@rustbot claim

@vincenzopalazzo
Copy link
Member

I work to reproduce this issue and start to understand what type of check we need to do, and in particular the work that we need to do is the following one:

If the traits are local traits we can perform the suggestion of the blanket impls, for example in the following code

trait Foo {}

trait Bar {}

impl Foo for Bar {}
//~^ should suggest `impl<T: Bar> Foo for T {}`

fn main() {}

But we need to skip the following suggestion in case we are using not local traits, like the following code here

use std::fmt;

trait MyTrait { }

impl<T: MyTrait> fmt::Display for T {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        todo!();
    }
}

fn main() {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants