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

Confusing suggestion when E0719 and E0191 occur at the same time. #100109

Closed
jendrikw opened this issue Aug 3, 2022 · 13 comments · Fixed by #117661
Closed

Confusing suggestion when E0719 and E0191 occur at the same time. #100109

jendrikw opened this issue Aug 3, 2022 · 13 comments · Fixed by #117661
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jendrikw
Copy link
Contributor

jendrikw commented Aug 3, 2022

Given the following code: play

trait A {
    type X;
}

trait B: A {
    type X; // note: this is legal
}

impl<X> Clone for Box<dyn B<X=X, X=X>> {
    fn clone(&self) -> Self {
        todo!()
    }
}

The current output is:

error[E0719]: the value of the associated type `X` (from trait `B`) is already specified
 --> src/lib.rs:9:34
  |
9 | impl<X> Clone for Box<dyn B<X=X, X=X>> {
  |                             ---  ^^^ re-bound here
  |                             |
  |                             `X` bound here first

error[E0191] the value of the associated type `X` (from trait `A`) must be specified
 --> src/lib.rs:9:27
  |
2 |     type X;
  |     ------- `X` defined here
...
9 | impl<X> Clone for Box<dyn B<X=X, X=X>> {
  |                           ^^^^^^^^^^^ help: specify the associated type: `B<X=X, X=X, X = Type>`

B<X=X, X=X, X = Type> is wrong, specifying another X=Type isn't going to fix the problem.

Writing just dyn B has a suggestion consider introducing a new type parameter, adding `where` constraints using the fully-qualified path to the associated types, which is good, but the concrete syntax would be nice.

@jendrikw jendrikw 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 Aug 3, 2022
@compiler-errors
Copy link
Member

We probably should modify E0191 to hint that an supertrait's associated type is shadowed. I think E0719 here is fine to keep as-is, imo.

@compiler-errors compiler-errors added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Aug 3, 2022
@jendrikw
Copy link
Contributor Author

jendrikw commented Aug 3, 2022

I agree the message for E0719 is good as-is.

@Abhicodes-crypto
Copy link

Abhicodes-crypto commented Dec 5, 2022

Is this issue fixed ? If not can I take this up, please.
Also how exactly should E0191 be modified. Imo, I don't think E0191 should pop up in this case as E0719 conveys the message. @compiler-errors

@obeis obeis removed their assignment Dec 6, 2022
@Abhicodes-crypto
Copy link

@rustbot claim

@LittleFall
Copy link
Contributor

@rustbot claim

@LittleFall
Copy link
Contributor

Ask for help:
How do I specify the association type of supertrait with the same name? In other words, is there an easy way to modify the code in the issue to make it pass compilation?

Here's my clumsy way of creating a new trait.

trait A {
    type X;
}

trait B: A {
    type X; // note: this is legal
}

trait BP<P, Q>: A<X = P> + B<X = Q> {}

impl<T> Clone for Box<dyn BP<T, T>> {
    fn clone(&self) -> Self {
        todo!()
    }
}

fn main() {
    let v: Box<dyn BP<(), ()>>;
}

@TheLazyDutchman
Copy link
Contributor

Is this issue still open?

If so I would like to take a crack at it.

@fmease
Copy link
Member

fmease commented Nov 4, 2023

If you can still reproduce it, then yes indeed.
@rustbot assign @TheLazyDutchman

@rustbot
Copy link
Collaborator

rustbot commented Nov 4, 2023

Error: Parsing assign command in comment failed: ...'zyDutchman' | error: user should start with @ at >| ''...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@TheLazyDutchman
Copy link
Contributor

I have now added a hint about the shadowing:

error[E0719]: the value of the associated type `X` in trait `B` is already specified
 --> test.rs:9:34
  |
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                             ---  ^^^ re-bound here
  |                             |
  |                             `X` bound here first

error[E0191]: the value of the associated type `X` in `A` must be specified
 --> test.rs:9:27
  |
2 |     type X;
  |     ------ `A::X` defined here
...
6 |     type X; // note: this is legal
  |     ------ `A::X` shadowed here
...
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                           ^^^^^^^^^^^ associated type `X` must be specified
  |
  = help: consider introducing a new type parameter, adding `where` constraints 
using the fully-qualified path to the associated types

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0191, E0719.
For more information about an error, try `rustc --explain E0191`.

But I am not quite sure about what to do about the actual help message.

  • Do we want to suggest renaming A::X to something else
  • Or do we want to suggest the work-around by @LittleFall

@compiler-errors
Copy link
Member

If the trait is local, then we could suggest renaming it. It's not required to be super specific here--we could just note the conflict to make the user aware of it, for example.

@TheLazyDutchman
Copy link
Contributor

I have updated the help message so that this case:

trait A {
    type X;
    type Z;
}

trait B: A {
    type X; // note: this is legal
    type Y;
}

impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
    fn clone(&self) -> Self {
        todo!()
    }
}

fn main() {
}

gives this error message:

error[E0719]: the value of the associated type `X` in trait `B` is already specified
  --> test.rs:11:34
   |
11 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
   |                             ---  ^^^ re-bound here
   |                             |
   |                             `X` bound here first

error[E0191]: the value of the associated types `X` and `Z` in `A`, `Y` in `B` must be specified
  --> test.rs:11:27
   |
2  |     type X;
   |     ------ `A::X` defined here
3  |     type Z;
   |     ------ `Z` defined here
...
7  |     type X; // note: this is legal
   |     ------ `A::X` shadowed here
8  |     type Y;
   |     ------ `Y` defined here
...
11 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
   |                           ^^^^^^^^^^^ associated types `X`, `Z`, `Y` must be specified
   |
help: consider renaming this associated type
  --> test.rs:2:5
   |
2  |     type X;
   |     ^^^^^^
help: consider renaming this associated type
  --> test.rs:7:5
   |
7  |     type X; // note: this is legal
   |     ^^^^^^

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0191, E0719.
For more information about an error, try `rustc --explain E0191`.

The rename help message is emitted for both the super trait and the subtrait, if they are local.

I think I am ready to make a merge request, or does somebody have something else to add?

@fmease
Copy link
Member

fmease commented Nov 6, 2023

I recommend you to just open a PR. Any further discussions can happen over there :)

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 E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. 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.

8 participants