-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide more context when denying invalid type params #97471
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic does not seem straightforward, so I have a lot of questions.
let suggestion = vec![ | ||
if is_self { | ||
// Account for people writing `Self::Variant::<Args>`, where | ||
// `Self` is the enum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the relation between this comment and the match
above which defines is_self
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expanded the comment
@@ -2017,69 +2017,79 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
self.normalize_ty(span, tcx.mk_projection(item_def_id, item_substs)) | |||
} | |||
|
|||
pub fn prohibit_generics<'a, T: IntoIterator<Item = &'a hir::PathSegment<'a>>>( | |||
pub fn prohibit_generics<'a, T: Iterator<Item = &'a hir::PathSegment<'a>> + Clone>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't T
always a slice &[&hir::PathSegment<'_>]
? Can't we just take this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried doing that. It isn't, there are a couple of callers that pass in an Iterator after skipping a few things. I think at least one uses filter_map
, which isn't amenable to a direct subslicing approach.
@@ -681,7 +681,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
trait_ref: &hir::TraitRef<'_>, | |||
self_ty: Ty<'tcx>, | |||
) -> ty::TraitRef<'tcx> { | |||
self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1); | |||
self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1.iter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit (pre-existing): an explicit slice may be easier to read.
@@ -784,7 +784,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
let args = trait_segment.args(); | |||
let infer_args = trait_segment.infer_args; | |||
|
|||
self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1); | |||
self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1.iter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise.
This comment was marked as resolved.
This comment was marked as resolved.
9b4436d
to
6fdd7e7
Compare
* Confirm the path segment being modified is an `enum` * Check whether type has type param before suggesting changing `Self` * Wording changes * Add clarifying comments * Suggest removing args from `Self` if the type doesn't have type params
6fdd7e7
to
9d9acff
Compare
This comment was marked as resolved.
This comment was marked as resolved.
9d9acff
to
da126eb
Compare
da126eb
to
ad63f90
Compare
01fa151
to
31e557b
Compare
31e557b
to
cd8cfbf
Compare
@bors r+ |
📌 Commit cd8cfbf has been approved by |
Provide more context when denying invalid type params
Rollup of 3 pull requests Successful merges: - rust-lang#97415 (Compute `is_late_bound_map` query separately from lifetime resolution) - rust-lang#97471 (Provide more context when denying invalid type params ) - rust-lang#97681 (Add more eslint checks) Failed merges: - rust-lang#97446 (Make hir().get_generics and generics_of consistent.) r? `@ghost` `@rustbot` modify labels: rollup
No description provided.