about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2020-04-10 10:19:47 -0700
committerEsteban Küber <esteban@kuber.com.ar>2020-04-20 09:24:41 -0700
commitd3c96f03b5f6b4457cbabf4f3e1ec6c1aed23385 (patch)
tree3cb4cc1be1ab808110ed3b17374b2a73a5824cc0
parent8ce3f840ae9b735a66531996c32330f24b877cb0 (diff)
downloadrust-d3c96f03b5f6b4457cbabf4f3e1ec6c1aed23385.tar.gz
rust-d3c96f03b5f6b4457cbabf4f3e1ec6c1aed23385.zip
Suggest `-> impl Trait` and `-> Box<dyn Trait>` on fn that doesn't return
During development, a function could have a return type set that is a
bare trait object by accident. We already suggest using either a boxed
trait object or `impl Trait` if the return paths will allow it. We now
do so too when there are *no* return paths or they all resolve to `!`.
We still don't handle cases where the trait object is *not* the entirety
of the return type gracefully.
-rw-r--r--src/librustc_trait_selection/traits/error_reporting/suggestions.rs85
-rw-r--r--src/test/ui/error-codes/E0746.stderr4
-rw-r--r--src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs2
-rw-r--r--src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr28
-rw-r--r--src/test/ui/issues/issue-18107.rs2
-rw-r--r--src/test/ui/issues/issue-18107.stderr22
6 files changed, 101 insertions, 42 deletions
diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs
index 2b19699d6ec..abebbe9e903 100644
--- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs
+++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs
@@ -826,12 +826,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             .iter()
             .filter_map(|expr| tables.node_type_opt(expr.hir_id))
             .map(|ty| self.resolve_vars_if_possible(&ty));
-        let (last_ty, all_returns_have_same_type) = ret_types.clone().fold(
-            (None, true),
-            |(last_ty, mut same): (std::option::Option<Ty<'_>>, bool), ty| {
+        let (last_ty, all_returns_have_same_type, count) = ret_types.clone().fold(
+            (None, true, 0),
+            |(last_ty, mut same, count): (std::option::Option<Ty<'_>>, bool, usize), ty| {
                 let ty = self.resolve_vars_if_possible(&ty);
                 same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error;
-                (Some(ty), same)
+                (Some(ty), same, count + 1)
             },
         );
         let all_returns_conform_to_trait =
@@ -846,7 +846,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                                 let obl = Obligation::new(cause.clone(), param_env, pred);
                                 self.predicate_may_hold(&obl)
                             })
-                        })
+                        }) || count == 0
                     }
                     _ => false,
                 }
@@ -855,21 +855,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             };
 
         let sm = self.tcx.sess.source_map();
-        let (snippet, last_ty) =
-            if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true, Some(last_ty)) = (
-                // 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,
-                last_ty,
-            ) {
-                (snippet, last_ty)
-            } else {
-                return false;
-            };
+        let snippet = if 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,
+        ) {
+            snippet
+        } else {
+            return false;
+        };
         err.code(error_code!(E0746));
         err.set_primary_message("return type cannot have an unboxed trait object");
         err.children.clear();
@@ -881,13 +879,50 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             #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 all_returns_have_same_type {
+        if count == 0 {
+            // No return paths. Suggest `-> T`, `-> impl Trait`, and if `Trait` is object safe,
+            // `-> Box<dyn Trait>`.
+            err.note(
+                "currently nothing is being returned, depending on the final implementation \
+                 you could change the return type in different ways",
+            );
+            err.span_suggestion(
+                ret_ty.span,
+                "you could use some type `T` that is `T: Sized` as the return type if all return \
+                 paths will have the same type",
+                "T".to_string(),
+                Applicability::MaybeIncorrect,
+            );
+            err.span_suggestion(
+                ret_ty.span,
+                &format!(
+                    "you could use `impl {}` as the return type if all return paths will have the \
+                     same type but you want to expose only the trait in the signature",
+                    trait_obj,
+                ),
+                format!("impl {}", trait_obj),
+                Applicability::MaybeIncorrect,
+            );
+            err.note(impl_trait_msg);
+            if is_object_safe {
+                err.span_suggestion(
+                    ret_ty.span,
+                    &format!(
+                        "you could use a boxed trait object if all return paths `impl` trait `{}`",
+                        trait_obj,
+                    ),
+                    format!("Box<dyn {}>", trait_obj),
+                    Applicability::MaybeIncorrect,
+                );
+                err.note(trait_obj_msg);
+            }
+        } else if let (Some(last_ty), true) = (last_ty, all_returns_have_same_type) {
             // Suggest `-> impl Trait`.
             err.span_suggestion(
                 ret_ty.span,
                 &format!(
-                    "return `impl {1}` instead, as all return paths are of type `{}`, \
-                        which implements `{1}`",
+                    "use `impl {1}` as the return type, as all return paths are of type `{}`, \
+                     which implements `{1}`",
                     last_ty, trait_obj,
                 ),
                 format!("impl {}", trait_obj),
@@ -925,8 +960,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             }
             err.note(trait_obj_msg);
             err.note(&format!(
-                "if all the returned values were of the same type you could use \
-                    `impl {}` as the return type",
+                "if all the returned values were of the same type you could use `impl {}` as the \
+                 return type",
                 trait_obj,
             ));
             err.note(impl_trait_msg);
diff --git a/src/test/ui/error-codes/E0746.stderr b/src/test/ui/error-codes/E0746.stderr
index e7a8fd304ca..3757ed6d092 100644
--- a/src/test/ui/error-codes/E0746.stderr
+++ b/src/test/ui/error-codes/E0746.stderr
@@ -5,7 +5,7 @@ LL | fn foo() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
    |
 LL | fn foo() -> impl Trait { Struct }
    |             ^^^^^^^^^^
@@ -17,7 +17,7 @@ LL | fn bar() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
    |
 LL | fn bar() -> impl Trait {
    |             ^^^^^^^^^^
diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs
index 08bab5734fd..af368203de0 100644
--- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs
+++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs
@@ -14,7 +14,7 @@ fn bap() -> Trait { Struct }
 //~^ ERROR E0746
 fn ban() -> dyn Trait { Struct }
 //~^ ERROR E0746
-fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0277
+fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0746
 // Suggest using `Box<dyn Trait>`
 fn bal() -> dyn Trait { //~ ERROR E0746
     if true {
diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
index 664a897c593..a01bfc4a81f 100644
--- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
+++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
@@ -49,7 +49,7 @@ LL | fn bap() -> Trait { Struct }
    |             ^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
    |
 LL | fn bap() -> impl Trait { Struct }
    |             ^^^^^^^^^^
@@ -61,20 +61,32 @@ LL | fn ban() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
    |
 LL | fn ban() -> impl Trait { Struct }
    |             ^^^^^^^^^^
 
-error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time
+error[E0746]: return type cannot have an unboxed trait object
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:17:13
    |
 LL | fn bak() -> dyn Trait { unimplemented!() }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-   = help: the trait `std::marker::Sized` is not implemented for `(dyn Trait + 'static)`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
-   = note: the return type of a function must have a statically known size
+   = note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways
+   = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
+   = 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>
+help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type
+   |
+LL | fn bak() -> T { unimplemented!() }
+   |             ^
+help: you could use `impl Trait` as the return type if all return paths will have the same type but you want to expose only the trait in the signature
+   |
+LL | fn bak() -> impl Trait { unimplemented!() }
+   |             ^^^^^^^^^^
+help: you could use a boxed trait object if all return paths `impl` 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
@@ -249,7 +261,7 @@ LL | fn bat() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
    |
 LL | fn bat() -> impl Trait {
    |             ^^^^^^^^^^
@@ -261,7 +273,7 @@ LL | fn bay() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
    = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
-help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
+help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
    |
 LL | fn bay() -> impl Trait {
    |             ^^^^^^^^^^
diff --git a/src/test/ui/issues/issue-18107.rs b/src/test/ui/issues/issue-18107.rs
index 122940f1c17..4bf5b6c0f30 100644
--- a/src/test/ui/issues/issue-18107.rs
+++ b/src/test/ui/issues/issue-18107.rs
@@ -2,7 +2,7 @@ pub trait AbstractRenderer {}
 
 fn _create_render(_: &()) ->
     dyn AbstractRenderer
-//~^ ERROR the size for values of type
+//~^ ERROR return type cannot have an unboxed trait object
 {
     match 0 {
         _ => unimplemented!()
diff --git a/src/test/ui/issues/issue-18107.stderr b/src/test/ui/issues/issue-18107.stderr
index 9bdf470413b..08785ffa73a 100644
--- a/src/test/ui/issues/issue-18107.stderr
+++ b/src/test/ui/issues/issue-18107.stderr
@@ -1,13 +1,25 @@
-error[E0277]: the size for values of type `(dyn AbstractRenderer + 'static)` cannot be known at compilation time
+error[E0746]: return type cannot have an unboxed trait object
   --> $DIR/issue-18107.rs:4:5
    |
 LL |     dyn AbstractRenderer
    |     ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-   = help: the trait `std::marker::Sized` is not implemented for `(dyn AbstractRenderer + 'static)`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
-   = note: the return type of a function must have a statically known size
+   = note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways
+   = note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
+   = 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>
+help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type
+   |
+LL |     T
+   |
+help: you could use `impl AbstractRenderer` as the return type if all return paths will have the same type but you want to expose only the trait in the signature
+   |
+LL |     impl AbstractRenderer
+   |
+help: you could use a boxed trait object if all return paths `impl` trait `AbstractRenderer`
+   |
+LL |     Box<dyn AbstractRenderer>
+   |
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0277`.
+For more information about this error, try `rustc --explain E0746`.