From 405a3dd91c4e844652c09810e71183babc9ed160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 15 Oct 2019 18:27:42 -0700 Subject: [PATCH 1/6] Tweak code formatting --- src/librustc/traits/error_reporting.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index c7e51ff321771..697bb3f467c54 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -713,20 +713,24 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } match obligation.predicate { ty::Predicate::Trait(ref trait_predicate) => { - let trait_predicate = - self.resolve_vars_if_possible(trait_predicate); + let trait_predicate = self.resolve_vars_if_possible(trait_predicate); if self.tcx.sess.has_errors() && trait_predicate.references_error() { return; } let trait_ref = trait_predicate.to_poly_trait_ref(); - let (post_message, pre_message) = - self.get_parent_trait_ref(&obligation.cause.code) + let ( + post_message, + pre_message, + ) = self.get_parent_trait_ref(&obligation.cause.code) .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t))) .unwrap_or_default(); - let OnUnimplementedNote { message, label, note } - = self.on_unimplemented_note(trait_ref, obligation); + let OnUnimplementedNote { + message, + label, + note, + } = self.on_unimplemented_note(trait_ref, obligation); let have_alt_message = message.is_some() || label.is_some(); let is_try = self.tcx.sess.source_map().span_to_snippet(span) .map(|s| &s == "?") From f57413b717e12449612068406063571f6a4b43a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 15 Oct 2019 18:42:27 -0700 Subject: [PATCH 2/6] Suggest borrowing when it would satisfy an unmet trait bound When there are multiple implementors for the same trait that is present in an unmet binding, modify the E0277 error to refer to the parent obligation and verify whether borrowing the argument being passed in would satisfy the unmet bound. If it would, suggest it. --- src/libcore/ops/function.rs | 4 - src/librustc/traits/error_reporting.rs | 74 ++++++++++++++++++- src/test/ui/derives/deriving-copyclone.rs | 6 +- src/test/ui/derives/deriving-copyclone.stderr | 15 +++- .../ui/kindck/kindck-impl-type-params-2.rs | 2 +- .../kindck/kindck-impl-type-params-2.stderr | 2 +- .../kindck-inherited-copy-bound.curr.stderr | 2 +- src/test/ui/suggestions/issue-62843.stderr | 8 +- src/test/ui/traits/traits-negative-impls.rs | 4 +- .../ui/traits/traits-negative-impls.stderr | 14 +++- 10 files changed, 107 insertions(+), 24 deletions(-) diff --git a/src/libcore/ops/function.rs b/src/libcore/ops/function.rs index 4a0a2720fe441..35790324a2f62 100644 --- a/src/libcore/ops/function.rs +++ b/src/libcore/ops/function.rs @@ -137,10 +137,6 @@ pub trait Fn : FnMut { #[rustc_paren_sugar] #[rustc_on_unimplemented( on(Args="()", note="wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}"), - on( - all(Args="(char,)", _Self="std::string::String"), - note="borrowing the `{Self}` might fix the problem" - ), message="expected a `{FnMut}<{Args}>` closure, found `{Self}`", label="expected an `FnMut<{Args}>` closure, found `{Self}`", )] diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 697bb3f467c54..f511862b37b84 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -33,7 +33,7 @@ use crate::ty::subst::Subst; use crate::ty::SubtypePredicate; use crate::util::nodemap::{FxHashMap, FxHashSet}; -use errors::{Applicability, DiagnosticBuilder, pluralize}; +use errors::{Applicability, DiagnosticBuilder, pluralise, Style}; use std::fmt; use syntax::ast; use syntax::symbol::{sym, kw}; @@ -723,7 +723,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { post_message, pre_message, ) = self.get_parent_trait_ref(&obligation.cause.code) - .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t))) + .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t))) .unwrap_or_default(); let OnUnimplementedNote { @@ -787,6 +787,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err); self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg); self.suggest_remove_reference(&obligation, &mut err, &trait_ref); + if self.suggest_add_reference_to_arg( + &obligation, + &mut err, + &trait_ref, + points_at_arg, + ) { + self.note_obligation_cause(&mut err, obligation); + err.emit(); + return; + } self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref); // Try to report a help message @@ -1302,6 +1312,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } + fn suggest_add_reference_to_arg( + &self, + obligation: &PredicateObligation<'tcx>, + err: &mut DiagnosticBuilder<'tcx>, + trait_ref: &ty::Binder>, + points_at_arg: bool, + ) -> bool { + if !points_at_arg { + return false; + } + + let span = obligation.cause.span; + let param_env = obligation.param_env; + let trait_ref = trait_ref.skip_binder(); + + if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code { + // Try to apply the original trait binding obligation by borrowing. + let self_ty = trait_ref.self_ty(); + let found = self_ty.to_string(); + let new_self_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self_ty); + let substs = self.tcx.mk_substs_trait(new_self_ty, &[]); + let new_trait_ref = ty::TraitRef::new(obligation.parent_trait_ref.def_id(), substs); + let new_obligation = Obligation::new( + ObligationCause::dummy(), + param_env, + new_trait_ref.to_predicate(), + ); + if self.predicate_may_hold(&new_obligation) { + if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { + // We have a very specific type of error, where just borrowing this argument + // might solve the problem. In cases like this, the important part is the + // original type obligation, not the last one that failed, which is arbitrary. + // Because of this, we modify the error to refer to the original obligation and + // return early in the caller. + err.message = vec![( + format!( + "the trait bound `{}: {}` is not satisfied", + found, + obligation.parent_trait_ref.skip_binder(), + ), + Style::NoStyle, + )]; + if snippet.starts_with('&') { + // This is already a literal borrow and the obligation is failing + // somewhere else in the obligation chain. Do not suggest non-sense. + return false; + } + err.span_suggestion( + span, + "consider borrowing here", + format!("&{}", snippet), + Applicability::MachineApplicable, + ); + return true; + } + } + } + false + } + /// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`, /// suggest removing these references until we reach a type that implements the trait. fn suggest_remove_reference( diff --git a/src/test/ui/derives/deriving-copyclone.rs b/src/test/ui/derives/deriving-copyclone.rs index 4565412bff7f3..06b3157a77a84 100644 --- a/src/test/ui/derives/deriving-copyclone.rs +++ b/src/test/ui/derives/deriving-copyclone.rs @@ -28,10 +28,10 @@ fn main() { is_clone(B { a: 1, b: 2 }); // B cannot be copied or cloned - is_copy(B { a: 1, b: C }); //~ERROR Copy - is_clone(B { a: 1, b: C }); //~ERROR Clone + is_copy(B { a: 1, b: C }); //~ ERROR Copy + is_clone(B { a: 1, b: C }); //~ ERROR Clone // B can be cloned but not copied - is_copy(B { a: 1, b: D }); //~ERROR Copy + is_copy(B { a: 1, b: D }); //~ ERROR Copy is_clone(B { a: 1, b: D }); } diff --git a/src/test/ui/derives/deriving-copyclone.stderr b/src/test/ui/derives/deriving-copyclone.stderr index 4cca14ae089e9..561706469c648 100644 --- a/src/test/ui/derives/deriving-copyclone.stderr +++ b/src/test/ui/derives/deriving-copyclone.stderr @@ -5,7 +5,10 @@ LL | fn is_copy(_: T) {} | ------- ---- required by this bound in `is_copy` ... LL | is_copy(B { a: 1, b: C }); - | ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `C` + | ^^^^^^^^^^^^^^^^ + | | + | the trait `std::marker::Copy` is not implemented for `C` + | help: consider borrowing here: `&B { a: 1, b: C }` | = note: required because of the requirements on the impl of `std::marker::Copy` for `B` @@ -16,7 +19,10 @@ LL | fn is_clone(_: T) {} | -------- ----- required by this bound in `is_clone` ... LL | is_clone(B { a: 1, b: C }); - | ^^^^^^^^^^^^^^^^ the trait `std::clone::Clone` is not implemented for `C` + | ^^^^^^^^^^^^^^^^ + | | + | the trait `std::clone::Clone` is not implemented for `C` + | help: consider borrowing here: `&B { a: 1, b: C }` | = note: required because of the requirements on the impl of `std::clone::Clone` for `B` @@ -27,7 +33,10 @@ LL | fn is_copy(_: T) {} | ------- ---- required by this bound in `is_copy` ... LL | is_copy(B { a: 1, b: D }); - | ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `D` + | ^^^^^^^^^^^^^^^^ + | | + | the trait `std::marker::Copy` is not implemented for `D` + | help: consider borrowing here: `&B { a: 1, b: D }` | = note: required because of the requirements on the impl of `std::marker::Copy` for `B` diff --git a/src/test/ui/kindck/kindck-impl-type-params-2.rs b/src/test/ui/kindck/kindck-impl-type-params-2.rs index ac9cc1a08f33d..d5fcc68a759cb 100644 --- a/src/test/ui/kindck/kindck-impl-type-params-2.rs +++ b/src/test/ui/kindck/kindck-impl-type-params-2.rs @@ -11,5 +11,5 @@ fn take_param(foo: &T) { } fn main() { let x: Box<_> = box 3; take_param(&x); - //~^ ERROR `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied + //~^ ERROR the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied } diff --git a/src/test/ui/kindck/kindck-impl-type-params-2.stderr b/src/test/ui/kindck/kindck-impl-type-params-2.stderr index 8e98911324411..318b7b0f10a0a 100644 --- a/src/test/ui/kindck/kindck-impl-type-params-2.stderr +++ b/src/test/ui/kindck/kindck-impl-type-params-2.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied +error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied --> $DIR/kindck-impl-type-params-2.rs:13:16 | LL | fn take_param(foo: &T) { } diff --git a/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr b/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr index a93f4686496ac..da1a7a7520e07 100644 --- a/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr +++ b/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied +error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied --> $DIR/kindck-inherited-copy-bound.rs:21:16 | LL | fn take_param(foo: &T) { } diff --git a/src/test/ui/suggestions/issue-62843.stderr b/src/test/ui/suggestions/issue-62843.stderr index b5801e9162fb3..9535bf1b2ced5 100644 --- a/src/test/ui/suggestions/issue-62843.stderr +++ b/src/test/ui/suggestions/issue-62843.stderr @@ -1,11 +1,13 @@ -error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string::String` +error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied --> $DIR/issue-62843.rs:4:32 | LL | println!("{:?}", line.find(pattern)); - | ^^^^^^^ expected an `FnMut<(char,)>` closure, found `std::string::String` + | ^^^^^^^ + | | + | expected an `FnMut<(char,)>` closure, found `std::string::String` + | help: consider borrowing here: `&pattern` | = help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String` - = note: borrowing the `std::string::String` might fix the problem = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String` error: aborting due to previous error diff --git a/src/test/ui/traits/traits-negative-impls.rs b/src/test/ui/traits/traits-negative-impls.rs index fb9a3a99748d0..8ed39986781cd 100644 --- a/src/test/ui/traits/traits-negative-impls.rs +++ b/src/test/ui/traits/traits-negative-impls.rs @@ -46,7 +46,7 @@ fn dummy2() { impl !Send for TestType {} is_send(Box::new(TestType)); - //~^ ERROR `dummy2::TestType` cannot be sent between threads safely + //~^ ERROR the trait bound `dummy2::TestType: std::marker::Send` is not satisfied } fn dummy3() { @@ -64,5 +64,5 @@ fn main() { // This will complain about a missing Send impl because `Sync` is implement *just* // for T that are `Send`. Look at #20366 and #19950 is_sync(Outer2(TestType)); - //~^ ERROR `main::TestType` cannot be sent between threads safely + //~^ ERROR the trait bound `main::TestType: std::marker::Sync` is not satisfied } diff --git a/src/test/ui/traits/traits-negative-impls.stderr b/src/test/ui/traits/traits-negative-impls.stderr index 22b6d2a0c4e98..448bb90205d03 100644 --- a/src/test/ui/traits/traits-negative-impls.stderr +++ b/src/test/ui/traits/traits-negative-impls.stderr @@ -43,14 +43,17 @@ LL | is_send((8, TestType)); = help: within `({integer}, dummy1c::TestType)`, the trait `std::marker::Send` is not implemented for `dummy1c::TestType` = note: required because it appears within the type `({integer}, dummy1c::TestType)` -error[E0277]: `dummy2::TestType` cannot be sent between threads safely +error[E0277]: the trait bound `dummy2::TestType: std::marker::Send` is not satisfied --> $DIR/traits-negative-impls.rs:48:13 | LL | fn is_send(_: T) {} | ------- ---- required by this bound in `is_send` ... LL | is_send(Box::new(TestType)); - | ^^^^^^^^^^^^^^^^^^ `dummy2::TestType` cannot be sent between threads safely + | ^^^^^^^^^^^^^^^^^^ + | | + | `dummy2::TestType` cannot be sent between threads safely + | help: consider borrowing here: `&Box::new(TestType)` | = help: the trait `std::marker::Send` is not implemented for `dummy2::TestType` = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique` @@ -70,14 +73,17 @@ LL | is_send(Box::new(Outer2(TestType))); = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique>` = note: required because it appears within the type `std::boxed::Box>` -error[E0277]: `main::TestType` cannot be sent between threads safely +error[E0277]: the trait bound `main::TestType: std::marker::Sync` is not satisfied --> $DIR/traits-negative-impls.rs:66:13 | LL | fn is_sync(_: T) {} | ------- ---- required by this bound in `is_sync` ... LL | is_sync(Outer2(TestType)); - | ^^^^^^^^^^^^^^^^ `main::TestType` cannot be sent between threads safely + | ^^^^^^^^^^^^^^^^ + | | + | `main::TestType` cannot be sent between threads safely + | help: consider borrowing here: `&Outer2(TestType)` | = help: the trait `std::marker::Send` is not implemented for `main::TestType` = note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2` From 970503b2e10be7c1b6aad7d3ccdcb6a2cc1ec8c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 15 Oct 2019 19:09:15 -0700 Subject: [PATCH 3/6] Modify primary label message to be inline with error message --- src/librustc/traits/error_reporting.rs | 5 +++++ src/libsyntax_pos/lib.rs | 5 +++++ src/test/ui/derives/deriving-copyclone.stderr | 6 +++--- src/test/ui/suggestions/issue-62843.stderr | 2 +- src/test/ui/traits/traits-negative-impls.stderr | 4 ++-- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index f511862b37b84..76bb28f7e9487 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -1359,6 +1359,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // somewhere else in the obligation chain. Do not suggest non-sense. return false; } + err.span.clear_span_labels(); + err.span_label(span, &format!( + "expected an implementor of trait `{}`", + obligation.parent_trait_ref.skip_binder(), + )); err.span_suggestion( span, "consider borrowing here", diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index b88d6dbc3f379..fc8c72e4f2320 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -736,6 +736,11 @@ impl MultiSpan { replacements_occurred } + /// This should be *rarely* used. Remove all the labels in this `MultiSpan`. + pub fn clear_span_labels(&mut self) { + self.span_labels.clear(); + } + /// Returns the strings to highlight. We always ensure that there /// is an entry for each of the primary spans -- for each primary /// span `P`, if there is at least one label with span `P`, we return diff --git a/src/test/ui/derives/deriving-copyclone.stderr b/src/test/ui/derives/deriving-copyclone.stderr index 561706469c648..e23d48ca6304b 100644 --- a/src/test/ui/derives/deriving-copyclone.stderr +++ b/src/test/ui/derives/deriving-copyclone.stderr @@ -7,7 +7,7 @@ LL | fn is_copy(_: T) {} LL | is_copy(B { a: 1, b: C }); | ^^^^^^^^^^^^^^^^ | | - | the trait `std::marker::Copy` is not implemented for `C` + | expected an implementor of trait `std::marker::Copy` | help: consider borrowing here: `&B { a: 1, b: C }` | = note: required because of the requirements on the impl of `std::marker::Copy` for `B` @@ -21,7 +21,7 @@ LL | fn is_clone(_: T) {} LL | is_clone(B { a: 1, b: C }); | ^^^^^^^^^^^^^^^^ | | - | the trait `std::clone::Clone` is not implemented for `C` + | expected an implementor of trait `std::clone::Clone` | help: consider borrowing here: `&B { a: 1, b: C }` | = note: required because of the requirements on the impl of `std::clone::Clone` for `B` @@ -35,7 +35,7 @@ LL | fn is_copy(_: T) {} LL | is_copy(B { a: 1, b: D }); | ^^^^^^^^^^^^^^^^ | | - | the trait `std::marker::Copy` is not implemented for `D` + | expected an implementor of trait `std::marker::Copy` | help: consider borrowing here: `&B { a: 1, b: D }` | = note: required because of the requirements on the impl of `std::marker::Copy` for `B` diff --git a/src/test/ui/suggestions/issue-62843.stderr b/src/test/ui/suggestions/issue-62843.stderr index 9535bf1b2ced5..ecaa39dbd2c8b 100644 --- a/src/test/ui/suggestions/issue-62843.stderr +++ b/src/test/ui/suggestions/issue-62843.stderr @@ -4,7 +4,7 @@ error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<' LL | println!("{:?}", line.find(pattern)); | ^^^^^^^ | | - | expected an `FnMut<(char,)>` closure, found `std::string::String` + | expected an implementor of trait `std::str::pattern::Pattern<'_>` | help: consider borrowing here: `&pattern` | = help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String` diff --git a/src/test/ui/traits/traits-negative-impls.stderr b/src/test/ui/traits/traits-negative-impls.stderr index 448bb90205d03..cfb8b1d7b5f3c 100644 --- a/src/test/ui/traits/traits-negative-impls.stderr +++ b/src/test/ui/traits/traits-negative-impls.stderr @@ -52,7 +52,7 @@ LL | fn is_send(_: T) {} LL | is_send(Box::new(TestType)); | ^^^^^^^^^^^^^^^^^^ | | - | `dummy2::TestType` cannot be sent between threads safely + | expected an implementor of trait `std::marker::Send` | help: consider borrowing here: `&Box::new(TestType)` | = help: the trait `std::marker::Send` is not implemented for `dummy2::TestType` @@ -82,7 +82,7 @@ LL | fn is_sync(_: T) {} LL | is_sync(Outer2(TestType)); | ^^^^^^^^^^^^^^^^ | | - | `main::TestType` cannot be sent between threads safely + | expected an implementor of trait `std::marker::Sync` | help: consider borrowing here: `&Outer2(TestType)` | = help: the trait `std::marker::Send` is not implemented for `main::TestType` From 0f7f2346a688466624d10bfa8310b8d6aedd0ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 15 Oct 2019 20:35:54 -0700 Subject: [PATCH 4/6] Remove unnecessary note --- src/librustc/traits/error_reporting.rs | 21 +++++++++---------- src/libsyntax_pos/lib.rs | 5 ----- src/test/ui/suggestions/issue-62843.stderr | 1 - .../ui/traits/traits-negative-impls.stderr | 2 -- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 76bb28f7e9487..ffd4560d32d8e 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -771,6 +771,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ) }; + if self.suggest_add_reference_to_arg( + &obligation, + &mut err, + &trait_ref, + points_at_arg, + ) { + self.note_obligation_cause(&mut err, obligation); + err.emit(); + return; + } if let Some(ref s) = label { // If it has a custom `#[rustc_on_unimplemented]` // error message, let's display it as the label! @@ -787,16 +797,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err); self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg); self.suggest_remove_reference(&obligation, &mut err, &trait_ref); - if self.suggest_add_reference_to_arg( - &obligation, - &mut err, - &trait_ref, - points_at_arg, - ) { - self.note_obligation_cause(&mut err, obligation); - err.emit(); - return; - } self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref); // Try to report a help message @@ -1359,7 +1359,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // somewhere else in the obligation chain. Do not suggest non-sense. return false; } - err.span.clear_span_labels(); err.span_label(span, &format!( "expected an implementor of trait `{}`", obligation.parent_trait_ref.skip_binder(), diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index fc8c72e4f2320..b88d6dbc3f379 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -736,11 +736,6 @@ impl MultiSpan { replacements_occurred } - /// This should be *rarely* used. Remove all the labels in this `MultiSpan`. - pub fn clear_span_labels(&mut self) { - self.span_labels.clear(); - } - /// Returns the strings to highlight. We always ensure that there /// is an entry for each of the primary spans -- for each primary /// span `P`, if there is at least one label with span `P`, we return diff --git a/src/test/ui/suggestions/issue-62843.stderr b/src/test/ui/suggestions/issue-62843.stderr index ecaa39dbd2c8b..1e0ae9bd06228 100644 --- a/src/test/ui/suggestions/issue-62843.stderr +++ b/src/test/ui/suggestions/issue-62843.stderr @@ -7,7 +7,6 @@ LL | println!("{:?}", line.find(pattern)); | expected an implementor of trait `std::str::pattern::Pattern<'_>` | help: consider borrowing here: `&pattern` | - = help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String` = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String` error: aborting due to previous error diff --git a/src/test/ui/traits/traits-negative-impls.stderr b/src/test/ui/traits/traits-negative-impls.stderr index cfb8b1d7b5f3c..f68cc6a370496 100644 --- a/src/test/ui/traits/traits-negative-impls.stderr +++ b/src/test/ui/traits/traits-negative-impls.stderr @@ -55,7 +55,6 @@ LL | is_send(Box::new(TestType)); | expected an implementor of trait `std::marker::Send` | help: consider borrowing here: `&Box::new(TestType)` | - = help: the trait `std::marker::Send` is not implemented for `dummy2::TestType` = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique` = note: required because it appears within the type `std::boxed::Box` @@ -85,7 +84,6 @@ LL | is_sync(Outer2(TestType)); | expected an implementor of trait `std::marker::Sync` | help: consider borrowing here: `&Outer2(TestType)` | - = help: the trait `std::marker::Send` is not implemented for `main::TestType` = note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2` error: aborting due to 7 previous errors From 7a2f3ee73f6286744ff7b36324ddb9fb5dca62ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 27 Oct 2019 13:42:20 -0700 Subject: [PATCH 5/6] Account for `rustc_on_unimplemented` --- src/librustc/traits/error_reporting.rs | 20 +++++++++++-------- ...copy-bound.object_safe_for_dispatch.stderr | 2 +- src/test/ui/suggestions/issue-62843.stderr | 3 ++- src/test/ui/traits/traits-negative-impls.rs | 4 ++-- .../ui/traits/traits-negative-impls.stderr | 6 ++++-- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index ffd4560d32d8e..1321d5348a299 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -776,6 +776,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { &mut err, &trait_ref, points_at_arg, + have_alt_message, ) { self.note_obligation_cause(&mut err, obligation); err.emit(); @@ -1318,6 +1319,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'tcx>, trait_ref: &ty::Binder>, points_at_arg: bool, + has_custom_message: bool, ) -> bool { if !points_at_arg { return false; @@ -1346,14 +1348,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // original type obligation, not the last one that failed, which is arbitrary. // Because of this, we modify the error to refer to the original obligation and // return early in the caller. - err.message = vec![( - format!( - "the trait bound `{}: {}` is not satisfied", - found, - obligation.parent_trait_ref.skip_binder(), - ), - Style::NoStyle, - )]; + let msg = format!( + "the trait bound `{}: {}` is not satisfied", + found, + obligation.parent_trait_ref.skip_binder(), + ); + if has_custom_message { + err.note(&msg); + } else { + err.message = vec![(msg, Style::NoStyle)]; + } if snippet.starts_with('&') { // This is already a literal borrow and the obligation is failing // somewhere else in the obligation chain. Do not suggest non-sense. diff --git a/src/test/ui/kindck/kindck-inherited-copy-bound.object_safe_for_dispatch.stderr b/src/test/ui/kindck/kindck-inherited-copy-bound.object_safe_for_dispatch.stderr index 7c67c5f9e9596..f272f829ba600 100644 --- a/src/test/ui/kindck/kindck-inherited-copy-bound.object_safe_for_dispatch.stderr +++ b/src/test/ui/kindck/kindck-inherited-copy-bound.object_safe_for_dispatch.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied +error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied --> $DIR/kindck-inherited-copy-bound.rs:21:16 | LL | fn take_param(foo: &T) { } diff --git a/src/test/ui/suggestions/issue-62843.stderr b/src/test/ui/suggestions/issue-62843.stderr index 1e0ae9bd06228..3b7f85c56689e 100644 --- a/src/test/ui/suggestions/issue-62843.stderr +++ b/src/test/ui/suggestions/issue-62843.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied +error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string::String` --> $DIR/issue-62843.rs:4:32 | LL | println!("{:?}", line.find(pattern)); @@ -7,6 +7,7 @@ LL | println!("{:?}", line.find(pattern)); | expected an implementor of trait `std::str::pattern::Pattern<'_>` | help: consider borrowing here: `&pattern` | + = note: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String` error: aborting due to previous error diff --git a/src/test/ui/traits/traits-negative-impls.rs b/src/test/ui/traits/traits-negative-impls.rs index 8ed39986781cd..fb9a3a99748d0 100644 --- a/src/test/ui/traits/traits-negative-impls.rs +++ b/src/test/ui/traits/traits-negative-impls.rs @@ -46,7 +46,7 @@ fn dummy2() { impl !Send for TestType {} is_send(Box::new(TestType)); - //~^ ERROR the trait bound `dummy2::TestType: std::marker::Send` is not satisfied + //~^ ERROR `dummy2::TestType` cannot be sent between threads safely } fn dummy3() { @@ -64,5 +64,5 @@ fn main() { // This will complain about a missing Send impl because `Sync` is implement *just* // for T that are `Send`. Look at #20366 and #19950 is_sync(Outer2(TestType)); - //~^ ERROR the trait bound `main::TestType: std::marker::Sync` is not satisfied + //~^ ERROR `main::TestType` cannot be sent between threads safely } diff --git a/src/test/ui/traits/traits-negative-impls.stderr b/src/test/ui/traits/traits-negative-impls.stderr index f68cc6a370496..599bbfe222546 100644 --- a/src/test/ui/traits/traits-negative-impls.stderr +++ b/src/test/ui/traits/traits-negative-impls.stderr @@ -43,7 +43,7 @@ LL | is_send((8, TestType)); = help: within `({integer}, dummy1c::TestType)`, the trait `std::marker::Send` is not implemented for `dummy1c::TestType` = note: required because it appears within the type `({integer}, dummy1c::TestType)` -error[E0277]: the trait bound `dummy2::TestType: std::marker::Send` is not satisfied +error[E0277]: `dummy2::TestType` cannot be sent between threads safely --> $DIR/traits-negative-impls.rs:48:13 | LL | fn is_send(_: T) {} @@ -55,6 +55,7 @@ LL | is_send(Box::new(TestType)); | expected an implementor of trait `std::marker::Send` | help: consider borrowing here: `&Box::new(TestType)` | + = note: the trait bound `dummy2::TestType: std::marker::Send` is not satisfied = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique` = note: required because it appears within the type `std::boxed::Box` @@ -72,7 +73,7 @@ LL | is_send(Box::new(Outer2(TestType))); = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique>` = note: required because it appears within the type `std::boxed::Box>` -error[E0277]: the trait bound `main::TestType: std::marker::Sync` is not satisfied +error[E0277]: `main::TestType` cannot be sent between threads safely --> $DIR/traits-negative-impls.rs:66:13 | LL | fn is_sync(_: T) {} @@ -84,6 +85,7 @@ LL | is_sync(Outer2(TestType)); | expected an implementor of trait `std::marker::Sync` | help: consider borrowing here: `&Outer2(TestType)` | + = note: the trait bound `main::TestType: std::marker::Sync` is not satisfied = note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2` error: aborting due to 7 previous errors From 2fe8371268b36193fa4dc8461341db90f4ec96b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 6 Nov 2019 10:39:57 -0800 Subject: [PATCH 6/6] review comments and fix rebase --- src/librustc/traits/error_reporting.rs | 6 +++--- src/librustc_errors/lib.rs | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 1321d5348a299..ea29cc0d93f53 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -33,7 +33,7 @@ use crate::ty::subst::Subst; use crate::ty::SubtypePredicate; use crate::util::nodemap::{FxHashMap, FxHashSet}; -use errors::{Applicability, DiagnosticBuilder, pluralise, Style}; +use errors::{Applicability, DiagnosticBuilder, pluralize, Style}; use std::fmt; use syntax::ast; use syntax::symbol::{sym, kw}; @@ -1341,7 +1341,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { param_env, new_trait_ref.to_predicate(), ); - if self.predicate_may_hold(&new_obligation) { + if self.predicate_must_hold_modulo_regions(&new_obligation) { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { // We have a very specific type of error, where just borrowing this argument // might solve the problem. In cases like this, the important part is the @@ -1371,7 +1371,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { span, "consider borrowing here", format!("&{}", snippet), - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); return true; } diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 17765ef9deefa..fb34d844fdafb 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -38,6 +38,7 @@ pub mod registry; mod styled_buffer; mod lock; pub mod json; +pub use snippet::Style; pub type PResult<'a, T> = Result>;