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

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -1045,8 +1045,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),
             ..
@@ -1084,16 +1084,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(_))
@@ -1114,39 +1115,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();
@@ -1156,6 +1178,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 {
@@ -1183,26 +1206,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
@@ -2677,13 +2699,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,
         );
     }

src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr

@@ -81,7 +81,7 @@ LL | fn bak() -> impl Trait { unimplemented!() }
 help: use a boxed trait object if all return paths implement trait `Trait`
    |
 LL | fn bak() -> Box<dyn Trait> { unimplemented!() }
-   |             ~~~~~~~~~~~~~~
+   |             ++++         +
 
 error[E0746]: return type cannot have an unboxed trait object
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:19:13
@@ -95,12 +95,16 @@ LL | fn bal() -> dyn Trait {
    = note: you can create a new `enum` with a variant for each returned type
 help: return a boxed trait object instead
    |
-LL ~ fn bal() -> Box<dyn Trait> {
-LL |     if true {
-LL ~         return Box::new(Struct);
-LL |     }
-LL ~     Box::new(42)
+LL | fn bal() -> Box<dyn Trait> {
+   |             ++++         +
+help: ... and box this value
+   |
+LL |         return Box::new(Struct);
+   |                +++++++++      +
+help: ... and box this value
    |
+LL |     Box::new(42)
+   |     +++++++++  +
 
 error[E0308]: `if` and `else` have incompatible types
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:9
@@ -126,12 +130,16 @@ LL | fn bax() -> dyn Trait {
    = note: you can create a new `enum` with a variant for each returned type
 help: return a boxed trait object instead
    |
-LL ~ fn bax() -> Box<dyn Trait> {
-LL |     if true {
-LL ~         Box::new(Struct)
-LL |     } else {
-LL ~         Box::new(42)
+LL | fn bax() -> Box<dyn Trait> {
+   |             ++++         +
+help: ... and box this value
+   |
+LL |         Box::new(Struct)
+   |         +++++++++      +
+help: ... and box this value
    |
+LL |         Box::new(42)
+   |         +++++++++  +
 
 error[E0308]: mismatched types
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:34:16

src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr

@@ -103,13 +103,16 @@ LL | fn hat() -> dyn std::fmt::Display {
    = note: you can create a new `enum` with a variant for each returned type
 help: return a boxed trait object instead
    |
-LL ~ fn hat() -> Box<dyn std::fmt::Display> {
-LL |     match 13 {
-LL |         0 => {
-LL ~             return Box::new(0i32);
-LL |         }
-LL |         _ => {
- ...
+LL | fn hat() -> Box<dyn std::fmt::Display> {
+   |             ++++                     +
+help: ... and box this value
+   |
+LL |             return Box::new(0i32);
+   |                    +++++++++    +
+help: ... and box this value
+   |
+LL |             Box::new(1u32)
+   |             +++++++++    +
 
 error[E0308]: `match` arms have incompatible types
   --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:80:14
@@ -135,12 +138,20 @@ LL | fn pug() -> dyn std::fmt::Display {
    = note: you can create a new `enum` with a variant for each returned type
 help: return a boxed trait object instead
    |
-LL ~ fn pug() -> Box<dyn std::fmt::Display> {
-LL |     match 13 {
-LL ~         0 => Box::new(0i32),
-LL ~         1 => Box::new(1u32),
-LL ~         _ => Box::new(2u32),
+LL | fn pug() -> Box<dyn std::fmt::Display> {
+   |             ++++                     +
+help: ... and box this value
+   |
+LL |         0 => Box::new(0i32),
+   |              +++++++++    +
+help: ... and box this value
    |
+LL |         1 => Box::new(1u32),
+   |              +++++++++    +
+help: ... and box this value
+   |
+LL |         _ => Box::new(2u32),
+   |              +++++++++    +
 
 error[E0308]: `if` and `else` have incompatible types
   --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:89:9
@@ -166,12 +177,16 @@ LL | fn man() -> dyn std::fmt::Display {
    = note: you can create a new `enum` with a variant for each returned type
 help: return a boxed trait object instead
    |
-LL ~ fn man() -> Box<dyn std::fmt::Display> {
-LL |     if false {
-LL ~         Box::new(0i32)
-LL |     } else {
-LL ~         Box::new(1u32)
+LL | fn man() -> Box<dyn std::fmt::Display> {
+   |             ++++                     +
+help: ... and box this value
+   |
+LL |         Box::new(0i32)
+   |         +++++++++    +
+help: ... and box this value
    |
+LL |         Box::new(1u32)
+   |         +++++++++    +
 
 error: aborting due to 14 previous errors
 

src/test/ui/issues/issue-18107.stderr

@@ -15,7 +15,7 @@ LL |     impl AbstractRenderer
 help: use a boxed trait object if all return paths implement trait `AbstractRenderer`
    |
 LL |     Box<dyn AbstractRenderer>
-   |
+   |     ++++                    +
 
 error: aborting due to previous error
 

src/test/ui/unsized/box-instead-of-dyn-fn.rs

@@ -0,0 +1,15 @@
+use std::fmt::Debug;
+
+// Test to suggest boxing the return type, and the closure branch of the `if`
+
+fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a {
+    //~^ ERROR return type cannot have an unboxed trait object
+    if a % 2 == 0 {
+        move || println!("{a}")
+    } else {
+        Box::new(move || println!("{}", b))
+        //~^ ERROR `if` and `else` have incompatible types
+    }
+}
+
+fn main() {}

src/test/ui/unsized/box-instead-of-dyn-fn.stderr

@@ -0,0 +1,39 @@
+error[E0308]: `if` and `else` have incompatible types
+  --> $DIR/box-instead-of-dyn-fn.rs:10:9
+   |
+LL | /     if a % 2 == 0 {
+LL | |         move || println!("{a}")
+   | |         ----------------------- expected because of this
+LL | |     } else {
+LL | |         Box::new(move || println!("{}", b))
+   | |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected closure, found struct `Box`
+LL | |
+LL | |     }
+   | |_____- `if` and `else` have incompatible types
+   |
+   = note: expected type `[closure@$DIR/box-instead-of-dyn-fn.rs:8:9: 8:32]`
+            found struct `Box<[closure@$DIR/box-instead-of-dyn-fn.rs:10:18: 10:43]>`
+
+error[E0746]: return type cannot have an unboxed trait object
+  --> $DIR/box-instead-of-dyn-fn.rs:5:56
+   |
+LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a {
+   |                                                        ^^^^^^^^^^^^^ doesn't have a size known at compile-time
+   |
+   = note: 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>
+   = note: if all the returned values were of the same type you could use `impl Fn() + 'a` as the return type
+   = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
+   = note: you can create a new `enum` with a variant for each returned type
+help: return a boxed trait object instead
+   |
+LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> Box<dyn Fn() + 'a> {
+   |                                                        ++++             +
+help: ... and box this value
+   |
+LL |         Box::new(move || println!("{a}"))
+   |         +++++++++                       +
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0308, E0746.
+For more information about an error, try `rustc --explain E0308`.