From ab7d8f0f5a8624d583989b76015254fff2f59cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 10 Oct 2019 09:50:45 -0700 Subject: [PATCH] Deduplicate some code and apply review comments --- src/librustc/traits/error_reporting.rs | 195 +++++++++--------- .../bad-bounds-on-assoc-in-trait.stderr | 6 +- .../associated-types-bound-failure.fixed | 2 +- .../associated-types-for-unimpl-trait.fixed | 2 +- .../associated-types-for-unimpl-trait.stderr | 6 +- ...ated-types-no-suitable-supertrait-2.stderr | 6 +- ...ciated-types-no-suitable-supertrait.stderr | 6 +- ...ated-trait-in-method-without-default.fixed | 2 +- .../associated-types-unsized.fixed | 2 +- .../associated-types-unsized.stderr | 2 +- ...typeck-default-trait-impl-assoc-type.fixed | 2 +- ...ypeck-default-trait-impl-assoc-type.stderr | 2 +- .../wf/wf-trait-associated-type-trait.stderr | 7 +- src/test/ui/wf/wf-trait-default-fn-arg.stderr | 2 +- 14 files changed, 117 insertions(+), 125 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 91106d35a7ee..abf265be7c50 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -984,84 +984,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } }; + let mut suggest_restriction = |generics: &hir::Generics, msg| { + 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) { - debug!("suggest_restricting_param_bound node={:?}", node); match node { hir::Node::Item(hir::Item { - kind: hir::ItemKind::Fn(decl, _, generics, _), .. + kind: hir::ItemKind::Fn(_, _, generics, _), .. }) | hir::Node::TraitItem(hir::TraitItem { generics, - kind: hir::TraitItemKind::Method(hir::MethodSig { decl, .. }, _), .. + kind: hir::TraitItemKind::Method(..), .. }) | hir::Node::ImplItem(hir::ImplItem { generics, - kind: hir::ImplItemKind::Method(hir::MethodSig { decl, .. }, _), .. - }) if param_ty.map(|p| p.name.as_str() == "Self").unwrap_or(false) => { - if !generics.where_clause.predicates.is_empty() { - err.span_suggestion( - generics.where_clause.span().unwrap().shrink_to_hi(), - "consider further restricting `Self`", - format!(", {}", trait_ref.to_predicate()), - Applicability::MachineApplicable, - ); - } else { - err.span_suggestion( - decl.output.span().shrink_to_hi(), - "consider further restricting `Self`", - format!(" where {}", trait_ref.to_predicate()), - Applicability::MachineApplicable, - ); - } + kind: hir::ImplItemKind::Method(..), .. + }) if param_ty.map_or(false, |p| p.name.as_str() == "Self") => { + // Restricting `Self` for a single method. + suggest_restriction(&generics, "`Self`"); return; } + hir::Node::Item(hir::Item { - kind: hir::ItemKind::Fn(decl, _, generics, _), .. + kind: hir::ItemKind::Fn(_, _, generics, _), .. }) | hir::Node::TraitItem(hir::TraitItem { generics, - kind: hir::TraitItemKind::Method(hir::MethodSig { decl, .. }, _), .. + kind: hir::TraitItemKind::Method(..), .. }) | hir::Node::ImplItem(hir::ImplItem { generics, - kind: hir::ImplItemKind::Method(hir::MethodSig { decl, .. }, _), .. - }) if projection.is_some() => { - if !generics.where_clause.predicates.is_empty() { - err.span_suggestion( - generics.where_clause.span().unwrap().shrink_to_hi(), - "consider further restricting the associated type", - format!(", {}", trait_ref.to_predicate()), - Applicability::MachineApplicable, - ); - } else { - err.span_suggestion( - decl.output.span().shrink_to_hi(), - "consider further restricting the associated type", - format!(" where {}", trait_ref.to_predicate()), - Applicability::MachineApplicable, - ); - } - return; - } + 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() => { - err.span_suggestion( - generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(), - "consider further restricting the associated type", - format!( - "{} {}", if generics.where_clause.predicates.is_empty() { - " where" - } else { - " ," - }, - trait_ref.to_predicate(), - ), - Applicability::MachineApplicable, - ); + // 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, .. }) | @@ -1086,73 +1068,82 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { hir::Node::TraitItem(hir::TraitItem { generics, span, .. }) | hir::Node::ImplItem(hir::ImplItem { generics, span, .. }) if param_ty.is_some() => { + // Missing generic type parameter bound. let restrict_msg = "consider further restricting this bound"; let param_name = param_ty.unwrap().name.as_str(); - for param in &generics.params { - if param_name == param.name.ident().as_str() { - if param_name.starts_with("impl ") { + for param in generics.params.iter().filter(|p| { + param_name == 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) {}` → `fn foo(t: T) {}` err.span_suggestion( param.span, - restrict_msg, - // `impl CurrentTrait + MissingTrait` - format!("{} + {}", param.name.ident(), trait_ref), + "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) where T: Debug {}` → + // `fn foo(t: T, x: X) {}` → `fn foo(t: T) {}` + // `fn foo(t: T) {}` → `fn foo(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`. + err.span_suggestion(span, restrict_msg, format!( + "{} + ", + trait_ref.to_predicate(), + ), Applicability::MachineApplicable); } else { - 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`. - 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(), - )); - } + err.span_label(param.span, &format!( + "consider adding a `where {}` bound", + trait_ref.to_predicate(), + )); } } - return; } + return; } } + hir::Node::Crate => return, + _ => {} } hir_id = self.tcx.hir().get_parent_item(hir_id); } - // FIXME: Add special check for `?Sized` so we don't suggest `T: Sized + ?Sized`. - - // Fallback in case we didn't find the type argument. Can happen on associated types - // bounds and when `Self` needs to be restricted, like in the ui test - // `associated-types-projection-to-unrelated-trait-in-method-without-default.rs`. - err.help(&format!("consider adding a `where {}` bound", trait_ref.to_predicate())); } /// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a diff --git a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr index 1795fb5acebe..9f6a73cfe391 100644 --- a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr +++ b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr @@ -10,7 +10,7 @@ error[E0277]: `<::C as std::iter::Iterator>::Item` is not an iterato --> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1 | LL | fn assume_case1() { - | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::iter::Iterator` + | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::iter::Iterator` | _| | | LL | | @@ -30,7 +30,7 @@ LL | trait Case1 { | ----------- required by `Case1` ... LL | fn assume_case1() { - | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::marker::Send` + | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::marker::Send` | _| | | LL | | @@ -50,7 +50,7 @@ LL | trait Case1 { | ----------- required by `Case1` ... LL | fn assume_case1() { - | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::marker::Sync` + | ^ - help: consider further restricting the associated type: `where <::C as std::iter::Iterator>::Item: std::marker::Sync` | _| | | LL | | diff --git a/src/test/ui/associated-types/associated-types-bound-failure.fixed b/src/test/ui/associated-types/associated-types-bound-failure.fixed index 68ee38d16b3f..cc47f31d0045 100644 --- a/src/test/ui/associated-types/associated-types-bound-failure.fixed +++ b/src/test/ui/associated-types/associated-types-bound-failure.fixed @@ -14,7 +14,7 @@ pub trait GetToInt } fn foo(g: G) -> isize - where G : GetToInt, ::R: ToInt + where G : GetToInt, ::R: ToInt { ToInt::to_int(&g.get()) //~ ERROR E0277 } diff --git a/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed b/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed index b27d58018c29..aa23326506f6 100644 --- a/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed +++ b/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed @@ -7,7 +7,7 @@ trait Get { } trait Other { - fn uhoh(&self, foo: U, bar: ::Value) where Self: Get{} + fn uhoh(&self, foo: U, bar: ::Value) where Self: Get {} //~^ ERROR the trait bound `Self: Get` is not satisfied } diff --git a/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr b/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr index f8fc2b37e730..83d5390417e7 100644 --- a/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr +++ b/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr @@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied --> $DIR/associated-types-for-unimpl-trait.rs:10:5 | LL | fn uhoh(&self, foo: U, bar: ::Value) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^ - | | | - | | help: consider further restricting `Self`: `where Self: Get` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^ + | | | + | | help: consider further restricting `Self`: `where Self: Get` | the trait `Get` is not implemented for `Self` error: aborting due to previous error diff --git a/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr b/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr index 43950defc7cb..6aa0403088d3 100644 --- a/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr +++ b/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr @@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied --> $DIR/associated-types-no-suitable-supertrait-2.rs:17:5 | LL | fn uhoh(&self, foo: U, bar: ::Value) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^ - | | | - | | help: consider further restricting `Self`: `where Self: Get` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^ + | | | + | | help: consider further restricting `Self`: `where Self: Get` | the trait `Get` is not implemented for `Self` error: aborting due to previous error diff --git a/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr b/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr index 82e366dbea4b..8c242be97961 100644 --- a/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr +++ b/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr @@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied --> $DIR/associated-types-no-suitable-supertrait.rs:17:5 | LL | fn uhoh(&self, foo: U, bar: ::Value) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^ - | | | - | | help: consider further restricting `Self`: `where Self: Get` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^ + | | | + | | help: consider further restricting `Self`: `where Self: Get` | the trait `Get` is not implemented for `Self` error[E0277]: the trait bound `(T, U): Get` is not satisfied diff --git a/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed b/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed index 9bc308465ebd..f357045a456e 100644 --- a/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed +++ b/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed @@ -7,7 +7,7 @@ trait Get { } trait Other { - fn okay(&self, foo: U, bar: ::Value) where Self: Get; + fn okay(&self, foo: U, bar: ::Value) where Self: Get ; //~^ ERROR E0277 } diff --git a/src/test/ui/associated-types/associated-types-unsized.fixed b/src/test/ui/associated-types/associated-types-unsized.fixed index 385de541e560..f780d171fee8 100644 --- a/src/test/ui/associated-types/associated-types-unsized.fixed +++ b/src/test/ui/associated-types/associated-types-unsized.fixed @@ -6,7 +6,7 @@ trait Get { fn get(&self) -> ::Value; } -fn foo(t: T) where ::Value: std::marker::Sized{ +fn foo(t: T) where ::Value: std::marker::Sized { let x = t.get(); //~ ERROR the size for values of type } diff --git a/src/test/ui/associated-types/associated-types-unsized.stderr b/src/test/ui/associated-types/associated-types-unsized.stderr index 18595721bcee..2352ac4ad382 100644 --- a/src/test/ui/associated-types/associated-types-unsized.stderr +++ b/src/test/ui/associated-types/associated-types-unsized.stderr @@ -2,7 +2,7 @@ error[E0277]: the size for values of type `::Value` cannot be known at --> $DIR/associated-types-unsized.rs:10:9 | LL | fn foo(t: T) { - | - help: consider further restricting the associated type: `where ::Value: std::marker::Sized` + | - help: consider further restricting the associated type: `where ::Value: std::marker::Sized` LL | let x = t.get(); | ^ doesn't have a size known at compile-time | diff --git a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed index 757d8b8a235a..7a108d880bed 100644 --- a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed +++ b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed @@ -7,7 +7,7 @@ trait Trait { type AssocType; fn dummy(&self) { } } -fn bar() where ::AssocType: std::marker::Send{ +fn bar() where ::AssocType: std::marker::Send { is_send::(); //~ ERROR E0277 } diff --git a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr index dd6dcbe2ef57..2e54cdf01320 100644 --- a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr +++ b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr @@ -2,7 +2,7 @@ error[E0277]: `::AssocType` cannot be sent between threads safely --> $DIR/typeck-default-trait-impl-assoc-type.rs:11:5 | LL | fn bar() { - | - help: consider further restricting the associated type: `where ::AssocType: std::marker::Send` + | - help: consider further restricting the associated type: `where ::AssocType: std::marker::Send` LL | is_send::(); | ^^^^^^^^^^^^^^^^^^^^^^^ `::AssocType` cannot be sent between threads safely ... diff --git a/src/test/ui/wf/wf-trait-associated-type-trait.stderr b/src/test/ui/wf/wf-trait-associated-type-trait.stderr index d8ab95504823..93cb948cdbfc 100644 --- a/src/test/ui/wf/wf-trait-associated-type-trait.stderr +++ b/src/test/ui/wf/wf-trait-associated-type-trait.stderr @@ -3,11 +3,12 @@ error[E0277]: the trait bound `::Type1: std::marker::Copy` is | LL | struct IsCopy { x: T } | --------------------- required by `IsCopy` -... +LL | +LL | trait SomeTrait { + | - help: consider further restricting the associated type: `where ::Type1: std::marker::Copy` +LL | type Type1; LL | type Type2 = IsCopy; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `::Type1` - | - = help: consider adding a `where ::Type1: std::marker::Copy` bound error: aborting due to previous error diff --git a/src/test/ui/wf/wf-trait-default-fn-arg.stderr b/src/test/ui/wf/wf-trait-default-fn-arg.stderr index 3efcdf72eee3..9f3545b9c6a6 100644 --- a/src/test/ui/wf/wf-trait-default-fn-arg.stderr +++ b/src/test/ui/wf/wf-trait-default-fn-arg.stderr @@ -5,7 +5,7 @@ LL | struct Bar { value: Box } | ----------------------- required by `Bar` ... LL | fn bar(&self, x: &Bar) { - | ^ - help: consider further restricting `Self`: `where Self: std::cmp::Eq` + | ^ - help: consider further restricting `Self`: `where Self: std::cmp::Eq` | _____| | | LL | |