Rollup merge of #95603 - compiler-errors:dyn-return, r=oli-obk

Fix late-bound ICE in `dyn` return type suggestion

This fixes the root-cause of the attached issues -- the root problem is that we're using the return type from a signature with late-bound instead of early-bound regions. The change on line 1087 (`let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };`) makes sure we're grabbing the _right_ return type for this suggestion to check the `dyn` predicates with.

Fixes #91801
Fixes #91803

This fix also includes some drive-by changes, specifically:

1. Don't suggest boxing when we have `-> dyn Trait` and are already returning `Box<T>` where `T: Trait` (before we always boxed the value).
2. Suggestion applies even when the return type is a type alias (e.g. `type Foo = dyn Trait`). This does cause the suggestion to expand to the aliased type, but I think it's still beneficial.
3. Split up the multipart suggestion because there's a 6-line max in the printed output...

I am open to splitting out the above changes, if we just want to fix the ICE first.

cc: ```@terrarier2111``` and #92289

1 files changed, 79 insertions, 55 deletions

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 2912dc3af18..c5324bf85a7 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -1121,8 +1121,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         }
 
         let hir = self.tcx.hir();
-        let parent_node = hir.get_parent_node(obligation.cause.body_id);
-        let node = hir.find(parent_node);
+        let fn_hir_id = hir.get_parent_node(obligation.cause.body_id);
+        let node = hir.find(fn_hir_id);
         let Some(hir::Node::Item(hir::Item {
             kind: hir::ItemKind::Fn(sig, _, body_id),
             ..
@@ -1160,16 +1160,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         visitor.visit_body(&body);
 
         let typeck_results = self.in_progress_typeck_results.map(|t| t.borrow()).unwrap();
+        let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };
 
-        let mut ret_types = visitor
+        let ret_types = visitor
             .returns
             .iter()
-            .filter_map(|expr| typeck_results.node_type_opt(expr.hir_id))
-            .map(|ty| self.resolve_vars_if_possible(ty));
+            .filter_map(|expr| Some((expr.span, typeck_results.node_type_opt(expr.hir_id)?)))
+            .map(|(expr_span, ty)| (expr_span, self.resolve_vars_if_possible(ty)));
         let (last_ty, all_returns_have_same_type, only_never_return) = ret_types.clone().fold(
             (None, true, true),
             |(last_ty, mut same, only_never_return): (std::option::Option<Ty<'_>>, bool, bool),
-             ty| {
+             (_, ty)| {
                 let ty = self.resolve_vars_if_possible(ty);
                 same &=
                     !matches!(ty.kind(), ty::Error(_))
@@ -1190,39 +1191,60 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 (Some(ty), same, only_never_return && matches!(ty.kind(), ty::Never))
             },
         );
-        let all_returns_conform_to_trait =
-            if let Some(ty_ret_ty) = typeck_results.node_type_opt(ret_ty.hir_id) {
-                match ty_ret_ty.kind() {
-                    ty::Dynamic(predicates, _) => {
-                        let cause = ObligationCause::misc(ret_ty.span, ret_ty.hir_id);
-                        let param_env = ty::ParamEnv::empty();
-                        only_never_return
-                            || ret_types.all(|returned_ty| {
-                                predicates.iter().all(|predicate| {
-                                    let pred = predicate.with_self_ty(self.tcx, returned_ty);
-                                    let obl = Obligation::new(cause.clone(), param_env, pred);
-                                    self.predicate_may_hold(&obl)
-                                })
+        let mut spans_and_needs_box = vec![];
+
+        match liberated_sig.output().kind() {
+            ty::Dynamic(predicates, _) => {
+                let cause = ObligationCause::misc(ret_ty.span, fn_hir_id);
+                let param_env = ty::ParamEnv::empty();
+
+                if !only_never_return {
+                    for (expr_span, return_ty) in ret_types {
+                        let self_ty_satisfies_dyn_predicates = |self_ty| {
+                            predicates.iter().all(|predicate| {
+                                let pred = predicate.with_self_ty(self.tcx, self_ty);
+                                let obl = Obligation::new(cause.clone(), param_env, pred);
+                                self.predicate_may_hold(&obl)
                             })
+                        };
+
+                        if let ty::Adt(def, substs) = return_ty.kind()
+                            && def.is_box()
+                            && self_ty_satisfies_dyn_predicates(substs.type_at(0))
+                        {
+                            spans_and_needs_box.push((expr_span, false));
+                        } else if self_ty_satisfies_dyn_predicates(return_ty) {
+                            spans_and_needs_box.push((expr_span, true));
+                        } else {
+                            return false;
+                        }
                     }
-                    _ => false,
                 }
-            } else {
-                true
-            };
+            }
+            _ => return false,
+        };
 
         let sm = self.tcx.sess.source_map();
-        let (true, hir::TyKind::TraitObject(..), Ok(snippet), true) = (
-            // Verify that we're dealing with a return `dyn Trait`
-            ret_ty.span.overlaps(span),
-            &ret_ty.kind,
-            sm.span_to_snippet(ret_ty.span),
-            // If any of the return types does not conform to the trait, then we can't
-            // suggest `impl Trait` nor trait objects: it is a type mismatch error.
-            all_returns_conform_to_trait,
-        ) else {
+        if !ret_ty.span.overlaps(span) {
             return false;
+        }
+        let snippet = if let hir::TyKind::TraitObject(..) = ret_ty.kind {
+            if let Ok(snippet) = sm.span_to_snippet(ret_ty.span) {
+                snippet
+            } else {
+                return false;
+            }
+        } else {
+            // Substitute the type, so we can print a fixup given `type Alias = dyn Trait`
+            let name = liberated_sig.output().to_string();
+            let name =
+                name.strip_prefix('(').and_then(|name| name.strip_suffix(')')).unwrap_or(&name);
+            if !name.starts_with("dyn ") {
+                return false;
+            }
+            name.to_owned()
         };
+
         err.code(error_code!(E0746));
         err.set_primary_message("return type cannot have an unboxed trait object");
         err.children.clear();
@@ -1232,6 +1254,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         let trait_obj_msg = "for information on trait objects, see \
             <https://doc.rust-lang.org/book/ch17-02-trait-objects.html\
             #using-trait-objects-that-allow-for-values-of-different-types>";
+
         let has_dyn = snippet.split_whitespace().next().map_or(false, |s| s == "dyn");
         let trait_obj = if has_dyn { &snippet[4..] } else { &snippet };
         if only_never_return {
@@ -1259,26 +1282,25 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         } else {
             if is_object_safe {
                 // Suggest `-> Box<dyn Trait>` and `Box::new(returned_value)`.
-                // Get all the return values and collect their span and suggestion.
-                let mut suggestions: Vec<_> = visitor
-                    .returns
-                    .iter()
-                    .flat_map(|expr| {
-                        [
-                            (expr.span.shrink_to_lo(), "Box::new(".to_string()),
-                            (expr.span.shrink_to_hi(), ")".to_string()),
-                        ]
-                        .into_iter()
-                    })
-                    .collect();
-                if !suggestions.is_empty() {
-                    // Add the suggestion for the return type.
-                    suggestions.push((ret_ty.span, format!("Box<dyn {}>", trait_obj)));
-                    err.multipart_suggestion(
-                        "return a boxed trait object instead",
-                        suggestions,
-                        Applicability::MaybeIncorrect,
-                    );
+                err.multipart_suggestion(
+                    "return a boxed trait object instead",
+                    vec![
+                        (ret_ty.span.shrink_to_lo(), "Box<".to_string()),
+                        (span.shrink_to_hi(), ">".to_string()),
+                    ],
+                    Applicability::MaybeIncorrect,
+                );
+                for (span, needs_box) in spans_and_needs_box {
+                    if needs_box {
+                        err.multipart_suggestion(
+                            "... and box this value",
+                            vec![
+                                (span.shrink_to_lo(), "Box::new(".to_string()),
+                                (span.shrink_to_hi(), ")".to_string()),
+                            ],
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
                 }
             } else {
                 // This is currently not possible to trigger because E0038 takes precedence, but
@@ -2753,13 +2775,15 @@ fn suggest_trait_object_return_type_alternatives(
         Applicability::MaybeIncorrect,
     );
     if is_object_safe {
-        err.span_suggestion(
-            ret_ty,
+        err.multipart_suggestion(
             &format!(
                 "use a boxed trait object if all return paths implement trait `{}`",
                 trait_obj,
             ),
-            format!("Box<dyn {}>", trait_obj),
+            vec![
+                (ret_ty.shrink_to_lo(), "Box<".to_string()),
+                (ret_ty.shrink_to_hi(), ">".to_string()),
+            ],
             Applicability::MaybeIncorrect,
         );
     }