Skip to content

Commit 2c998aa

Browse files
estebankestebank
committed
review comments
1 parent 9d83cc8 commit 2c998aa

File tree

2 files changed

+140
-130
lines changed

2 files changed

+140
-130
lines changed

‎src/librustc_trait_selection/lib.rs

Lines changed: 1 addition & 0 deletions
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

Lines changed: 139 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,126 @@ pub trait InferCtxtExt<'tcx> {
148148
fn suggest_new_overflow_limit(&self, err: &mut DiagnosticBuilder<'_>);
149149
}
150150

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

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

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

0 commit comments

Comments
 (0)