Skip to content

Commit 02711fd

Browse files
estebankestebank
committed
review comments
1 parent 9dc9785 commit 02711fd

File tree

2 files changed

+140
-130
lines changed

2 files changed

+140
-130
lines changed

‎src/librustc_trait_selection/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#![feature(in_band_lifetimes)]
1717
#![feature(crate_visibility_modifier)]
1818
#![feature(or_patterns)]
19+
#![feature(str_strip)]
1920
#![recursion_limit = "512"] // For rustdoc
2021

2122
#[macro_use]

‎src/librustc_trait_selection/traits/error_reporting/suggestions.rs

+139-130
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,126 @@ crate trait InferCtxtExt<'tcx> {
147147
fn suggest_new_overflow_limit(&self, err: &mut DiagnosticBuilder<'_>);
148148
}
149149

150+
fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) {
151+
(
152+
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(),
153+
format!(
154+
"{} {} ",
155+
if !generics.where_clause.predicates.is_empty() { "," } else { " where" },
156+
pred,
157+
),
158+
)
159+
}
160+
161+
/// Type parameter needs more bounds. The trivial case is `T` `where T: Bound`, but
162+
/// it can also be an `impl Trait` param that needs to be decomposed to a type
163+
/// param for cleaner code.
164+
fn suggest_restriction(
165+
generics: &hir::Generics<'_>,
166+
msg: &str,
167+
err: &mut DiagnosticBuilder<'_>,
168+
fn_sig: Option<&hir::FnSig<'_>>,
169+
projection: Option<&ty::ProjectionTy<'_>>,
170+
trait_ref: &ty::PolyTraitRef<'_>,
171+
) {
172+
let span = generics.where_clause.span_for_predicates_or_empty_place();
173+
if !span.from_expansion() && span.desugaring_kind().is_none() {
174+
// Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`...
175+
if let Some((name, fn_sig)) = fn_sig.and_then(|sig| {
176+
projection.and_then(|p| {
177+
// Shenanigans to get the `Trait` from the `impl Trait`.
178+
match p.self_ty().kind {
179+
ty::Param(param) => {
180+
// `fn foo(t: impl Trait)`
181+
// ^^^^^ get this string
182+
param
183+
.name
184+
.as_str()
185+
.strip_prefix("impl")
186+
.map(|s| (s.trim_start().to_string(), sig))
187+
}
188+
_ => None,
189+
}
190+
})
191+
}) {
192+
// We know we have an `impl Trait` that doesn't satisfy a required projection.
193+
194+
// Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments'
195+
// types. There should be at least one, but there might be *more* than one. In that
196+
// case we could just ignore it and try to identify which one needs the restriction,
197+
// but instead we choose to suggest replacing all instances of `impl Trait` with `T`
198+
// where `T: Trait`.
199+
let mut ty_spans = vec![];
200+
let impl_name = format!("impl {}", name);
201+
for input in fn_sig.decl.inputs {
202+
if let hir::TyKind::Path(hir::QPath::Resolved(
203+
None,
204+
hir::Path { segments: [segment], .. },
205+
)) = input.kind
206+
{
207+
if segment.ident.as_str() == impl_name.as_str() {
208+
// `fn foo(t: impl Trait)`
209+
// ^^^^^^^^^^ get this to suggest
210+
// `T` instead
211+
212+
// There might be more than one `impl Trait`.
213+
ty_spans.push(input.span);
214+
}
215+
}
216+
}
217+
218+
// The type param `T: Trait` we will suggest to introduce.
219+
let type_param = format!("{}: {}", "T", name);
220+
221+
// FIXME: modify the `trait_ref` instead of string shenanigans.
222+
// Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
223+
let pred = trait_ref.without_const().to_predicate().to_string();
224+
let pred = pred.replace(&impl_name, "T");
225+
let mut sugg = vec![
226+
match generics
227+
.params
228+
.iter()
229+
.filter(|p| match p.kind {
230+
hir::GenericParamKind::Type {
231+
synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
232+
..
233+
} => false,
234+
_ => true,
235+
})
236+
.last()
237+
{
238+
// `fn foo(t: impl Trait)`
239+
// ^ suggest `<T: Trait>` here
240+
None => (generics.span, format!("<{}>", type_param)),
241+
// `fn foo<A>(t: impl Trait)`
242+
// ^^^ suggest `<A, T: Trait>` here
243+
Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)),
244+
},
245+
// `fn foo(t: impl Trait)`
246+
// ^ suggest `where <T as Trait>::A: Bound`
247+
predicate_constraint(generics, pred),
248+
];
249+
sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string())));
250+
251+
// Suggest `fn foo<T: Trait>(t: T) where <T as Trait>::A: Bound`.
252+
err.multipart_suggestion(
253+
"introduce a type parameter with a trait bound instead of using \
254+
`impl Trait`",
255+
sugg,
256+
Applicability::MaybeIncorrect,
257+
);
258+
} else {
259+
// Trivial case: `T` needs an extra bound: `T: Bound`.
260+
let (sp, s) = predicate_constraint(
261+
generics,
262+
trait_ref.without_const().to_predicate().to_string(),
263+
);
264+
let appl = Applicability::MachineApplicable;
265+
err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl);
266+
}
267+
}
268+
}
269+
150270
impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
151271
fn suggest_restricting_param_bound(
152272
&self,
@@ -161,143 +281,18 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
161281
_ => return,
162282
};
163283

164-
let suggest_restriction =
165-
|generics: &hir::Generics<'_>,
166-
msg,
167-
err: &mut DiagnosticBuilder<'_>,
168-
fn_sig: Option<&hir::FnSig<'_>>| {
169-
// Type parameter needs more bounds. The trivial case is `T` `where T: Bound`, but
170-
// it can also be an `impl Trait` param that needs to be decomposed to a type
171-
// param for cleaner code.
172-
let span = generics.where_clause.span_for_predicates_or_empty_place();
173-
if !span.from_expansion() && span.desugaring_kind().is_none() {
174-
// Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`...
175-
if let Some((name, fn_sig)) = fn_sig.and_then(|sig| {
176-
projection.and_then(|p| {
177-
// Shenanigans to get the `Trait` from the `impl Trait`.
178-
match p.self_ty().kind {
179-
ty::Param(param) if param.name.as_str().starts_with("impl ") => {
180-
let n = param.name.as_str();
181-
// `fn foo(t: impl Trait)`
182-
// ^^^^^ get this string
183-
n.split_whitespace()
184-
.skip(1)
185-
.next()
186-
.map(|n| (n.to_string(), sig))
187-
}
188-
_ => None,
189-
}
190-
})
191-
}) {
192-
// FIXME: Cleanup.
193-
let mut ty_spans = vec![];
194-
let impl_name = format!("impl {}", name);
195-
for i in fn_sig.decl.inputs {
196-
if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = i.kind {
197-
match path.segments {
198-
[segment] if segment.ident.to_string() == impl_name => {
199-
// `fn foo(t: impl Trait)`
200-
// ^^^^^^^^^^ get this to suggest
201-
// `T` instead
202-
203-
// There might be more than one `impl Trait`.
204-
ty_spans.push(i.span);
205-
}
206-
_ => {}
207-
}
208-
}
209-
}
210-
211-
let type_param = format!("{}: {}", "T", name);
212-
// FIXME: modify the `trait_ref` instead of string shenanigans.
213-
// Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
214-
let pred = trait_ref.without_const().to_predicate().to_string();
215-
let pred = pred.replace(&impl_name, "T");
216-
let mut sugg = vec![
217-
match generics
218-
.params
219-
.iter()
220-
.filter(|p| match p.kind {
221-
hir::GenericParamKind::Type {
222-
synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
223-
..
224-
} => false,
225-
_ => true,
226-
})
227-
.last()
228-
{
229-
// `fn foo(t: impl Trait)`
230-
// ^ suggest `<T: Trait>` here
231-
None => (generics.span, format!("<{}>", type_param)),
232-
Some(param) => {
233-
(param.span.shrink_to_hi(), format!(", {}", type_param))
234-
}
235-
},
236-
(
237-
// `fn foo(t: impl Trait)`
238-
// ^ suggest `where <T as Trait>::A: Bound`
239-
generics
240-
.where_clause
241-
.span_for_predicates_or_empty_place()
242-
.shrink_to_hi(),
243-
format!(
244-
"{} {} ",
245-
if !generics.where_clause.predicates.is_empty() {
246-
","
247-
} else {
248-
" where"
249-
},
250-
pred,
251-
),
252-
),
253-
];
254-
sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string())));
255-
// Suggest `fn foo<T: Trait>(t: T) where <T as Trait>::A: Bound`.
256-
err.multipart_suggestion(
257-
"introduce a type parameter with a trait bound instead of using \
258-
`impl Trait`",
259-
sugg,
260-
Applicability::MaybeIncorrect,
261-
);
262-
} else {
263-
// Trivial case: `T` needs an extra bound.
264-
err.span_suggestion(
265-
generics
266-
.where_clause
267-
.span_for_predicates_or_empty_place()
268-
.shrink_to_hi(),
269-
&format!("consider further restricting {}", msg),
270-
format!(
271-
"{} {} ",
272-
if !generics.where_clause.predicates.is_empty() {
273-
","
274-
} else {
275-
" where"
276-
},
277-
trait_ref.without_const().to_predicate(),
278-
),
279-
Applicability::MachineApplicable,
280-
);
281-
}
282-
}
283-
};
284-
285284
// FIXME: Add check for trait bound that is already present, particularly `?Sized` so we
286285
// don't suggest `T: Sized + ?Sized`.
287286
let mut hir_id = body_id;
288287
while let Some(node) = self.tcx.hir().find(hir_id) {
289-
debug!(
290-
"suggest_restricting_param_bound {:?} {:?} {:?} {:?}",
291-
trait_ref, self_ty.kind, projection, node
292-
);
293288
match node {
294289
hir::Node::TraitItem(hir::TraitItem {
295290
generics,
296291
kind: hir::TraitItemKind::Fn(..),
297292
..
298293
}) if param_ty && self_ty == self.tcx.types.self_param => {
299294
// Restricting `Self` for a single method.
300-
suggest_restriction(&generics, "`Self`", err, None);
295+
suggest_restriction(&generics, "`Self`", err, None, projection, trait_ref);
301296
return;
302297
}
303298

@@ -314,16 +309,30 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
314309
| hir::Node::Item(hir::Item {
315310
kind: hir::ItemKind::Fn(fn_sig, generics, _), ..
316311
}) if projection.is_some() => {
317-
// Missing associated type bound.
318-
suggest_restriction(&generics, "the associated type", err, Some(fn_sig));
312+
// Missing restriction on associated type of type parameter (unmet projection).
313+
suggest_restriction(
314+
&generics,
315+
"the associated type",
316+
err,
317+
Some(fn_sig),
318+
projection,
319+
trait_ref,
320+
);
319321
return;
320322
}
321323
hir::Node::Item(
322324
hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. }
323325
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. },
324326
) if projection.is_some() => {
325-
// Missing associated type bound.
326-
suggest_restriction(&generics, "the associated type", err, None);
327+
// Missing restriction on associated type of type parameter (unmet projection).
328+
suggest_restriction(
329+
&generics,
330+
"the associated type",
331+
err,
332+
None,
333+
projection,
334+
trait_ref,
335+
);
327336
return;
328337
}
329338

0 commit comments

Comments
 (0)