Suggest how to fix with unconstrained type parameters#134270
Closed
kei519 wants to merge 2 commits intorust-lang:masterfrom
Closed
Suggest how to fix with unconstrained type parameters#134270kei519 wants to merge 2 commits intorust-lang:masterfrom
kei519 wants to merge 2 commits intorust-lang:masterfrom
Conversation
Collaborator
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
Collaborator
|
☔ 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.
Collaborator
|
☔ 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"); |
Contributor
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(); |
Contributor
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) |
Contributor
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 | ||
| }, |
Contributor
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> {} | ||
| | +++ |
Contributor
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.
Contributor
Author
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> { |
Contributor
There was a problem hiding this comment.
This case is more subtle. We should not suggest to remove T if it appears somewhere.
Contributor
Author
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.
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Member
|
@kei519 any updates on this? thanks |
Contributor
Author
|
@Dylan-DPC Thank you for your comment. |
Collaborator
|
☔ The latest upstream changes (presumably #140043) made this pull request unmergeable. Please resolve the merge conflicts. |
Member
|
Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks |
rustbot
removed
the
S-waiting-on-author
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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