1
Fork 0

review comments

This commit is contained in:
Esteban Küber 2020-04-05 10:52:54 -07:00
parent 2c998aa8bb
commit 794b644f0b
3 changed files with 87 additions and 93 deletions

View file

@ -17,6 +17,7 @@
#![feature(crate_visibility_modifier)] #![feature(crate_visibility_modifier)]
#![feature(or_patterns)] #![feature(or_patterns)]
#![feature(str_strip)] #![feature(str_strip)]
#![feature(option_zip)]
#![recursion_limit = "512"] // For rustdoc #![recursion_limit = "512"] // For rustdoc
#[macro_use] #[macro_use]

View file

@ -388,7 +388,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// which is somewhat confusing. // which is somewhat confusing.
self.suggest_restricting_param_bound( self.suggest_restricting_param_bound(
&mut err, &mut err,
&trait_ref, trait_ref,
obligation.cause.body_id, obligation.cause.body_id,
); );
} else { } else {

View file

@ -27,7 +27,7 @@ pub trait InferCtxtExt<'tcx> {
fn suggest_restricting_param_bound( fn suggest_restricting_param_bound(
&self, &self,
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::PolyTraitRef<'_>, trait_ref: ty::PolyTraitRef<'_>,
body_id: hir::HirId, body_id: hir::HirId,
); );
@ -168,103 +168,96 @@ fn suggest_restriction(
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
fn_sig: Option<&hir::FnSig<'_>>, fn_sig: Option<&hir::FnSig<'_>>,
projection: Option<&ty::ProjectionTy<'_>>, projection: Option<&ty::ProjectionTy<'_>>,
trait_ref: &ty::PolyTraitRef<'_>, trait_ref: ty::PolyTraitRef<'_>,
) { ) {
let span = generics.where_clause.span_for_predicates_or_empty_place(); let span = generics.where_clause.span_for_predicates_or_empty_place();
if !span.from_expansion() && span.desugaring_kind().is_none() { if span.from_expansion() || span.desugaring_kind().is_some() {
// Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... return;
if let Some((name, fn_sig)) = fn_sig.and_then(|sig| { }
projection.and_then(|p| { // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`...
// Shenanigans to get the `Trait` from the `impl Trait`. if let Some((name, fn_sig)) =
match p.self_ty().kind { fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind {
ty::Param(param) => { // Shenanigans to get the `Trait` from the `impl Trait`.
// `fn foo(t: impl Trait)` ty::Param(param) => {
// ^^^^^ get this string // `fn foo(t: impl Trait)`
param // ^^^^^ get this string
.name param.name.as_str().strip_prefix("impl").map(|s| (s.trim_start().to_string(), sig))
.as_str() }
.strip_prefix("impl") _ => None,
.map(|s| (s.trim_start().to_string(), sig)) })
} {
_ => None, // We know we have an `impl Trait` that doesn't satisfy a required projection.
}
})
}) {
// We know we have an `impl Trait` that doesn't satisfy a required projection.
// Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments' // Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments'
// types. There should be at least one, but there might be *more* than one. In that // types. There should be at least one, but there might be *more* than one. In that
// case we could just ignore it and try to identify which one needs the restriction, // case we could just ignore it and try to identify which one needs the restriction,
// but instead we choose to suggest replacing all instances of `impl Trait` with `T` // but instead we choose to suggest replacing all instances of `impl Trait` with `T`
// where `T: Trait`. // where `T: Trait`.
let mut ty_spans = vec![]; let mut ty_spans = vec![];
let impl_name = format!("impl {}", name); let impl_name = format!("impl {}", name);
for input in fn_sig.decl.inputs { for input in fn_sig.decl.inputs {
if let hir::TyKind::Path(hir::QPath::Resolved( if let hir::TyKind::Path(hir::QPath::Resolved(
None, None,
hir::Path { segments: [segment], .. }, hir::Path { segments: [segment], .. },
)) = input.kind )) = input.kind
{ {
if segment.ident.as_str() == impl_name.as_str() { if segment.ident.as_str() == impl_name.as_str() {
// `fn foo(t: impl Trait)` // `fn foo(t: impl Trait)`
// ^^^^^^^^^^ get this to suggest // ^^^^^^^^^^ get this to suggest
// `T` instead // `T` instead
// There might be more than one `impl Trait`. // There might be more than one `impl Trait`.
ty_spans.push(input.span); ty_spans.push(input.span);
}
} }
} }
// The type param `T: Trait` we will suggest to introduce.
let type_param = format!("{}: {}", "T", name);
// FIXME: modify the `trait_ref` instead of string shenanigans.
// Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
let pred = trait_ref.without_const().to_predicate().to_string();
let pred = pred.replace(&impl_name, "T");
let mut sugg = vec![
match generics
.params
.iter()
.filter(|p| match p.kind {
hir::GenericParamKind::Type {
synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
..
} => false,
_ => true,
})
.last()
{
// `fn foo(t: impl Trait)`
// ^ suggest `<T: Trait>` here
None => (generics.span, format!("<{}>", type_param)),
// `fn foo<A>(t: impl Trait)`
// ^^^ suggest `<A, T: Trait>` here
Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)),
},
// `fn foo(t: impl Trait)`
// ^ suggest `where <T as Trait>::A: Bound`
predicate_constraint(generics, pred),
];
sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string())));
// Suggest `fn foo<T: Trait>(t: T) where <T as Trait>::A: Bound`.
err.multipart_suggestion(
"introduce a type parameter with a trait bound instead of using \
`impl Trait`",
sugg,
Applicability::MaybeIncorrect,
);
} else {
// Trivial case: `T` needs an extra bound: `T: Bound`.
let (sp, s) = predicate_constraint(
generics,
trait_ref.without_const().to_predicate().to_string(),
);
let appl = Applicability::MachineApplicable;
err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl);
} }
// The type param `T: Trait` we will suggest to introduce.
let type_param = format!("{}: {}", "T", name);
// FIXME: modify the `trait_ref` instead of string shenanigans.
// Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
let pred = trait_ref.without_const().to_predicate().to_string();
let pred = pred.replace(&impl_name, "T");
let mut sugg = vec![
match generics
.params
.iter()
.filter(|p| match p.kind {
hir::GenericParamKind::Type {
synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
..
} => false,
_ => true,
})
.last()
{
// `fn foo(t: impl Trait)`
// ^ suggest `<T: Trait>` here
None => (generics.span, format!("<{}>", type_param)),
// `fn foo<A>(t: impl Trait)`
// ^^^ suggest `<A, T: Trait>` here
Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)),
},
// `fn foo(t: impl Trait)`
// ^ suggest `where <T as Trait>::A: Bound`
predicate_constraint(generics, pred),
];
sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string())));
// Suggest `fn foo<T: Trait>(t: T) where <T as Trait>::A: Bound`.
err.multipart_suggestion(
"introduce a type parameter with a trait bound instead of using \
`impl Trait`",
sugg,
Applicability::MaybeIncorrect,
);
} else {
// Trivial case: `T` needs an extra bound: `T: Bound`.
let (sp, s) =
predicate_constraint(generics, trait_ref.without_const().to_predicate().to_string());
let appl = Applicability::MachineApplicable;
err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl);
} }
} }
@ -272,7 +265,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
fn suggest_restricting_param_bound( fn suggest_restricting_param_bound(
&self, &self,
mut err: &mut DiagnosticBuilder<'_>, mut err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::PolyTraitRef<'_>, trait_ref: ty::PolyTraitRef<'_>,
body_id: hir::HirId, body_id: hir::HirId,
) { ) {
let self_ty = trait_ref.self_ty(); let self_ty = trait_ref.self_ty();