E0599 suggestions and elision of generic argument if no canditate is found #84221
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
…d anywhere and sugetion of type with method when found.
|
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
ABouttefeux
changed the title
crlf0710
added
S-waiting-on-review
JohnCSimon
added
S-waiting-on-review
|
I change it to show |
This comment has been minimized.
This comment has been minimized.
ABouttefeux
force-pushed
the
generic-arg-elision
branch
from
3cbd7a6
to
6efa14b
Compare
estebank
reviewed
May 21, 2021
| // number of type to shows at most. | ||
| const LIMIT: usize = 3; | ||
| let mut note = format!("The {item_kind} was found for"); | ||
| if inherent_impls_candidate.len() > 1 { |
There was a problem hiding this comment.
I think it might be better to make a list of items instead of enumerating them in text, because that is easier to scan when reading the error.
There was a problem hiding this comment.
This might work better as well in the case of really long type names with lots of type parameters (that we see in the wild).
| inherent_impls_candidate.len() - LIMIT | ||
| ); | ||
| } | ||
| err.note(&format!("{}.", note)); |
There was a problem hiding this comment.
We don't capitalize or have a final period on sentences. We only break that convention if we have long text with multiple sentences, but it is heavily frown upon.
| LL | wrapper.method(); | ||
| | ^^^^^^ method not found in `Wrapper2<'_, bool, 3_usize>` | ||
| | | ||
| = note: The method was found for Wrapper2<'a, i8, C>, Wrapper2<'a, i32, C> and Wrapper2<'a, i32, C>. |
There was a problem hiding this comment.
Looking at this I am concerned about some trait bounds I've seen that generate huge types, but I don't know how we'd present these in a better way 🤷♂️
There was a problem hiding this comment.
The case where trait bounds are not met are manage by another part of the code, therefore we should not see long trait bounds. I have added a test for this case.
Comment on lines
+412
to
+414
| if let Some((path_string, _)) = ty_str.split_once('<') { | ||
| ty_str_reported = path_string.to_string(); | ||
| } |
There was a problem hiding this comment.
Ideally we'd just take the name ident of the type and use that. I think item_name.to_string() would work here.
String operations are brittle and can break as things change in the codebase.
There was a problem hiding this comment.
item_name is the method we are looking to call. I did not find a way to print the type without the generic parameter otherwise. Do you know a way to do it ?
There was a problem hiding this comment.
What is the type of item_name from which you get span from?
I think that might have a string representation, and if not a field that is only the name with type Ident, which has a string representation of only the identifier.
There was a problem hiding this comment.
Yes it is an Ident, but this does not represent the type. This is the indent of the method
For instance it is used here:
https://github.com/ABouttefeux/rust/blob/120691c590c4309fda31994931b9a561b4249c33/compiler/rustc_typeck/src/check/method/suggest.rs#L422-L431
Co-authored-by: Esteban Kuber <estebank@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
ABouttefeux
force-pushed
the
generic-arg-elision
branch
from
8401384
to
cbeae1f
Compare
This comment has been minimized.
This comment has been minimized.
ABouttefeux
force-pushed
the
generic-arg-elision
branch
from
cbeae1f
to
120691c
Compare
estebank
approved these changes
May 24, 2021
| --> $DIR/method-not-found-generic-arg-elision.rs:82:23 | ||
| | | ||
| LL | struct Point<T> { | ||
| | --------------- method `distance` not found for this |
There was a problem hiding this comment.
Looking at this with fresh eyes in context, it might make sense to expand this label to be method distance not found for Point when T is i32, but it is found when T is f64, but maybe that's too verbose.
There was a problem hiding this comment.
I am not sure how to get the indent for T. Also it might become very long with multiple generic parameters.
Comment on lines
36
to
39
| = note: the method was found for Wrapper<i8> | ||
| = note: the method was found for Wrapper<i16> | ||
| = note: the method was found for Wrapper<i32> | ||
| = note: the method was found for 3 more types |
There was a problem hiding this comment.
Ideally this would be a single note:
= note: the method `method` was found for
- `Wrapper<i8>`
- `Wrapper<i16>`
- `Wrapper<i32>`
- `Wrapper<u8>`
and 2 more types
| @@ -3,6 +3,8 @@ error[E0599]: no function or associated item named `new_undirected` found for st | |||
| | | |||
| LL | let ug = Graph::<i32, i32>::new_undirected(); | |||
| | ^^^^^^^^^^^^^^ function or associated item not found in `issue_30123_aux::Graph<i32, i32>` | |||
| | | |||
| = note: the function or associated item was found for issue_30123_aux::Graph<N, E, Undirected> | |||
There was a problem hiding this comment.
This would ideally talk about Graph<i32, i32, Undirected>, potentially mentioning the Ty default of Directed. No need to tackle this now.
| .map(|impl_item| self.tcx.at(span).type_of(*impl_item)) | ||
| .collect::<Vec<_>>(); | ||
| // number of type to shows at most. | ||
| let limit = if type_candidates.len() == 4 { 4 } else { 3 }; |
There was a problem hiding this comment.
Suggested change
| let limit = if type_candidates.len() == 4 { 4 } else { 3 }; | |
| let limit = if type_candidates.len() == 5 { 5 } else { 4 }; |
Comment on lines
526
to
534
| for ty in type_candidates.iter().take(limit) { | ||
| err.note(&format!("the {item_kind} was found for {}", ty)); | ||
| } | ||
| if type_candidates.len() > limit { | ||
| err.note(&format!( | ||
| "the {item_kind} was found for {} more types", | ||
| type_candidates.len() - limit | ||
| )); | ||
| } |
There was a problem hiding this comment.
Instead of emitting a note on each iteration, construct a single string to be emitted.
|
@bors r+ |
|
📌 Commit 5d8e6ea has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 5d8e6ea with merge 1c625170734846ea91039d65a49508b01f0fa69e... |
|
A job failed! Check out the build log: (web) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
May 26, 2021
…estebank E0599 suggestions and elision of generic argument if no canditate is found fixes rust-lang#81576 changes: In error E0599 (method not found) generic argument are eluded if the method was not found anywhere. If the method was found in another inherent implementation suggest that it was found elsewhere. Example ```rust struct Wrapper<T>(T); struct Wrapper2<T> { x: T, } impl Wrapper2<i8> { fn method(&self) {} } fn main() { let wrapper = Wrapper(i32); wrapper.method(); let wrapper2 = Wrapper2{x: i32}; wrapper2.method(); } ``` ``` Error[E0599]: no method named `method` found for struct `Wrapper<_>` in the current scope .... error[E0599]: no method named `method` found for struct `Wrapper2<i32>` in the current scope ... = note: The method was found for Wrapper2<i8>. ``` I am not very happy with the ```no method named `test` found for struct `Vec<_, _>` in the current scope```. I think it might be better to show only one generic argument `Vec<_>` if there is a default one. But I haven't yet found a way to do that,
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
May 27, 2021
…estebank E0599 suggestions and elision of generic argument if no canditate is found fixes rust-lang#81576 changes: In error E0599 (method not found) generic argument are eluded if the method was not found anywhere. If the method was found in another inherent implementation suggest that it was found elsewhere. Example ```rust struct Wrapper<T>(T); struct Wrapper2<T> { x: T, } impl Wrapper2<i8> { fn method(&self) {} } fn main() { let wrapper = Wrapper(i32); wrapper.method(); let wrapper2 = Wrapper2{x: i32}; wrapper2.method(); } ``` ``` Error[E0599]: no method named `method` found for struct `Wrapper<_>` in the current scope .... error[E0599]: no method named `method` found for struct `Wrapper2<i32>` in the current scope ... = note: The method was found for Wrapper2<i8>. ``` I am not very happy with the ```no method named `test` found for struct `Vec<_, _>` in the current scope```. I think it might be better to show only one generic argument `Vec<_>` if there is a default one. But I haven't yet found a way to do that,
This was referenced May 27, 2021
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 27, 2021
Rollup of 8 pull requests Successful merges: - rust-lang#84221 (E0599 suggestions and elision of generic argument if no canditate is found) - rust-lang#84701 (stabilize member constraints) - rust-lang#85564 ( readd capture disjoint fields gate) - rust-lang#85583 (Get rid of PreviousDepGraph.) - rust-lang#85649 (Update cc) - rust-lang#85689 (Remove Iterator #[rustc_on_unimplemented]s that no longer apply.) - rust-lang#85719 (Add inline attr to CString::into_inner so it can optimize out NonNull checks) - rust-lang#85725 (Remove unneeded workaround) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
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.
fixes #81576
changes: In error E0599 (method not found) generic argument are eluded if the method was not found anywhere. If the method was found in another inherent implementation suggest that it was found elsewhere.
Example
I am not very happy with the
no method named `test` found for struct `Vec<_, _>` in the current scope. I think it might be better to show only one generic argumentVec<_>if there is a default one. But I haven't yet found a way to do that,