Point out shadowed associated types on E0191 #117642
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
fixes #100109 |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Thanks for the contribution! This needs a UI test. And tests need to be blessed with Also, can you amend the PR title to have some sort of verb? "[Point out] overlapping associated types [when...?]" or something is more conventional title for a PR like this. Also, can you squash the commits into one? @rustbot author |
rustbot
added
S-waiting-on-author
compiler-errors
requested changes
Nov 6, 2023
| @@ -585,6 +586,24 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
| } | |||
| } | |||
|
|
|||
| let bound_names = trait_bounds | |||
There was a problem hiding this comment.
Can you explain what this code is doing? Some comments throuhout may be helpful.
| .iter() | ||
| .flat_map(|poly_trait_ref| { | ||
| poly_trait_ref.trait_ref.path.segments.iter().flat_map(|x| { | ||
| x.args.iter().flat_map(|args| { |
There was a problem hiding this comment.
Why are you looking at all of the segments in the trait ref? That shouldn't be necessary. Only the final path segment should be interesting here.
There was a problem hiding this comment.
Also, please don't iter() over an Option -- if let or let .. else should work here more effectively.
| let name = binding.ident.name; | ||
| let trait_def = poly_trait_ref.trait_ref.path.res.opt_def_id(); | ||
| let assoc_item = trait_def.and_then(|did| { | ||
| tcx.associated_items(did).filter_by_name_unhygienic(name).next() |
There was a problem hiding this comment.
We shouldn't be filtering unhygenically. You have an identifier in binding.ident.
TheLazyDutchman
changed the title
|
continues in #117661, because I didn't know renaming the branch would close the PR |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 6, 2023
…ssociated_types, r=petrochenkov
Added shadowed hint for overlapping associated types
Previously, when you tried to set an associated type that is shadowed by an associated type in a subtrait, like this:
```rust
trait A {
type X;
}
trait B: A {
type X; // note: this is legal
}
impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
fn clone(&self) -> Self {
todo!()
}
}
you got a confusing error message, that says nothing about the shadowing:
error[E0719]: the value of the associated type `X` (from 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` (from trait `A`) must be specified
--> test.rs:9:27
|
2 | type X;
| ------ `X` defined here
...
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
| ^^^^^^^^^^^ help: specify the associated type: `B<X=Y, X=Y, X = Type>`
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0191, E0719.
For more information about an error, try `rustc --explain E0191`.
```
Now instead, the error shows that the associated type is shadowed, and suggests renaming as a potential fix.
```rust
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 renaming this associated type
--> test.rs:2:5
|
2 | type X;
| ^^^^^^
help: consider renaming this associated type
--> test.rs:6:5
|
6 | 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 only emitted when the trait is local. This is true both for the supertrait as for the subtrait.
There might be cases where you can use the fully qualified path (for instance, in a where clause), but this PR currently does not deal with that.
fixes rust-lang#100109
(continues from rust-lang#117642, because I didn't know renaming the branch would close the PR)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when you tried to set an associated type that is shadowed by an associated type in a subtrait, like this:
you got a confusing error message, that says nothing about the shadowing:
Now instead, the error shows that the associated type is shadowed, and suggests renaming as a potential fix.
The rename help message is only emitted when the trait is local. This is true both for the supertrait as for the subtrait.
There might be cases where you can use the fully qualified path (for instance, in a where clause), but this PR currently does not deal with that.