Rollup merge of rust-lang#71618 - ecstatic-morse:issue-71394, r=nikomatsakis

Preserve substitutions when making trait obligations for suggestions

Resolves rust-lang#71394.

I *think* `map_bound_ref` is correct here. In any case, I think a lot of the diagnostic code is using `skip_binder` more aggressively than it should be, so I doubt that this is worse than the status quo. The assertion that `new_self_ty` has no escaping bound vars should be enough.

r? @estebank

cc @nikomatsakis Is the call to `skip_binder` on line 551 (and elsewhere in this file) appropriate? https://github.com/rust-lang/rust/blob/46ec74e60f238f694b46c976d6217e7cf8d4cf1a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs#L537-L565

Author: Dylan-DPC

src/librustc_trait_selection/traits/error_reporting/mod.rs

@@ -1000,12 +1000,15 @@ trait InferCtxtPrivExt<'tcx> {
         trait_ref: &ty::PolyTraitRef<'tcx>,
     );
 
-    fn mk_obligation_for_def_id(
+    /// 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,
-        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 +1383,24 @@ 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))
+        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
+        });
+
+        Obligation::new(
+            ObligationCause::dummy(),
+            param_env,
+            trait_ref.without_const().to_predicate(self.tcx),
+        )
     }
 
     fn maybe_report_ambiguity(

src/librustc_trait_selection/traits/error_reporting/suggestions.rs

@@ -532,14 +532,17 @@ 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(),
-            obligation.param_env,
-        );
+        // `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(&obligation) {
+        match self.evaluate_obligation(&new_obligation) {
             Ok(
                 EvaluationResult::EvaluatedToOk
                 | EvaluationResult::EvaluatedToOkModuloRegions
@@ -694,7 +697,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
     ) {
-        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 +707,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 +783,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 +813,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(),
                         ));
                     }
@@ -1891,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,
@@ -1946,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('?') {

src/test/ui/suggestions/into-str.stderr

@@ -8,7 +8,6 @@ LL |     foo(String::new());
    |     ^^^ the trait `std::convert::From<std::string::String>` is not implemented for `&str`
    |
    = note: to coerce a `std::string::String` into a `&str`, use `&*` as a prefix
-   = note: `std::convert::From<std::string::String>` 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

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
+}

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`.