Skip to content

Commit

Permalink
Rollup merge of rust-lang#65192 - estebank:restrict-bound, r=matthewj…
Browse files Browse the repository at this point in the history
…asper

Use structured suggestion for restricting bounds

When a trait bound is not met and restricting a type parameter would
make the restriction hold, use a structured suggestion pointing at an
appropriate place (type param in param list or `where` clause).

Account for opaque parameters where instead of suggesting extending
the `where` clause, we suggest appending the new restriction:
`fn foo(impl Trait + UnmetTrait)`. Fix rust-lang#64565, fix rust-lang#41817, fix rust-lang#24354,
cc rust-lang#26026, cc rust-lang#37808, cc rust-lang#24159, fix rust-lang#37138, fix rust-lang#24354, cc rust-lang#20671.
  • Loading branch information
Centril authored Oct 19, 2019
2 parents e5b8c11 + c6dce78 commit ef8ac78
Show file tree
Hide file tree
Showing 102 changed files with 847 additions and 243 deletions.
6 changes: 6 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,12 @@ impl WhereClause {
Some(self.span)
}
}

/// The `WhereClause` under normal circumstances points at either the predicates or the empty
/// space where the `where` clause should be. Only of use for diagnostic suggestions.
pub fn span_for_predicates_or_empty_place(&self) -> Span {
self.span
}
}

/// A single predicate in a where-clause.
Expand Down
176 changes: 174 additions & 2 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,8 +715,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// these notes will often be of the form
// "the type `T` can't be frobnicated"
// which is somewhat confusing.
err.help(&format!("consider adding a `where {}` bound",
trait_ref.to_predicate()));
self.suggest_restricting_param_bound(
&mut err,
&trait_ref,
obligation.cause.body_id,
);
} else {
if !have_alt_message {
// Can't show anything else useful, try to find similar impls.
Expand Down Expand Up @@ -960,6 +963,175 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.emit();
}

fn suggest_restricting_param_bound(
&self,
err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::PolyTraitRef<'_>,
body_id: hir::HirId,
) {
let self_ty = trait_ref.self_ty();
let (param_ty, projection) = match &self_ty.kind {
ty::Param(_) => (true, None),
ty::Projection(projection) => (false, Some(projection)),
_ => return,
};

let mut suggest_restriction = |generics: &hir::Generics, msg| {
let span = generics.where_clause.span_for_predicates_or_empty_place();
if !span.from_expansion() && span.desugaring_kind().is_none() {
err.span_suggestion(
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(),
&format!("consider further restricting {}", msg),
format!(
"{} {} ",
if !generics.where_clause.predicates.is_empty() {
","
} else {
" where"
},
trait_ref.to_predicate(),
),
Applicability::MachineApplicable,
);
}
};

// FIXME: Add check for trait bound that is already present, particularly `?Sized` so we
// don't suggest `T: Sized + ?Sized`.
let mut hir_id = body_id;
while let Some(node) = self.tcx.hir().find(hir_id) {
match node {
hir::Node::TraitItem(hir::TraitItem {
generics,
kind: hir::TraitItemKind::Method(..), ..
}) if param_ty && self_ty == self.tcx.types.self_param => {
// Restricting `Self` for a single method.
suggest_restriction(&generics, "`Self`");
return;
}

hir::Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, _, generics, _), ..
}) |
hir::Node::TraitItem(hir::TraitItem {
generics,
kind: hir::TraitItemKind::Method(..), ..
}) |
hir::Node::ImplItem(hir::ImplItem {
generics,
kind: hir::ImplItemKind::Method(..), ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, _, _), ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl(_, _, _, generics, ..), ..
}) if projection.is_some() => {
// Missing associated type bound.
suggest_restriction(&generics, "the associated type");
return;
}

hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(_, generics), span, .. }) |
hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(_, generics), span, .. }) |
hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(_, generics), span, .. }) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, ..), span, ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl(_, _, _, generics, ..), span, ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, _, generics, _), span, ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::TyAlias(_, generics), span, ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::TraitAlias(generics, _), span, ..
}) |
hir::Node::Item(hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }), span, ..
}) |
hir::Node::TraitItem(hir::TraitItem { generics, span, .. }) |
hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
if param_ty => {
// Missing generic type parameter bound.
let restrict_msg = "consider further restricting this bound";
let param_name = self_ty.to_string();
for param in generics.params.iter().filter(|p| {
&param_name == std::convert::AsRef::<str>::as_ref(&p.name.ident().as_str())
}) {
if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
param.span,
restrict_msg,
// `impl CurrentTrait + MissingTrait`
format!("{} + {}", param.name.ident(), trait_ref),
Applicability::MachineApplicable,
);
} else if generics.where_clause.predicates.is_empty() &&
param.bounds.is_empty()
{
// If there are no bounds whatsoever, suggest adding a constraint
// to the type parameter:
// `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
err.span_suggestion(
param.span,
"consider restricting this bound",
format!("{}", trait_ref.to_predicate()),
Applicability::MachineApplicable,
);
} else if !generics.where_clause.predicates.is_empty() {
// There is a `where` clause, so suggest expanding it:
// `fn foo<T>(t: T) where T: Debug {}` →
// `fn foo<T>(t: T) where T: Debug, T: Trait {}`
err.span_suggestion(
generics.where_clause.span().unwrap().shrink_to_hi(),
&format!(
"consider further restricting type parameter `{}`",
param_name,
),
format!(", {}", trait_ref.to_predicate()),
Applicability::MachineApplicable,
);
} else {
// If there is no `where` clause lean towards constraining to the
// type parameter:
// `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
// `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
let sp = param.span.with_hi(span.hi());
let span = self.tcx.sess.source_map()
.span_through_char(sp, ':');
if sp != param.span && sp != span {
// Only suggest if we have high certainty that the span
// covers the colon in `foo<T: Trait>`.
err.span_suggestion(span, restrict_msg, format!(
"{} + ",
trait_ref.to_predicate(),
), Applicability::MachineApplicable);
} else {
err.span_label(param.span, &format!(
"consider adding a `where {}` bound",
trait_ref.to_predicate(),
));
}
}
return;
}
}

hir::Node::Crate => return,

_ => {}
}

hir_id = self.tcx.hir().get_parent_item(hir_id);
}
}

/// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a
/// suggestion to borrow the initializer in order to use have a slice instead.
fn suggest_borrow_on_unsized_slice(
Expand Down
6 changes: 1 addition & 5 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// If the span is from a macro, then it's hard to extract the text
// and make a good suggestion, so don't bother.
let is_desugaring = match sp.desugaring_kind() {
Some(k) => sp.is_desugaring(k),
None => false
};
let is_macro = sp.from_expansion() && !is_desugaring;
let is_macro = sp.from_expansion() && sp.desugaring_kind().is_none();

// `ExprKind::DropTemps` is semantically irrelevant for these suggestions.
let expr = expr.peel_drop_temps();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ error[E0277]: the trait bound `A: Foo` is not satisfied
LL | const Y: usize;
| --------------- required by `Foo::Y`
...
LL | pub fn test<A: Foo, B: Foo>() {
| -- help: consider further restricting this bound: `A: Foo +`
LL | let _array = [4; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
= help: consider adding a `where A: Foo` bound

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ error[E0277]: the trait bound `A: Foo` is not satisfied
LL | const Y: usize;
| --------------- required by `Foo::Y`
...
LL | pub fn test<A: Foo, B: Foo>() {
| -- help: consider further restricting this bound: `A: Foo +`
LL | let _array: [u32; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
= help: consider adding a `where A: Foo` bound

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ LL | impl Case1 for S1 {
error[E0277]: `<<T as Case1>::C as std::iter::Iterator>::Item` is not an iterator
--> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1
|
LL | / fn assume_case1<T: Case1>() {
LL | fn assume_case1<T: Case1>() {
| ^ - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::iter::Iterator`
| _|
| |
LL | |
LL | |
LL | |
Expand All @@ -19,15 +22,17 @@ LL | | }
| |_^ `<<T as Case1>::C as std::iter::Iterator>::Item` is not an iterator
|
= help: the trait `std::iter::Iterator` is not implemented for `<<T as Case1>::C as std::iter::Iterator>::Item`
= help: consider adding a `where <<T as Case1>::C as std::iter::Iterator>::Item: std::iter::Iterator` bound

error[E0277]: `<<T as Case1>::C as std::iter::Iterator>::Item` cannot be sent between threads safely
--> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1
|
LL | trait Case1 {
| ----------- required by `Case1`
...
LL | / fn assume_case1<T: Case1>() {
LL | fn assume_case1<T: Case1>() {
| ^ - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Send`
| _|
| |
LL | |
LL | |
LL | |
Expand All @@ -37,15 +42,17 @@ LL | | }
| |_^ `<<T as Case1>::C as std::iter::Iterator>::Item` cannot be sent between threads safely
|
= help: the trait `std::marker::Send` is not implemented for `<<T as Case1>::C as std::iter::Iterator>::Item`
= help: consider adding a `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Send` bound

error[E0277]: `<<T as Case1>::C as std::iter::Iterator>::Item` cannot be shared between threads safely
--> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1
|
LL | trait Case1 {
| ----------- required by `Case1`
...
LL | / fn assume_case1<T: Case1>() {
LL | fn assume_case1<T: Case1>() {
| ^ - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Sync`
| _|
| |
LL | |
LL | |
LL | |
Expand All @@ -55,7 +62,6 @@ LL | | }
| |_^ `<<T as Case1>::C as std::iter::Iterator>::Item` cannot be shared between threads safely
|
= help: the trait `std::marker::Sync` is not implemented for `<<T as Case1>::C as std::iter::Iterator>::Item`
= help: consider adding a `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Sync` bound

error[E0277]: `<_ as Lam<&'a u8>>::App` doesn't implement `std::fmt::Debug`
--> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1
Expand Down
29 changes: 29 additions & 0 deletions src/test/ui/associated-types/associated-types-bound-failure.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// run-rustfix
// Test equality constraints on associated types in a where clause.
#![allow(dead_code)]

pub trait ToInt {
fn to_int(&self) -> isize;
}

pub trait GetToInt
{
type R;

fn get(&self) -> <Self as GetToInt>::R;
}

fn foo<G>(g: G) -> isize
where G : GetToInt, <G as GetToInt>::R: ToInt
{
ToInt::to_int(&g.get()) //~ ERROR E0277
}

fn bar<G : GetToInt>(g: G) -> isize
where G::R : ToInt
{
ToInt::to_int(&g.get()) // OK
}

pub fn main() {
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// run-rustfix
// Test equality constraints on associated types in a where clause.
#![allow(dead_code)]

pub trait ToInt {
fn to_int(&self) -> isize;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
error[E0277]: the trait bound `<G as GetToInt>::R: ToInt` is not satisfied
--> $DIR/associated-types-bound-failure.rs:17:19
--> $DIR/associated-types-bound-failure.rs:19:19
|
LL | fn to_int(&self) -> isize;
| -------------------------- required by `ToInt::to_int`
...
LL | where G : GetToInt
| - help: consider further restricting the associated type: `, <G as GetToInt>::R: ToInt`
LL | {
LL | ToInt::to_int(&g.get())
| ^^^^^^^^ the trait `ToInt` is not implemented for `<G as GetToInt>::R`
|
= help: consider adding a `where <G as GetToInt>::R: ToInt` bound

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-rustfix
#![allow(unused_variables)]

trait Get {
type Value;
fn get(&self) -> <Self as Get>::Value;
}

trait Other {
fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get {}
//~^ ERROR the trait bound `Self: Get` is not satisfied
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// run-rustfix
#![allow(unused_variables)]

trait Get {
type Value;
fn get(&self) -> <Self as Get>::Value;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error[E0277]: the trait bound `Self: Get` is not satisfied
--> $DIR/associated-types-for-unimpl-trait.rs:7:5
--> $DIR/associated-types-for-unimpl-trait.rs:10:5
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
= help: consider adding a `where Self: Get` bound
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^
| | |
| | help: consider further restricting `Self`: `where Self: Get`
| the trait `Get` is not implemented for `Self`

error: aborting due to previous error

Expand Down
Loading

0 comments on commit ef8ac78

Please sign in to comment.