Account for type parameters in bound suggestion #134937
Conversation
When encountering a missing bound that involves a type parameter, on associated functions we look at the generics to see if the type parameter is present. If so, we suggest the bound on the associated function instead of on the impl/trait. At the impl/trait doesn't have another type parameter of that name, then we don't suggest the bound at that item level.
```
error[E0277]: the trait bound `B: From<T>` is not satisfied
--> $DIR/suggest-restriction-involving-type-param.rs:20:33
|
LL | B { x: v.iter().map(|e| B::from(e.clone()).x).collect::<Vec<String>>().join(" ") }
| ^ the trait `From<T>` is not implemented for `B`
|
help: consider further restricting the type
|
LL | pub fn from_many<T: Into<B> + Clone>(v: Vec<T>) -> Self where B: From<T> {
| ++++++++++++++++
```
Fix rust-lang#104089.
|
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
rustbot
added
S-waiting-on-review
| let mut v = ParamFinder { params: vec![] }; | ||
| // Get all type parameters from the predicate. If the predicate references a type parameter | ||
| // at all, then we can only suggestion a bound on an item that has access to that parameter. | ||
| v.visit_predicate(UpcastFrom::upcast_from(trait_pred, self.tcx)); |
There was a problem hiding this comment.
Also, what do you mean? We should never be suggesting a bound involving a parameter that we don't have "access to" --we shouldn't be checking predicates involving parameters that aren't in scope anyways?
There was a problem hiding this comment.
We currently have a fallback where regardless of what the predicate that failed, we end up suggesting a bound (with weasel wording that there might be a better way). This is in place because there are some complex (if uncommon) bounds that are still useful to have that were given only by that. But in #104089, we had a report of that fallback doing the wrong thing (obvious in retrospect). The predicate might be something like Self: From<T>, where the T is a param only in the associated item and not at the trait/impl level. That's what this additional check guards against: If any part of the predicate references one of the generics of that item, we don't skip suggesting the bound at that level because it would be invalid in the parent item.
| let mut v = ParamFinder { params: vec![] }; | ||
| // Get all type parameters from the predicate. If the predicate references a type parameter | ||
| // at all, then we can only suggestion a bound on an item that has access to that parameter. | ||
| v.visit_predicate(UpcastFrom::upcast_from(trait_pred, self.tcx)); |
There was a problem hiding this comment.
Also, what do you mean? We should never be suggesting a bound involving a parameter that we don't have "access to" --we shouldn't be checking predicates involving parameters that aren't in scope anyways?
| }) if !param_ty | ||
| && (generics.params.iter().any(|param| { | ||
| v.params.iter().any(|p| p.name == param.name.ident().name) | ||
| }) || v.params.is_empty()) => | ||
| { |
There was a problem hiding this comment.
Don't you want this?
| }) if !param_ty | |
| && (generics.params.iter().any(|param| { | |
| v.params.iter().any(|p| p.name == param.name.ident().name) | |
| }) || v.params.is_empty()) => | |
| { | |
| }) if !param_ty | |
| && v.params.iter().all(|param| { | |
| generics.params.iter().any(|p| p.name == param.name.ident().name) | |
| }) => | |
| { |
There was a problem hiding this comment.
I'm confused by the logic: what's a suggestion you're trying to avoid emitting by adding this condition?
When encountering a missing bound that involves a type parameter, on associated functions we look at the generics to see if the type parameter is present. If so, we suggest the bound on the associated function instead of on the impl/trait. At the impl/trait doesn't have another type parameter of that name, then we don't suggest the bound at that item level.
Fix #104089.