diff options
| author | Esteban Küber <esteban@kuber.com.ar> | 2020-04-10 10:19:47 -0700 |
|---|---|---|
| committer | Esteban Küber <esteban@kuber.com.ar> | 2020-04-20 09:24:41 -0700 |
| commit | d3c96f03b5f6b4457cbabf4f3e1ec6c1aed23385 (patch) | |
| tree | 3cb4cc1be1ab808110ed3b17374b2a73a5824cc0 | |
| parent | 8ce3f840ae9b735a66531996c32330f24b877cb0 (diff) | |
| download | rust-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.
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`. |
