Suggest missing trait bounds when a method exists but the bounds aren't satisfied #26435
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| .chain(ref_obligations.iter()) { | ||
| if !selcx.evaluate_obligation(o) { | ||
| all_true = false; | ||
| // FIXME: is this the only case we care about? |
There was a problem hiding this comment.
Predicate::Trait is the most common case and the case that I've ran into in my code. I'm not completely familiar with the other Predicate variants, so I'm not sure if they should be considered, or at least considered in this PR.
There was a problem hiding this comment.
I would say just limit it to Predicate::Trait. The others kinds are typically "evaluated" to "true" anyway, and we can always extend it. (So feel free to remove the FIXME.)
|
☔ The latest upstream changes (presumably #26351) made this pull request unmergeable. Please resolve the merge conflicts. |
| if !predicates.is_empty() { | ||
| let bound_list = predicates.iter() | ||
| .map(|p| format!("`{} : {}`", | ||
| p.substs.self_ty().unwrap(), |
There was a problem hiding this comment.
Is this unwrap ok?
There was a problem hiding this comment.
this unwrap is ok, but it's better still to call p.self_ty(), so that the unwrap is not required.
the reason it is ok is that all trait-ref substitutions should always have a self-type. Calling self_ty on the TraitRef leverages this invariant, indicating that if a failure occurs, it's because the trait-ref was malformed, not because your code is at fault.
|
This will be excellent! |
| // Did not find an applicable method, but we did find various | ||
| // static methods that may apply, as well as a list of | ||
| // not-in-scope traits which may work. | ||
| NoMatch(Vec<CandidateSource>, Vec<ast::DefId>, probe::Mode), | ||
| NoMatch(Vec<CandidateSource>, Vec<TraitRef<'tcx>>, Vec<ast::DefId>, probe::Mode), |
There was a problem hiding this comment.
Nit: it might be worth changing this to a struct-like enum variant, or creating a separate struct like NoMatch(NoMatchData).
|
@gsingh93 this looks great, thanks for doing it! r+ with nits fixed, please ping me on IRC (or others with r+ rights...) once you've done that so I can instruct bors to merge. |
|
@bors r+ |
|
📌 Commit a006a82 has been approved by |
|
@gsingh93 nice, thanks! |
When a method exists in an impl but can not be used due to missing trait bounds for the type parameters, we should inform the user which trait bounds are missing.
For example, this code
```
// Note this is missing a Debug impl
struct Foo;
fn main() {
let a: Result<(), Foo> = Ok(());
a.unwrap()
}
```
Now gives the following error:
```
/home/gulshan/tmp/tmp.rs:6:7: 6:15 error: no method named `unwrap` found for type `core::result::Result<(), Foo>` in the current scope
/home/gulshan/tmp/tmp.rs:6 a.unwrap()
^~~~~~~~
/home/gulshan/tmp/tmp.rs:6:7: 6:15 note: The method `unwrap` exists but the following trait bounds were not satisfied: `Foo : core::fmt::Debug`
error: aborting due to previous error
```
Fixes #20941.
When a method exists in an impl but can not be used due to missing trait bounds for the type parameters, we should inform the user which trait bounds are missing.
For example, this code
Now gives the following error:
Fixes #20941.