From 730c6f3e57a4068547d2d08e7a84efea215cd1ee Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 27 Apr 2020 12:28:24 -0700 Subject: [PATCH 1/7] Preserve substitutions when trying to prove trait obligation `mk_obligation_for_def_id` is only correct if the trait and self type do not have any substitutions. Use a different method, `mk_trait_obligation_with_new_self_ty` that is more clear about what is happening. --- .../traits/error_reporting/mod.rs | 27 +++++++----- .../traits/error_reporting/suggestions.rs | 41 +++++++++---------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 2aef8aaf0e303..20de0b786ed70 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -1000,12 +1000,11 @@ trait InferCtxtPrivExt<'tcx> { trait_ref: &ty::PolyTraitRef<'tcx>, ); - fn mk_obligation_for_def_id( + fn mk_trait_obligation_with_new_self_ty( &self, - def_id: DefId, - output_ty: Ty<'tcx>, - cause: ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, + trait_ref: &ty::PolyTraitRef<'tcx>, + new_self_ty: Ty<'tcx>, ) -> PredicateObligation<'tcx>; fn maybe_report_ambiguity( @@ -1380,16 +1379,22 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { } } - fn mk_obligation_for_def_id( + fn mk_trait_obligation_with_new_self_ty( &self, - def_id: DefId, - output_ty: Ty<'tcx>, - cause: ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, + trait_ref: &ty::PolyTraitRef<'tcx>, + new_self_ty: Ty<'tcx>, ) -> PredicateObligation<'tcx> { - let new_trait_ref = - ty::TraitRef { def_id, substs: self.tcx.mk_substs_trait(output_ty, &[]) }; - Obligation::new(cause, param_env, new_trait_ref.without_const().to_predicate(self.tcx)) + let trait_ref = trait_ref.map_bound_ref(|tr| ty::TraitRef { + substs: self.tcx.mk_substs_trait(new_self_ty, &tr.substs[1..]), + ..*tr + }); + + Obligation::new( + ObligationCause::dummy(), + param_env, + trait_ref.without_const().to_predicate(self.tcx), + ) } fn maybe_report_ambiguity( diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 0760bd523b7c3..6712ddf0ae544 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -532,14 +532,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; let msg = format!("use parentheses to call the {}", callable); - let obligation = self.mk_obligation_for_def_id( - trait_ref.def_id(), - output_ty.skip_binder(), - obligation.cause.clone(), + let new_obligation = self.mk_trait_obligation_with_new_self_ty( obligation.param_env, + trait_ref, + output_ty.skip_binder(), ); - match self.evaluate_obligation(&obligation) { + match self.evaluate_obligation(&new_obligation) { Ok( EvaluationResult::EvaluatedToOk | EvaluationResult::EvaluatedToOkModuloRegions @@ -694,7 +693,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, trait_ref: &ty::Binder>, ) { - let trait_ref = trait_ref.skip_binder(); let span = obligation.cause.span; if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { @@ -705,17 +703,16 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { return; } - let mut trait_type = trait_ref.self_ty(); + let mut suggested_ty = trait_ref.self_ty(); for refs_remaining in 0..refs_number { - if let ty::Ref(_, t_type, _) = trait_type.kind { - trait_type = t_type; + if let ty::Ref(_, inner_ty, _) = suggested_ty.kind { + suggested_ty = inner_ty; - let new_obligation = self.mk_obligation_for_def_id( - trait_ref.def_id, - trait_type, - ObligationCause::dummy(), + let new_obligation = self.mk_trait_obligation_with_new_self_ty( obligation.param_env, + trait_ref, + suggested_ty, ); if self.predicate_may_hold(&new_obligation) { @@ -782,20 +779,20 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { return; } - let trait_type = match mutability { + let suggested_ty = match mutability { hir::Mutability::Mut => self.tcx.mk_imm_ref(region, t_type), hir::Mutability::Not => self.tcx.mk_mut_ref(region, t_type), }; - let new_obligation = self.mk_obligation_for_def_id( - trait_ref.skip_binder().def_id, - trait_type, - ObligationCause::dummy(), + let new_obligation = self.mk_trait_obligation_with_new_self_ty( obligation.param_env, + &trait_ref, + suggested_ty, ); - - if self.evaluate_obligation_no_overflow(&new_obligation).must_apply_modulo_regions() - { + let suggested_ty_would_satisfy_obligation = self + .evaluate_obligation_no_overflow(&new_obligation) + .must_apply_modulo_regions(); + if suggested_ty_would_satisfy_obligation { let sp = self .tcx .sess @@ -812,7 +809,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.note(&format!( "`{}` is implemented for `{:?}`, but not for `{:?}`", trait_ref.print_only_trait_path(), - trait_type, + suggested_ty, trait_ref.skip_binder().self_ty(), )); } From 8ea828be2f1266578121a2dfe76693115e8f03ff Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 27 Apr 2020 12:34:00 -0700 Subject: [PATCH 2/7] Add regression test for #71394 --- src/test/ui/suggestions/issue-71394-no-from-impl.rs | 5 +++++ .../ui/suggestions/issue-71394-no-from-impl.stderr | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 src/test/ui/suggestions/issue-71394-no-from-impl.rs create mode 100644 src/test/ui/suggestions/issue-71394-no-from-impl.stderr diff --git a/src/test/ui/suggestions/issue-71394-no-from-impl.rs b/src/test/ui/suggestions/issue-71394-no-from-impl.rs new file mode 100644 index 0000000000000..9ffcc3f7bc1c1 --- /dev/null +++ b/src/test/ui/suggestions/issue-71394-no-from-impl.rs @@ -0,0 +1,5 @@ +fn main() { + let data: &[u8] = &[0; 10]; + let _: &[i8] = data.into(); + //~^ ERROR the trait bound `&[i8]: std::convert::From<&[u8]>` is not satisfied +} diff --git a/src/test/ui/suggestions/issue-71394-no-from-impl.stderr b/src/test/ui/suggestions/issue-71394-no-from-impl.stderr new file mode 100644 index 0000000000000..84c73c2f67e70 --- /dev/null +++ b/src/test/ui/suggestions/issue-71394-no-from-impl.stderr @@ -0,0 +1,11 @@ +error[E0277]: the trait bound `&[i8]: std::convert::From<&[u8]>` is not satisfied + --> $DIR/issue-71394-no-from-impl.rs:3:25 + | +LL | let _: &[i8] = data.into(); + | ^^^^ the trait `std::convert::From<&[u8]>` is not implemented for `&[i8]` + | + = note: required because of the requirements on the impl of `std::convert::Into<&[i8]>` for `&[u8]` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From 4d9e9c6d4ec11dcfa1e89040f894c9400828a9fb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 27 Apr 2020 12:34:25 -0700 Subject: [PATCH 3/7] Bless other example of #71394 --- src/test/ui/suggestions/into-str.stderr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/ui/suggestions/into-str.stderr b/src/test/ui/suggestions/into-str.stderr index 7414a7cc24c92..f7affdbf1b408 100644 --- a/src/test/ui/suggestions/into-str.stderr +++ b/src/test/ui/suggestions/into-str.stderr @@ -8,7 +8,6 @@ LL | foo(String::new()); | ^^^ the trait `std::convert::From` is not implemented for `&str` | = note: to coerce a `std::string::String` into a `&str`, use `&*` as a prefix - = note: `std::convert::From` is implemented for `&mut str`, but not for `&str` = note: required because of the requirements on the impl of `std::convert::Into<&str>` for `std::string::String` error: aborting due to previous error From d2bacb18d2bf9eadca86fe7c1483e38ce774750e Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 28 Apr 2020 13:04:48 -0700 Subject: [PATCH 4/7] Ensure that `new_self_ty` has no escaping bound vars Otherwise inserting it to the `Binder` used by `trait_ref` would cause problems. This is just to be extra carefult: we aren't going to start recommending that the user start using HKTs anytime soon. --- src/librustc_trait_selection/traits/error_reporting/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 20de0b786ed70..c59abd3602116 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -1385,6 +1385,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { trait_ref: &ty::PolyTraitRef<'tcx>, new_self_ty: Ty<'tcx>, ) -> PredicateObligation<'tcx> { + assert!(!new_self_ty.has_escaping_bound_vars()); + let trait_ref = trait_ref.map_bound_ref(|tr| ty::TraitRef { substs: self.tcx.mk_substs_trait(new_self_ty, &tr.substs[1..]), ..*tr From 7278e29592b96207c755bedcc846b64a93aa9281 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 18 May 2020 11:18:41 -0700 Subject: [PATCH 5/7] Document invariants of `mk_trait_obligation_with_new_self_ty` --- src/librustc_trait_selection/traits/error_reporting/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index c59abd3602116..b1c6815c7414f 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -1000,6 +1000,10 @@ trait InferCtxtPrivExt<'tcx> { trait_ref: &ty::PolyTraitRef<'tcx>, ); + /// Creates a `PredicateObligation` with `new_self_ty` replacing the existing type in the + /// `trait_ref`. + /// + /// For this to work, `new_self_ty` must have no escaping bound variables. fn mk_trait_obligation_with_new_self_ty( &self, param_env: ty::ParamEnv<'tcx>, From f99519bebb2cdd90379bb2b07a0cfdeeb5d2d469 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 18 May 2020 11:18:55 -0700 Subject: [PATCH 6/7] Bail out if `output_ty` has bound variables --- .../traits/error_reporting/suggestions.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 6712ddf0ae544..ead857ee88745 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -532,11 +532,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; let msg = format!("use parentheses to call the {}", callable); - let new_obligation = self.mk_trait_obligation_with_new_self_ty( - obligation.param_env, - trait_ref, - output_ty.skip_binder(), - ); + // `mk_trait_obligation_with_new_self_ty` only works for types with no escaping bound + // variables, so bail out if we have any. + let output_ty = match output_ty.no_bound_vars() { + Some(ty) => ty, + None => return, + }; + + let new_obligation = + self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_ref, output_ty); match self.evaluate_obligation(&new_obligation) { Ok( From 1fad3b7a0535c4a4da046170d4080e0cd214ee42 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 18 May 2020 11:45:55 -0700 Subject: [PATCH 7/7] Use `mk_trait_obligation_with_new_self_ty` for new suggestion --- .../traits/error_reporting/suggestions.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index ead857ee88745..db29f6f3bc60b 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1892,7 +1892,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { span: Span, ) { debug!( - "suggest_await_befor_try: obligation={:?}, span={:?}, trait_ref={:?}, trait_ref_self_ty={:?}", + "suggest_await_before_try: obligation={:?}, span={:?}, trait_ref={:?}, trait_ref_self_ty={:?}", obligation, span, trait_ref, @@ -1947,16 +1947,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); debug!( - "suggest_await_befor_try: normalized_projection_type {:?}", + "suggest_await_before_try: normalized_projection_type {:?}", self.resolve_vars_if_possible(&normalized_ty) ); - let try_obligation = self.mk_obligation_for_def_id( - trait_ref.def_id(), - normalized_ty, - obligation.cause.clone(), + let try_obligation = self.mk_trait_obligation_with_new_self_ty( obligation.param_env, + trait_ref, + normalized_ty, ); - debug!("suggest_await_befor_try: try_trait_obligation {:?}", try_obligation); + debug!("suggest_await_before_try: try_trait_obligation {:?}", try_obligation); if self.predicate_may_hold(&try_obligation) && impls_future { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { if snippet.ends_with('?') {