Silence unnecessary "missing dyn" errors and tweak E0746 suggestions

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

@@ -204,7 +204,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable);
             if is_object_safe {
                 diag.multipart_suggestion_verbose(
-                    "alternatively, you can return an owned trait object",
+                    "alternatively, you can return a boxed trait object",
                     vec![
                         (ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
                         (ty.span.shrink_to_hi(), ">".to_string()),

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

@@ -10,6 +10,7 @@ use crate::infer::InferCtxt;
 use crate::traits::{ImplDerivedObligationCause, NormalizeExt, ObligationCtxt};
 
 use hir::def::CtorOf;
+use rustc_ast::TraitObjectSyntax;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{
@@ -1791,27 +1792,90 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         err.primary_message("return type cannot have an unboxed trait object");
         err.children.clear();
 
-        let span = obligation.cause.span;
-        if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span)
-            && snip.starts_with("dyn ")
-        {
-            err.span_suggestion(
-                span.with_hi(span.lo() + BytePos(4)),
-                "return an `impl Trait` instead of a `dyn Trait`, \
-                if all returned values are the same type",
-                "impl ",
-                Applicability::MaybeIncorrect,
-            );
-        }
-
         let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(obligation.cause.body_id));
 
         let mut visitor = ReturnsVisitor::default();
         visitor.visit_body(body);
 
-        let mut sugg =
-            vec![(span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string())];
-        sugg.extend(visitor.returns.into_iter().flat_map(|expr| {
+        let fn_id = self.tcx.hir().body_owner(body.id());
+        let Some(fn_decl) = self.tcx.hir_node(fn_id).fn_decl() else {
+            return false;
+        };
+        let hir::FnRetTy::Return(hir::Ty {
+            kind: hir::TyKind::TraitObject(traits @ [tr, ..], _lt, syn),
+            span,
+            ..
+        }) = fn_decl.output
+        else {
+            return false;
+        };
+
+        // Suggest `-> impl Trait`
+        let ret_tys = if let Some(typeck) = &self.typeck_results {
+            visitor
+                .returns
+                .iter()
+                .filter_map(|expr| typeck.node_type_opt(expr.hir_id).map(|ty| (expr, ty)))
+                .collect()
+        } else {
+            vec![]
+        };
+
+        // FIXME: account for `impl` assoc `fn`, where suggesting `Box<dyn Trait>` if `impl Trait`
+        // is expected is ok, but suggesting `impl Trait` where `Box<dyn Trait>` is expected is not.
+        // FIXME: account for the case where `-> dyn Trait` but the returned value is
+        // `Box<dyn Trait>`, like in `tests/ui/unsized/issue-91801.rs`.
+
+        let mut all_same_ty = true;
+        let mut prev_ty = None;
+        for (_, ty) in &ret_tys {
+            if let Some(prev_ty) = prev_ty {
+                if prev_ty != ty {
+                    all_same_ty = false;
+                    break;
+                }
+            } else {
+                prev_ty = Some(ty);
+            }
+        }
+        let mut alternatively = "";
+        if all_same_ty || visitor.returns.len() == 1 {
+            // We're certain that `impl Trait` will work.
+            err.span_suggestion_verbose(
+                // This will work for both `Dyn` and `None` object syntax.
+                span.until(tr.span),
+                "return an `impl Trait` instead of a `dyn Trait`",
+                "impl ",
+                Applicability::MachineApplicable,
+            );
+            alternatively = "alternatively, ";
+        } else {
+            let mut span: MultiSpan =
+                ret_tys.iter().map(|(expr, _)| expr.span).collect::<Vec<Span>>().into();
+            for (expr, ty) in &ret_tys {
+                span.push_span_label(expr.span, format!("this returned value has type `{ty}`"));
+            }
+
+            err.span_help(
+                span,
+                format!(
+                    "the returned values do not all have the same type, so `impl Trait` can't be \
+                     used",
+                ),
+            );
+        }
+
+        // Suggest `-> Box<dyn Trait>`
+        let pre = match syn {
+            TraitObjectSyntax::None => "dyn ",
+            _ => "",
+        };
+        let mut sugg = vec![
+            (span.shrink_to_lo(), format!("Box<{pre}")),
+            (span.shrink_to_hi(), ">".to_string()),
+        ];
+
+        sugg.extend(visitor.returns.iter().flat_map(|expr| {
             let span =
                 expr.span.find_ancestor_in_same_ctxt(obligation.cause.span).unwrap_or(expr.span);
             if !span.can_be_used_for_suggestions() {
@@ -1835,11 +1899,77 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             }
         }));
 
-        err.multipart_suggestion(
-            "box the return type, and wrap all of the returned values in `Box::new`",
-            sugg,
-            Applicability::MaybeIncorrect,
-        );
+        let object_unsafe: Vec<_> = traits
+            .iter()
+            .filter_map(|tr| {
+                tr.trait_ref
+                    .path
+                    .res
+                    .opt_def_id()
+                    .filter(|def_id| !self.tcx.object_safety_violations(def_id).is_empty())
+                    .map(|def_id| (def_id, tr.span))
+            })
+            .collect();
+        if !object_unsafe.is_empty() {
+            let span: MultiSpan =
+                object_unsafe.iter().map(|(_, sp)| *sp).collect::<Vec<Span>>().into();
+            err.span_help(
+                span,
+                format!(
+                    "{} not object safe, so you can't return a boxed trait object `{}`",
+                    if let [(def_id, _)] = &object_unsafe[..] {
+                        format!("trait `{}` is", self.tcx.def_path_str(*def_id))
+                    } else {
+                        "the following traits are".to_string()
+                    },
+                    format!(
+                        "Box<dyn {}>",
+                        object_unsafe
+                            .iter()
+                            .map(|(def_id, _)| self.tcx.def_path_str(*def_id))
+                            .collect::<Vec<_>>()
+                            .join(" + ")
+                    ),
+                ),
+            );
+        } else {
+            err.multipart_suggestion(
+                format!(
+                    "{alternatively}box the return type to make a boxed trait object, and wrap \
+                     all of the returned values in `Box::new`",
+                ),
+                sugg,
+                Applicability::MaybeIncorrect,
+            );
+            alternatively = if alternatively.is_empty() { "alternatively, " } else { "finally, " };
+        }
+
+        // Suggest `-> &dyn Trait`
+        let borrow_input_tys: Vec<_> = fn_decl
+            .inputs
+            .iter()
+            .filter(|hir_ty| matches!(hir_ty.kind, hir::TyKind::Ref(..)))
+            .collect();
+        if !borrow_input_tys.is_empty() {
+            // Maybe returning a `&dyn Trait` borrowing from an argument might be ok?
+            err.span_suggestion_verbose(
+                // This will work for both `Dyn` and `None` object syntax.
+                span.shrink_to_lo(),
+                format!(
+                    "{alternatively}you might be able to borrow from {}",
+                    if borrow_input_tys.len() == 1 {
+                        "the function's argument"
+                    } else {
+                        "one of the function's arguments"
+                    },
+                ),
+                match syn {
+                    TraitObjectSyntax::None => "&dyn ",
+                    _ => "&",
+                },
+                Applicability::MachineApplicable,
+            );
+        }
 
         true
     }

tests/ui/error-codes/E0746.stderr

@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn foo() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn foo() -> impl Trait { Struct }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL | fn foo() -> Box<dyn Trait> { Box::new(Struct) }
    |             ++++         +   +++++++++      +
@@ -19,11 +19,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bar() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn bar() -> impl Trait {
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bar() -> Box<dyn Trait> {
 LL |     if true {

tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr

@@ -48,22 +48,26 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bap() -> Trait { Struct }
    |             ^^^^^ doesn't have a size known at compile-time
    |
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: return an `impl Trait` instead of a `dyn Trait`
    |
-LL | fn bap() -> Box<Trait> { Box::new(Struct) }
-   |             ++++     +   +++++++++      +
+LL | fn bap() -> impl Trait { Struct }
+   |             ++++
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
+   |
+LL | fn bap() -> Box<dyn Trait> { Box::new(Struct) }
+   |             +++++++      +   +++++++++      +
 
 error[E0746]: return type cannot have an unboxed trait object
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:15:13
    |
 LL | fn ban() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn ban() -> impl Trait { Struct }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL | fn ban() -> Box<dyn Trait> { Box::new(Struct) }
    |             ++++         +   +++++++++      +
@@ -74,11 +78,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bak() -> dyn Trait { unimplemented!() }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn bak() -> impl Trait { unimplemented!() }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL | fn bak() -> Box<dyn Trait> { Box::new(unimplemented!()) }
    |             ++++         +   +++++++++                +
@@ -89,11 +93,15 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bal() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: the returned values do not all have the same type, so `impl Trait` can't be used
+  --> $DIR/dyn-trait-return-should-be-impl-trait.rs:21:16
    |
-LL | fn bal() -> impl Trait {
-   |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+LL |         return Struct;
+   |                ^^^^^^ this returned value has type `Struct`
+LL |     }
+LL |     42
+   |     ^^ this returned value has type `{integer}`
+help: box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bal() -> Box<dyn Trait> {
 LL |     if true {
@@ -108,11 +116,15 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bax() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: the returned values do not all have the same type, so `impl Trait` can't be used
+  --> $DIR/dyn-trait-return-should-be-impl-trait.rs:27:9
    |
-LL | fn bax() -> impl Trait {
-   |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+LL |         Struct
+   |         ^^^^^^ this returned value has type `Struct`
+LL |     } else {
+LL |         42
+   |         ^^ this returned value has type `{integer}`
+help: box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bax() -> Box<dyn Trait> {
 LL |     if true {
@@ -263,11 +275,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bat() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn bat() -> impl Trait {
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bat() -> Box<dyn Trait> {
 LL |     if true {
@@ -282,11 +294,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bay() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: return an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn bay() -> impl Trait {
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bay() -> Box<dyn Trait> {
 LL |     if true {

tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.rs

@@ -33,4 +33,10 @@ fn cat() -> Box<dyn NotObjectSafe> { //~ ERROR the trait `NotObjectSafe` cannot
     Box::new(B) //~ ERROR cannot be made into an object
 }
 
+fn can(x: &A) -> dyn NotObjectSafe { //~ ERROR the trait `NotObjectSafe` cannot be made into an object
+//~^ ERROR return type cannot have an unboxed trait object
+    x
+}
+
+
 fn main() {}