Suggest how to fix with unconstrained type parameters #134270
Draft
+519
−1
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
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kei519
changed the title
|
☔ The latest upstream changes (presumably #135057) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #135151) made this pull request unmergeable. Please resolve the merge conflicts. |
cjgillot
reviewed
Jan 8, 2025
Comment on lines
251
to
257
| let node = tcx.hir().get_if_local(impl_def_id.into()).expect("cannot get `Node`"); | ||
| let hir_impl = if let rustc_hir::Node::Item(item) = node { | ||
| if let rustc_hir::ItemKind::Impl(imp) = item.kind { Some(imp) } else { None } | ||
| } else { | ||
| None | ||
| } | ||
| .expect("cannot take `Impl` in a impl block"); |
There was a problem hiding this comment.
Suggested change
| let node = tcx.hir().get_if_local(impl_def_id.into()).expect("cannot get `Node`"); | |
| let hir_impl = if let rustc_hir::Node::Item(item) = node { | |
| if let rustc_hir::ItemKind::Impl(imp) = item.kind { Some(imp) } else { None } | |
| } else { | |
| None | |
| } | |
| .expect("cannot take `Impl` in a impl block"); | |
| let node = tcx.hir_node_by_def_id(impl_def_id); | |
| let hir_impl = node.expect_item().expect_impl(); |
| .iter() | ||
| .enumerate() | ||
| .find(|(_, par)| par.name.ident().name == param.name) | ||
| .unwrap(); |
There was a problem hiding this comment.
Could you avoid unwrapping here? We may get very weird stuff in diagnostic code.
| .params | ||
| .iter() | ||
| .enumerate() | ||
| .find(|(_, par)| par.name.ident().name == param.name) |
There was a problem hiding this comment.
This identifies by parameter name, which is ambiguous due to hygiene. This should perhaps identify using par.def_id and param.def_id.
Comment on lines
277
to
304
| if let Some(prev) = hir_impl.generics.params[..index].iter().rfind(is_impl_generic) { | ||
| let mut span = prev.span; | ||
|
|
||
| // Consider the span of the bounds with the generic parameter when there is. | ||
| if let Some(predicate) = hir_impl.generics.predicates.iter().find(|pred| { | ||
| if let WherePredicateKind::BoundPredicate(WhereBoundPredicate { | ||
| origin: PredicateOrigin::GenericParam, | ||
| bounded_ty, | ||
| .. | ||
| }) = pred.kind | ||
| { | ||
| bounded_ty.span == prev.span | ||
| } else { | ||
| false | ||
| } | ||
| }) { | ||
| span = span.to(predicate.span) | ||
| }; | ||
|
|
||
| span.shrink_to_hi().to(hir_param.span) | ||
| } else if let Some(next) = | ||
| hir_impl.generics.params[index + 1..].iter().find(is_impl_generic) | ||
| { | ||
| hir_param.span.until(next.span) | ||
| } else { | ||
| // Remove also angle brackets <> when there is just ONE generic parameter. | ||
| hir_impl.generics.span | ||
| }, |
There was a problem hiding this comment.
Do you mind making all this a method on Generics? For instance fn span_for_param_removal(&self, param: usize) -> Span.
Comment on lines
+12
to
+13
| LL | unsafe impl<Q: Trait> Send for Inner<Q> {} | ||
| | +++ |
There was a problem hiding this comment.
I'm not really convinced that suggesting to use Q in some path is the right solution. Inner takes no generic params, so the suggestion is wrong.
There was a problem hiding this comment.
To do this, we must know the ty::Ty for self_ty (e.g. Inner here). Is there any way to get it?
| help: either remove the type parameter T, or make use of it, for example | ||
| | | ||
| LL - impl<T: Tr> S<T::Assoc> { | ||
| LL + impl S<T::Assoc> { |
There was a problem hiding this comment.
This case is more subtle. We should not suggest to remove T if it appears somewhere.
There was a problem hiding this comment.
It looks like avoidable, but only we can do is to compare symbols of generics parameters and, bounds of them and generics of the type. Is it acceptable, or do you know better ways?
cjgillot
added
S-waiting-on-author
Unconstrained type parameters made no suggestion and it makes difficult to know how to fix (rust-lang#107295). To make fixing easier, we output suggestions how to fix.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
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.
Unconstrained type parameters made no suggestion and it makes difficult to know how to fix (#107295).
To make fixing easier, we output suggestions how to fix.
close #107295