From 02bc412d192549977a9244174cd871b394ce4960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 19 Nov 2019 16:43:24 -0800 Subject: [PATCH 1/3] Use structured suggestion when requiring `Copy` constraint in type param --- .../borrow_check/conflict_errors.rs | 64 +++++++++++++++++-- src/test/ui/binop/binop-consume-args.stderr | 60 ++++++++--------- src/test/ui/binop/binop-move-semantics.stderr | 8 +-- .../borrowck/borrowck-unboxed-closures.stderr | 4 +- src/test/ui/issues/issue-34721.fixed | 34 ++++++++++ src/test/ui/issues/issue-34721.rs | 2 + src/test/ui/issues/issue-34721.stderr | 6 +- ...-on-type-no-recursive-stack-closure.stderr | 6 +- .../ui/once-cant-call-twice-on-heap.stderr | 4 +- src/test/ui/unop-move-semantics.stderr | 4 +- 10 files changed, 140 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/issues/issue-34721.fixed diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index 8508bf62d8f60..e3da090a62d30 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -231,12 +231,64 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let ty::Param(param_ty) = ty.kind { let tcx = self.infcx.tcx; let generics = tcx.generics_of(self.mir_def_id); - let def_id = generics.type_param(¶m_ty, tcx).def_id; - if let Some(sp) = tcx.hir().span_if_local(def_id) { - err.span_label( - sp, - "consider adding a `Copy` constraint to this type argument", - ); + let param = generics.type_param(¶m_ty, tcx); + let generics = tcx.hir().get_generics(self.mir_def_id).unwrap(); + let msg = "consider adding a `Copy` constraint to this type argument"; + for param in generics.params.iter().filter(|p| { + p.name.ident().as_str() == param.name.as_str() + }) { + let param_name = param.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, + msg, + // `impl CurrentTrait + MissingTrait` + format!("{} + Copy", param_name), + 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, + msg, + format!("{}: Copy", param_name), + 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) where T: Debug, T: Trait {}` + err.span_suggestion( + generics.where_clause.span().unwrap().shrink_to_hi(), + msg, + format!(", {}: Copy", param_name), + Applicability::MachineApplicable, + ); + } else { + // If there is no `where` clause lean towards constraining to the + // type parameter: + // `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 = 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, msg, format!( + "{}: Copy +", + param_name, + ), Applicability::MachineApplicable); + } else { + err.span_label(param.span, msg); + } + } } } let span = if let Some(local) = place.as_local() { diff --git a/src/test/ui/binop/binop-consume-args.stderr b/src/test/ui/binop/binop-consume-args.stderr index 5751af27fcb42..5a6a2f9258393 100644 --- a/src/test/ui/binop/binop-consume-args.stderr +++ b/src/test/ui/binop/binop-consume-args.stderr @@ -2,9 +2,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:7:10 | LL | fn add, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs + rhs; | --- value moved here LL | drop(lhs); @@ -16,7 +16,7 @@ error[E0382]: use of moved value: `rhs` LL | fn add, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs + rhs; | --- value moved here LL | drop(lhs); @@ -27,9 +27,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:13:10 | LL | fn sub, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs - rhs; | --- value moved here LL | drop(lhs); @@ -41,7 +41,7 @@ error[E0382]: use of moved value: `rhs` LL | fn sub, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs - rhs; | --- value moved here LL | drop(lhs); @@ -52,9 +52,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:19:10 | LL | fn mul, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs * rhs; | --- value moved here LL | drop(lhs); @@ -66,7 +66,7 @@ error[E0382]: use of moved value: `rhs` LL | fn mul, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs * rhs; | --- value moved here LL | drop(lhs); @@ -77,9 +77,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:25:10 | LL | fn div, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs / rhs; | --- value moved here LL | drop(lhs); @@ -91,7 +91,7 @@ error[E0382]: use of moved value: `rhs` LL | fn div, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs / rhs; | --- value moved here LL | drop(lhs); @@ -102,9 +102,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:31:10 | LL | fn rem, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs % rhs; | --- value moved here LL | drop(lhs); @@ -116,7 +116,7 @@ error[E0382]: use of moved value: `rhs` LL | fn rem, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs % rhs; | --- value moved here LL | drop(lhs); @@ -127,9 +127,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:37:10 | LL | fn bitand, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs & rhs; | --- value moved here LL | drop(lhs); @@ -141,7 +141,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitand, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs & rhs; | --- value moved here LL | drop(lhs); @@ -152,9 +152,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:43:10 | LL | fn bitor, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs | rhs; | --- value moved here LL | drop(lhs); @@ -166,7 +166,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitor, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs | rhs; | --- value moved here LL | drop(lhs); @@ -177,9 +177,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:49:10 | LL | fn bitxor, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs ^ rhs; | --- value moved here LL | drop(lhs); @@ -191,7 +191,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitxor, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs ^ rhs; | --- value moved here LL | drop(lhs); @@ -202,9 +202,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:55:10 | LL | fn shl, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs << rhs; | --- value moved here LL | drop(lhs); @@ -216,7 +216,7 @@ error[E0382]: use of moved value: `rhs` LL | fn shl, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs << rhs; | --- value moved here LL | drop(lhs); @@ -227,9 +227,9 @@ error[E0382]: use of moved value: `lhs` --> $DIR/binop-consume-args.rs:61:10 | LL | fn shr, B>(lhs: A, rhs: B) { - | - --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait + | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` LL | lhs >> rhs; | --- value moved here LL | drop(lhs); @@ -241,7 +241,7 @@ error[E0382]: use of moved value: `rhs` LL | fn shr, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `B: Copy` LL | lhs >> rhs; | --- value moved here LL | drop(lhs); diff --git a/src/test/ui/binop/binop-move-semantics.stderr b/src/test/ui/binop/binop-move-semantics.stderr index 897607dc2d8d5..8efb73657fa81 100644 --- a/src/test/ui/binop/binop-move-semantics.stderr +++ b/src/test/ui/binop/binop-move-semantics.stderr @@ -2,9 +2,9 @@ error[E0382]: use of moved value: `x` --> $DIR/binop-move-semantics.rs:8:5 | LL | fn double_move>(x: T) { - | - - move occurs because `x` has type `T`, which does not implement the `Copy` trait + | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` LL | x | - value moved here LL | + @@ -15,9 +15,9 @@ error[E0382]: borrow of moved value: `x` --> $DIR/binop-move-semantics.rs:14:5 | LL | fn move_then_borrow + Clone>(x: T) { - | - - move occurs because `x` has type `T`, which does not implement the `Copy` trait + | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` LL | x | - value moved here LL | + diff --git a/src/test/ui/borrowck/borrowck-unboxed-closures.stderr b/src/test/ui/borrowck/borrowck-unboxed-closures.stderr index 40b8e31348470..5a0afda013ee6 100644 --- a/src/test/ui/borrowck/borrowck-unboxed-closures.stderr +++ b/src/test/ui/borrowck/borrowck-unboxed-closures.stderr @@ -20,9 +20,9 @@ error[E0382]: use of moved value: `f` --> $DIR/borrowck-unboxed-closures.rs:12:5 | LL | fn c isize>(f: F) { - | - - move occurs because `f` has type `F`, which does not implement the `Copy` trait + | -- - move occurs because `f` has type `F`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `F: Copy +` LL | f(1, 2); | - value moved here LL | f(1, 2); diff --git a/src/test/ui/issues/issue-34721.fixed b/src/test/ui/issues/issue-34721.fixed new file mode 100644 index 0000000000000..88f0345bde62a --- /dev/null +++ b/src/test/ui/issues/issue-34721.fixed @@ -0,0 +1,34 @@ +// run-rustfix + +pub trait Foo { + fn zero(self) -> Self; +} + +impl Foo for u32 { + fn zero(self) -> u32 { 0u32 } +} + +pub mod bar { + pub use Foo; + pub fn bar(x: T) -> T { + x.zero() + } +} + +mod baz { + use bar; + use Foo; + pub fn baz(x: T) -> T { + if 0 == 1 { + bar::bar(x.zero()) + } else { + x.zero() + }; + x.zero() + //~^ ERROR use of moved value + } +} + +fn main() { + let _ = baz::baz(0u32); +} diff --git a/src/test/ui/issues/issue-34721.rs b/src/test/ui/issues/issue-34721.rs index bdc9fe43a8bd9..14dd01766aa44 100644 --- a/src/test/ui/issues/issue-34721.rs +++ b/src/test/ui/issues/issue-34721.rs @@ -1,3 +1,5 @@ +// run-rustfix + pub trait Foo { fn zero(self) -> Self; } diff --git a/src/test/ui/issues/issue-34721.stderr b/src/test/ui/issues/issue-34721.stderr index d5cede990a335..abf65afd7ed98 100644 --- a/src/test/ui/issues/issue-34721.stderr +++ b/src/test/ui/issues/issue-34721.stderr @@ -1,10 +1,10 @@ error[E0382]: use of moved value: `x` - --> $DIR/issue-34721.rs:25:9 + --> $DIR/issue-34721.rs:27:9 | LL | pub fn baz(x: T) -> T { - | - - move occurs because `x` has type `T`, which does not implement the `Copy` trait + | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` LL | if 0 == 1 { LL | bar::bar(x.zero()) | - value moved here diff --git a/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr b/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr index 483c364752b7b..ead2cceebf857 100644 --- a/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr +++ b/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr @@ -11,9 +11,9 @@ error[E0382]: borrow of moved value: `f` --> $DIR/moves-based-on-type-no-recursive-stack-closure.rs:32:5 | LL | fn conspirator(mut f: F) where F: FnMut(&mut R, bool) { - | - ----- move occurs because `f` has type `F`, which does not implement the `Copy` trait - | | - | consider adding a `Copy` constraint to this type argument + | ----- - help: consider further restricting type parameter `F`: `, F: Copy` + | | + | move occurs because `f` has type `F`, which does not implement the `Copy` trait LL | let mut r = R {c: Box::new(f)}; | - value moved here LL | f(&mut r, false) diff --git a/src/test/ui/once-cant-call-twice-on-heap.stderr b/src/test/ui/once-cant-call-twice-on-heap.stderr index f98d3d8384537..22f98c2a979b1 100644 --- a/src/test/ui/once-cant-call-twice-on-heap.stderr +++ b/src/test/ui/once-cant-call-twice-on-heap.stderr @@ -2,9 +2,9 @@ error[E0382]: use of moved value: `blk` --> $DIR/once-cant-call-twice-on-heap.rs:9:5 | LL | fn foo(blk: F) { - | - --- move occurs because `blk` has type `F`, which does not implement the `Copy` trait + | -- --- move occurs because `blk` has type `F`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `F: Copy +` LL | blk(); | --- value moved here LL | blk(); diff --git a/src/test/ui/unop-move-semantics.stderr b/src/test/ui/unop-move-semantics.stderr index 6aa3d0b09896b..0dbd3b8cd0d5a 100644 --- a/src/test/ui/unop-move-semantics.stderr +++ b/src/test/ui/unop-move-semantics.stderr @@ -2,9 +2,9 @@ error[E0382]: borrow of moved value: `x` --> $DIR/unop-move-semantics.rs:8:5 | LL | fn move_then_borrow + Clone>(x: T) { - | - - move occurs because `x` has type `T`, which does not implement the `Copy` trait + | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | consider adding a `Copy` constraint to this type argument + | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` LL | !x; | - value moved here LL | From 9fb446d4726c88490c1dd069fa6982c7e5718643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 19 Nov 2019 17:11:55 -0800 Subject: [PATCH 2/3] Deduplicate type param constraint suggestion code --- src/librustc/hir/mod.rs | 77 +++++++++++++++++- src/librustc/traits/error_reporting.rs | 81 ++++--------------- .../borrow_check/conflict_errors.rs | 64 ++------------- src/test/ui/binop/binop-consume-args.stderr | 40 ++++----- src/test/ui/binop/binop-move-semantics.stderr | 4 +- .../borrowck/borrowck-unboxed-closures.stderr | 2 +- ...igher-ranker-supertraits-transitive.stderr | 2 +- .../hrtb-higher-ranker-supertraits.stderr | 4 +- src/test/ui/issues/issue-34721.fixed | 2 +- src/test/ui/issues/issue-34721.stderr | 2 +- .../ui/once-cant-call-twice-on-heap.stderr | 2 +- src/test/ui/unop-move-semantics.stderr | 2 +- 12 files changed, 128 insertions(+), 154 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 66bb3a8d883a4..0b79f6247095b 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -16,9 +16,9 @@ use crate::ty::AdtKind; use crate::ty::query::Providers; use crate::util::nodemap::{NodeMap, FxHashSet}; -use errors::FatalError; +use errors::{Applicability, DiagnosticBuilder, FatalError}; use syntax_pos::{Span, DUMMY_SP, MultiSpan}; -use syntax::source_map::Spanned; +use syntax::source_map::{Spanned, SourceMap}; use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, AsmDialect}; use syntax::ast::{Attribute, Label, LitKind, StrStyle, FloatTy, IntTy, UintTy}; pub use syntax::ast::{Mutability, Constness, Unsafety, Movability, CaptureBy}; @@ -644,6 +644,79 @@ impl Generics { self.params.iter().map(|p| p.span).collect::>().into() } } + + /// Suggest restricting a type param with a new bound. + pub fn suggest_constraining_type_param( + &self, + err: &mut DiagnosticBuilder<'_>, + param_name: &str, + constraint: &str, + source_map: &SourceMap, + span: Span, + ) -> bool { + let restrict_msg = "consider further restricting this bound"; + if let Some(param) = self.params.iter().filter(|p| { + p.name.ident().as_str() == param_name + }).next() { + 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, constraint), + Applicability::MachineApplicable, + ); + } else if self.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, + "consider restricting this bound", + format!("{}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else if !self.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) where T: Debug, T: Trait {}` + err.span_suggestion( + self.where_clause.span().unwrap().shrink_to_hi(), + &format!("consider further restricting type parameter `{}`", param_name), + format!(", {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + // If there is no `where` clause lean towards constraining to the + // type parameter: + // `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 = 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!("{}: {} + ", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + err.span_label( + param.span, + &format!("consider adding a `where {}: {}` bound", param_name, constraint), + ); + } + } + return true; + } + false + } } /// Synthetic type parameters are converted to another form during lowering; this allows diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 65d08ab03aaaf..db7c5c3f77876 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -1091,7 +1091,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } fn suggest_restricting_param_bound( &self, - err: &mut DiagnosticBuilder<'_>, + mut err: &mut DiagnosticBuilder<'_>, trait_ref: &ty::PolyTraitRef<'_>, body_id: hir::HirId, ) { @@ -1102,7 +1102,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { _ => return, }; - let mut suggest_restriction = |generics: &hir::Generics, msg| { + let suggest_restriction = | + generics: &hir::Generics, + msg, + err: &mut DiagnosticBuilder<'_>, + | { let span = generics.where_clause.span_for_predicates_or_empty_place(); if !span.from_expansion() && span.desugaring_kind().is_none() { err.span_suggestion( @@ -1132,7 +1136,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { kind: hir::TraitItemKind::Method(..), .. }) if param_ty && self_ty == self.tcx.types.self_param => { // Restricting `Self` for a single method. - suggest_restriction(&generics, "`Self`"); + suggest_restriction(&generics, "`Self`", err); return; } @@ -1154,7 +1158,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { kind: hir::ItemKind::Impl(_, _, _, generics, ..), .. }) if projection.is_some() => { // Missing associated type bound. - suggest_restriction(&generics, "the associated type"); + suggest_restriction(&generics, "the associated type", err); return; } @@ -1183,68 +1187,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { 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| { - p.name.ident().as_str() == param_name - }) { - 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, - "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) 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(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 { - err.span_label(param.span, &format!( - "consider adding a `where {}` bound", - trait_ref.to_predicate(), - )); - } - } + let constraint = trait_ref.to_string(); + if generics.suggest_constraining_type_param( + &mut err, + ¶m_name, + &constraint, + self.tcx.sess.source_map(), + *span, + ) { return; } } diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index e3da090a62d30..e9065abcebe76 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -233,63 +233,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let generics = tcx.generics_of(self.mir_def_id); let param = generics.type_param(¶m_ty, tcx); let generics = tcx.hir().get_generics(self.mir_def_id).unwrap(); - let msg = "consider adding a `Copy` constraint to this type argument"; - for param in generics.params.iter().filter(|p| { - p.name.ident().as_str() == param.name.as_str() - }) { - let param_name = param.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, - msg, - // `impl CurrentTrait + MissingTrait` - format!("{} + Copy", param_name), - 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, - msg, - format!("{}: Copy", param_name), - 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) where T: Debug, T: Trait {}` - err.span_suggestion( - generics.where_clause.span().unwrap().shrink_to_hi(), - msg, - format!(", {}: Copy", param_name), - Applicability::MachineApplicable, - ); - } else { - // If there is no `where` clause lean towards constraining to the - // type parameter: - // `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 = 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, msg, format!( - "{}: Copy +", - param_name, - ), Applicability::MachineApplicable); - } else { - err.span_label(param.span, msg); - } - } - } + generics.suggest_constraining_type_param( + &mut err, + ¶m.name.as_str(), + "Copy", + tcx.sess.source_map(), + span, + ); } let span = if let Some(local) = place.as_local() { let decl = &self.body.local_decls[local]; diff --git a/src/test/ui/binop/binop-consume-args.stderr b/src/test/ui/binop/binop-consume-args.stderr index 5a6a2f9258393..876e984ecb0bf 100644 --- a/src/test/ui/binop/binop-consume-args.stderr +++ b/src/test/ui/binop/binop-consume-args.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `lhs` LL | fn add, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs + rhs; | --- value moved here LL | drop(lhs); @@ -16,7 +16,7 @@ error[E0382]: use of moved value: `rhs` LL | fn add, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs + rhs; | --- value moved here LL | drop(lhs); @@ -29,7 +29,7 @@ error[E0382]: use of moved value: `lhs` LL | fn sub, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs - rhs; | --- value moved here LL | drop(lhs); @@ -41,7 +41,7 @@ error[E0382]: use of moved value: `rhs` LL | fn sub, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs - rhs; | --- value moved here LL | drop(lhs); @@ -54,7 +54,7 @@ error[E0382]: use of moved value: `lhs` LL | fn mul, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs * rhs; | --- value moved here LL | drop(lhs); @@ -66,7 +66,7 @@ error[E0382]: use of moved value: `rhs` LL | fn mul, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs * rhs; | --- value moved here LL | drop(lhs); @@ -79,7 +79,7 @@ error[E0382]: use of moved value: `lhs` LL | fn div, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs / rhs; | --- value moved here LL | drop(lhs); @@ -91,7 +91,7 @@ error[E0382]: use of moved value: `rhs` LL | fn div, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs / rhs; | --- value moved here LL | drop(lhs); @@ -104,7 +104,7 @@ error[E0382]: use of moved value: `lhs` LL | fn rem, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs % rhs; | --- value moved here LL | drop(lhs); @@ -116,7 +116,7 @@ error[E0382]: use of moved value: `rhs` LL | fn rem, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs % rhs; | --- value moved here LL | drop(lhs); @@ -129,7 +129,7 @@ error[E0382]: use of moved value: `lhs` LL | fn bitand, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs & rhs; | --- value moved here LL | drop(lhs); @@ -141,7 +141,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitand, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs & rhs; | --- value moved here LL | drop(lhs); @@ -154,7 +154,7 @@ error[E0382]: use of moved value: `lhs` LL | fn bitor, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs | rhs; | --- value moved here LL | drop(lhs); @@ -166,7 +166,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitor, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs | rhs; | --- value moved here LL | drop(lhs); @@ -179,7 +179,7 @@ error[E0382]: use of moved value: `lhs` LL | fn bitxor, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs ^ rhs; | --- value moved here LL | drop(lhs); @@ -191,7 +191,7 @@ error[E0382]: use of moved value: `rhs` LL | fn bitxor, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs ^ rhs; | --- value moved here LL | drop(lhs); @@ -204,7 +204,7 @@ error[E0382]: use of moved value: `lhs` LL | fn shl, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs << rhs; | --- value moved here LL | drop(lhs); @@ -216,7 +216,7 @@ error[E0382]: use of moved value: `rhs` LL | fn shl, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs << rhs; | --- value moved here LL | drop(lhs); @@ -229,7 +229,7 @@ error[E0382]: use of moved value: `lhs` LL | fn shr, B>(lhs: A, rhs: B) { | -- --- move occurs because `lhs` has type `A`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `A: Copy +` + | help: consider further restricting this bound: `A: Copy +` LL | lhs >> rhs; | --- value moved here LL | drop(lhs); @@ -241,7 +241,7 @@ error[E0382]: use of moved value: `rhs` LL | fn shr, B>(lhs: A, rhs: B) { | - --- move occurs because `rhs` has type `B`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `B: Copy` + | help: consider restricting this bound: `B: Copy` LL | lhs >> rhs; | --- value moved here LL | drop(lhs); diff --git a/src/test/ui/binop/binop-move-semantics.stderr b/src/test/ui/binop/binop-move-semantics.stderr index 8efb73657fa81..7552dc669749c 100644 --- a/src/test/ui/binop/binop-move-semantics.stderr +++ b/src/test/ui/binop/binop-move-semantics.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `x` LL | fn double_move>(x: T) { | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` + | help: consider further restricting this bound: `T: Copy +` LL | x | - value moved here LL | + @@ -17,7 +17,7 @@ error[E0382]: borrow of moved value: `x` LL | fn move_then_borrow + Clone>(x: T) { | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` + | help: consider further restricting this bound: `T: Copy +` LL | x | - value moved here LL | + diff --git a/src/test/ui/borrowck/borrowck-unboxed-closures.stderr b/src/test/ui/borrowck/borrowck-unboxed-closures.stderr index 5a0afda013ee6..5cd0471cd0d6e 100644 --- a/src/test/ui/borrowck/borrowck-unboxed-closures.stderr +++ b/src/test/ui/borrowck/borrowck-unboxed-closures.stderr @@ -22,7 +22,7 @@ error[E0382]: use of moved value: `f` LL | fn c isize>(f: F) { | -- - move occurs because `f` has type `F`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `F: Copy +` + | help: consider further restricting this bound: `F: Copy +` LL | f(1, 2); | - value moved here LL | f(1, 2); diff --git a/src/test/ui/hrtb/hrtb-higher-ranker-supertraits-transitive.stderr b/src/test/ui/hrtb/hrtb-higher-ranker-supertraits-transitive.stderr index afcb467ad4711..5c38f47b363e9 100644 --- a/src/test/ui/hrtb/hrtb-higher-ranker-supertraits-transitive.stderr +++ b/src/test/ui/hrtb/hrtb-higher-ranker-supertraits-transitive.stderr @@ -7,7 +7,7 @@ LL | where B : for<'ccx> Bar<'ccx> | ------------------- required by this bound in `want_bar_for_any_ccx` ... LL | where B : Qux - | - help: consider further restricting type parameter `B`: `, for<'ccx> B: Bar<'ccx>` + | - help: consider further restricting type parameter `B`: `, B: for<'ccx> Bar<'ccx>` ... LL | want_bar_for_any_ccx(b); | ^ the trait `for<'ccx> Bar<'ccx>` is not implemented for `B` diff --git a/src/test/ui/hrtb/hrtb-higher-ranker-supertraits.stderr b/src/test/ui/hrtb/hrtb-higher-ranker-supertraits.stderr index 20913b4f28c8e..768bc6c71847f 100644 --- a/src/test/ui/hrtb/hrtb-higher-ranker-supertraits.stderr +++ b/src/test/ui/hrtb/hrtb-higher-ranker-supertraits.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `for<'tcx> F: Foo<'tcx>` is not satisfied --> $DIR/hrtb-higher-ranker-supertraits.rs:18:26 | LL | where F : Foo<'x> - | - help: consider further restricting type parameter `F`: `, for<'tcx> F: Foo<'tcx>` + | - help: consider further restricting type parameter `F`: `, F: for<'tcx> Foo<'tcx>` ... LL | want_foo_for_any_tcx(f); | ^ the trait `for<'tcx> Foo<'tcx>` is not implemented for `F` @@ -16,7 +16,7 @@ error[E0277]: the trait bound `for<'ccx> B: Bar<'ccx>` is not satisfied --> $DIR/hrtb-higher-ranker-supertraits.rs:35:26 | LL | where B : Bar<'x> - | - help: consider further restricting type parameter `B`: `, for<'ccx> B: Bar<'ccx>` + | - help: consider further restricting type parameter `B`: `, B: for<'ccx> Bar<'ccx>` ... LL | want_bar_for_any_ccx(b); | ^ the trait `for<'ccx> Bar<'ccx>` is not implemented for `B` diff --git a/src/test/ui/issues/issue-34721.fixed b/src/test/ui/issues/issue-34721.fixed index 88f0345bde62a..ba2810ee3d725 100644 --- a/src/test/ui/issues/issue-34721.fixed +++ b/src/test/ui/issues/issue-34721.fixed @@ -18,7 +18,7 @@ pub mod bar { mod baz { use bar; use Foo; - pub fn baz(x: T) -> T { + pub fn baz(x: T) -> T { if 0 == 1 { bar::bar(x.zero()) } else { diff --git a/src/test/ui/issues/issue-34721.stderr b/src/test/ui/issues/issue-34721.stderr index abf65afd7ed98..3002b07e8c909 100644 --- a/src/test/ui/issues/issue-34721.stderr +++ b/src/test/ui/issues/issue-34721.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `x` LL | pub fn baz(x: T) -> T { | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` + | help: consider further restricting this bound: `T: Copy +` LL | if 0 == 1 { LL | bar::bar(x.zero()) | - value moved here diff --git a/src/test/ui/once-cant-call-twice-on-heap.stderr b/src/test/ui/once-cant-call-twice-on-heap.stderr index 22f98c2a979b1..fd99806021856 100644 --- a/src/test/ui/once-cant-call-twice-on-heap.stderr +++ b/src/test/ui/once-cant-call-twice-on-heap.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `blk` LL | fn foo(blk: F) { | -- --- move occurs because `blk` has type `F`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `F: Copy +` + | help: consider further restricting this bound: `F: Copy +` LL | blk(); | --- value moved here LL | blk(); diff --git a/src/test/ui/unop-move-semantics.stderr b/src/test/ui/unop-move-semantics.stderr index 0dbd3b8cd0d5a..092c419d7cba8 100644 --- a/src/test/ui/unop-move-semantics.stderr +++ b/src/test/ui/unop-move-semantics.stderr @@ -4,7 +4,7 @@ error[E0382]: borrow of moved value: `x` LL | fn move_then_borrow + Clone>(x: T) { | -- - move occurs because `x` has type `T`, which does not implement the `Copy` trait | | - | help: consider adding a `Copy` constraint to this type argument: `T: Copy +` + | help: consider further restricting this bound: `T: Copy +` LL | !x; | - value moved here LL | From 0f530ecb6875540f5cf5032ea7df54a38d01ab1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 23 Nov 2019 21:44:28 -0800 Subject: [PATCH 3/3] review comments --- src/librustc/hir/mod.rs | 77 +------------------ src/librustc/traits/error_reporting.rs | 77 ++++++++++++++++++- .../borrow_check/conflict_errors.rs | 4 +- 3 files changed, 81 insertions(+), 77 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 0b79f6247095b..66bb3a8d883a4 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -16,9 +16,9 @@ use crate::ty::AdtKind; use crate::ty::query::Providers; use crate::util::nodemap::{NodeMap, FxHashSet}; -use errors::{Applicability, DiagnosticBuilder, FatalError}; +use errors::FatalError; use syntax_pos::{Span, DUMMY_SP, MultiSpan}; -use syntax::source_map::{Spanned, SourceMap}; +use syntax::source_map::Spanned; use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, AsmDialect}; use syntax::ast::{Attribute, Label, LitKind, StrStyle, FloatTy, IntTy, UintTy}; pub use syntax::ast::{Mutability, Constness, Unsafety, Movability, CaptureBy}; @@ -644,79 +644,6 @@ impl Generics { self.params.iter().map(|p| p.span).collect::>().into() } } - - /// Suggest restricting a type param with a new bound. - pub fn suggest_constraining_type_param( - &self, - err: &mut DiagnosticBuilder<'_>, - param_name: &str, - constraint: &str, - source_map: &SourceMap, - span: Span, - ) -> bool { - let restrict_msg = "consider further restricting this bound"; - if let Some(param) = self.params.iter().filter(|p| { - p.name.ident().as_str() == param_name - }).next() { - 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, constraint), - Applicability::MachineApplicable, - ); - } else if self.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, - "consider restricting this bound", - format!("{}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } else if !self.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) where T: Debug, T: Trait {}` - err.span_suggestion( - self.where_clause.span().unwrap().shrink_to_hi(), - &format!("consider further restricting type parameter `{}`", param_name), - format!(", {}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } else { - // If there is no `where` clause lean towards constraining to the - // type parameter: - // `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 = 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!("{}: {} + ", param_name, constraint), - Applicability::MachineApplicable, - ); - } else { - err.span_label( - param.span, - &format!("consider adding a `where {}: {}` bound", param_name, constraint), - ); - } - } - return true; - } - false - } } /// Synthetic type parameters are converted to another form during lowering; this allows diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index db7c5c3f77876..6e723cdc999b0 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -39,6 +39,7 @@ use syntax::ast; use syntax::symbol::{sym, kw}; use syntax_pos::{DUMMY_SP, Span, ExpnKind, MultiSpan}; use rustc::hir::def_id::LOCAL_CRATE; +use syntax_pos::source_map::SourceMap; use rustc_error_codes::*; @@ -1189,7 +1190,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // Missing generic type parameter bound. let param_name = self_ty.to_string(); let constraint = trait_ref.to_string(); - if generics.suggest_constraining_type_param( + if suggest_constraining_type_param( + generics, &mut err, ¶m_name, &constraint, @@ -2497,3 +2499,76 @@ impl ArgKind { } } } + +/// Suggest restricting a type param with a new bound. +pub fn suggest_constraining_type_param( + generics: &hir::Generics, + err: &mut DiagnosticBuilder<'_>, + param_name: &str, + constraint: &str, + source_map: &SourceMap, + span: Span, +) -> bool { + let restrict_msg = "consider further restricting this bound"; + if let Some(param) = generics.params.iter().filter(|p| { + p.name.ident().as_str() == param_name + }).next() { + 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, constraint), + 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, + "consider restricting this bound", + format!("{}: {}", param_name, constraint), + 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) 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!(", {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + // If there is no `where` clause lean towards constraining to the + // type parameter: + // `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 = 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!("{}: {} + ", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + err.span_label( + param.span, + &format!("consider adding a `where {}: {}` bound", param_name, constraint), + ); + } + } + return true; + } + false +} diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index e9065abcebe76..48f8ad9bbd8d6 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -7,6 +7,7 @@ use rustc::mir::{ PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, Ty}; +use rustc::traits::error_reporting::suggest_constraining_type_param; use rustc_data_structures::fx::FxHashSet; use rustc_index::vec::Idx; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -233,7 +234,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let generics = tcx.generics_of(self.mir_def_id); let param = generics.type_param(¶m_ty, tcx); let generics = tcx.hir().get_generics(self.mir_def_id).unwrap(); - generics.suggest_constraining_type_param( + suggest_constraining_type_param( + generics, &mut err, ¶m.name.as_str(), "Copy",