From d58d52a39759960009ca2817f7edcdf22d8722df Mon Sep 17 00:00:00 2001 From: Jakob Degen Date: Thu, 11 Nov 2021 18:28:19 -0500 Subject: [PATCH] Fix handling of substitutions and binders when deciding whether to suggest references When suggesting references, substitutions were being forgotten and some types were misused. This led to at least one ICE and other incorrectly emitted diagnostics. This has been fixed; in some cases this leads to diagnostics changing, and tests have been adjusted. --- .../src/traits/error_reporting/suggestions.rs | 59 +++++++------------ src/test/ui/derives/deriving-copyclone.stderr | 6 +- .../negated-auto-traits-error.stderr | 12 +--- ...ue-90804-incorrect-reference-suggestion.rs | 11 ++++ ...0804-incorrect-reference-suggestion.stderr | 17 ++++++ 5 files changed, 55 insertions(+), 50 deletions(-) create mode 100644 src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.rs create mode 100644 src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index b21936a00b04f..89aa507061af0 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -706,36 +706,29 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } let param_env = obligation.param_env; - let trait_ref = poly_trait_ref.skip_binder(); - - let found_ty = trait_ref.self_ty(); - let found_ty_str = found_ty.to_string(); - let imm_borrowed_found_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, found_ty); - let imm_substs = self.tcx.mk_substs_trait(imm_borrowed_found_ty, &[]); - let mut_borrowed_found_ty = self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, found_ty); - let mut_substs = self.tcx.mk_substs_trait(mut_borrowed_found_ty, &[]); // Try to apply the original trait binding obligation by borrowing. - let mut try_borrowing = |new_imm_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, - new_mut_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, - expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + let mut try_borrowing = |old_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, blacklist: &[DefId]| -> bool { - if blacklist.contains(&expected_trait_ref.def_id()) { + if blacklist.contains(&old_ref.def_id()) { return false; } - let imm_result = self.predicate_must_hold_modulo_regions(&Obligation::new( - ObligationCause::dummy(), - param_env, - new_imm_trait_ref.without_const().to_predicate(self.tcx), - )); - - let mut_result = self.predicate_must_hold_modulo_regions(&Obligation::new( - ObligationCause::dummy(), - param_env, - new_mut_trait_ref.without_const().to_predicate(self.tcx), - )); + let orig_ty = old_ref.self_ty().skip_binder(); + let mk_result = |new_ty| { + let new_ref = old_ref.rebind(ty::TraitRef::new( + old_ref.def_id(), + self.tcx.mk_substs_trait(new_ty, &old_ref.skip_binder().substs[1..]), + )); + self.predicate_must_hold_modulo_regions(&Obligation::new( + ObligationCause::dummy(), + param_env, + new_ref.without_const().to_predicate(self.tcx), + )) + }; + let imm_result = mk_result(self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, orig_ty)); + let mut_result = mk_result(self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, orig_ty)); if imm_result || mut_result { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { @@ -747,8 +740,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let msg = format!( "the trait bound `{}: {}` is not satisfied", - found_ty_str, - expected_trait_ref.print_only_trait_path(), + orig_ty.to_string(), + old_ref.print_only_trait_path(), ); if has_custom_message { err.note(&msg); @@ -764,7 +757,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { span, &format!( "expected an implementor of trait `{}`", - expected_trait_ref.print_only_trait_path(), + old_ref.print_only_trait_path(), ), ); @@ -807,21 +800,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; if let ObligationCauseCode::ImplDerivedObligation(obligation) = &*code { - let expected_trait_ref = obligation.parent_trait_ref; - let new_imm_trait_ref = poly_trait_ref - .rebind(ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs)); - let new_mut_trait_ref = poly_trait_ref - .rebind(ty::TraitRef::new(obligation.parent_trait_ref.def_id(), mut_substs)); - return try_borrowing(new_imm_trait_ref, new_mut_trait_ref, expected_trait_ref, &[]); + try_borrowing(obligation.parent_trait_ref, &[]) } else if let ObligationCauseCode::BindingObligation(_, _) | ObligationCauseCode::ItemObligation(_) = &*code { - return try_borrowing( - poly_trait_ref.rebind(ty::TraitRef::new(trait_ref.def_id, imm_substs)), - poly_trait_ref.rebind(ty::TraitRef::new(trait_ref.def_id, mut_substs)), - *poly_trait_ref, - &never_suggest_borrow[..], - ); + try_borrowing(*poly_trait_ref, &never_suggest_borrow[..]) } else { false } diff --git a/src/test/ui/derives/deriving-copyclone.stderr b/src/test/ui/derives/deriving-copyclone.stderr index 1d0554ad0477f..13097edf0adf9 100644 --- a/src/test/ui/derives/deriving-copyclone.stderr +++ b/src/test/ui/derives/deriving-copyclone.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `C: Copy` is not satisfied +error[E0277]: the trait bound `B: Copy` is not satisfied --> $DIR/deriving-copyclone.rs:31:13 | LL | is_copy(B { a: 1, b: C }); @@ -22,7 +22,7 @@ help: consider borrowing here LL | is_copy(&B { a: 1, b: C }); | + -error[E0277]: the trait bound `C: Clone` is not satisfied +error[E0277]: the trait bound `B: Clone` is not satisfied --> $DIR/deriving-copyclone.rs:32:14 | LL | is_clone(B { a: 1, b: C }); @@ -46,7 +46,7 @@ help: consider borrowing here LL | is_clone(&B { a: 1, b: C }); | + -error[E0277]: the trait bound `D: Copy` is not satisfied +error[E0277]: the trait bound `B: Copy` is not satisfied --> $DIR/deriving-copyclone.rs:35:13 | LL | is_copy(B { a: 1, b: D }); diff --git a/src/test/ui/traits/negative-impls/negated-auto-traits-error.stderr b/src/test/ui/traits/negative-impls/negated-auto-traits-error.stderr index 1cf73fcdebd0b..1ab130e0ab143 100644 --- a/src/test/ui/traits/negative-impls/negated-auto-traits-error.stderr +++ b/src/test/ui/traits/negative-impls/negated-auto-traits-error.stderr @@ -65,7 +65,7 @@ LL | is_send(Box::new(TestType)); | | | required by a bound introduced by this call | - = note: the trait bound `dummy2::TestType: Send` is not satisfied + = note: the trait bound `Unique: Send` is not satisfied = note: required because of the requirements on the impl of `Send` for `Unique` = note: required because it appears within the type `Box` note: required by a bound in `is_send` @@ -104,11 +104,11 @@ error[E0277]: `main::TestType` cannot be sent between threads safely --> $DIR/negated-auto-traits-error.rs:66:13 | LL | is_sync(Outer2(TestType)); - | ------- ^^^^^^^^^^^^^^^^ expected an implementor of trait `Sync` + | ------- ^^^^^^^^^^^^^^^^ `main::TestType` cannot be sent between threads safely | | | required by a bound introduced by this call | - = note: the trait bound `main::TestType: Sync` is not satisfied + = help: the trait `Send` is not implemented for `main::TestType` note: required because of the requirements on the impl of `Sync` for `Outer2` --> $DIR/negated-auto-traits-error.rs:14:22 | @@ -119,12 +119,6 @@ note: required by a bound in `is_sync` | LL | fn is_sync(_: T) {} | ^^^^ required by this bound in `is_sync` -help: consider borrowing here - | -LL | is_sync(&Outer2(TestType)); - | + -LL | is_sync(&mut Outer2(TestType)); - | ++++ error: aborting due to 7 previous errors diff --git a/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.rs b/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.rs new file mode 100644 index 0000000000000..f891a42fc2af0 --- /dev/null +++ b/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.rs @@ -0,0 +1,11 @@ +// Do not suggest referencing the parameter to `check` + +trait Marker {} + +impl Marker for T {} + +pub fn check>(_: T) {} + +pub fn main() { + check::<()>(()); //~ ERROR [E0277] +} diff --git a/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.stderr b/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.stderr new file mode 100644 index 0000000000000..08eab0253701e --- /dev/null +++ b/src/test/ui/typeck/issue-90804-incorrect-reference-suggestion.stderr @@ -0,0 +1,17 @@ +error[E0277]: the trait bound `(): Marker` is not satisfied + --> $DIR/issue-90804-incorrect-reference-suggestion.rs:10:17 + | +LL | check::<()>(()); + | ----------- ^^ the trait `Marker` is not implemented for `()` + | | + | required by a bound introduced by this call + | +note: required by a bound in `check` + --> $DIR/issue-90804-incorrect-reference-suggestion.rs:7:17 + | +LL | pub fn check>(_: T) {} + | ^^^^^^^^^^^ required by this bound in `check` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`.