Skip to content

Commit d3c96f0

Browse files
estebankestebank
committed
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.
1 parent 8ce3f84 commit d3c96f0

File tree

6 files changed

+101
-42
lines changed

6 files changed

+101
-42
lines changed

‎src/librustc_trait_selection/traits/error_reporting/suggestions.rs‎

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -826,12 +826,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
826826
.iter()
827827
.filter_map(|expr| tables.node_type_opt(expr.hir_id))
828828
.map(|ty| self.resolve_vars_if_possible(&ty));
829-
let (last_ty, all_returns_have_same_type) = ret_types.clone().fold(
830-
(None, true),
831-
|(last_ty, mut same): (std::option::Option<Ty<'_>>, bool), ty| {
829+
let (last_ty, all_returns_have_same_type, count) = ret_types.clone().fold(
830+
(None, true, 0),
831+
|(last_ty, mut same, count): (std::option::Option<Ty<'_>>, bool, usize), ty| {
832832
let ty = self.resolve_vars_if_possible(&ty);
833833
same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error;
834-
(Some(ty), same)
834+
(Some(ty), same, count + 1)
835835
},
836836
);
837837
let all_returns_conform_to_trait =
@@ -846,7 +846,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
846846
let obl = Obligation::new(cause.clone(), param_env, pred);
847847
self.predicate_may_hold(&obl)
848848
})
849-
})
849+
}) || count == 0
850850
}
851851
_ => false,
852852
}
@@ -855,21 +855,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
855855
};
856856

857857
let sm = self.tcx.sess.source_map();
858-
let (snippet, last_ty) =
859-
if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true, Some(last_ty)) = (
860-
// Verify that we're dealing with a return `dyn Trait`
861-
ret_ty.span.overlaps(span),
862-
&ret_ty.kind,
863-
sm.span_to_snippet(ret_ty.span),
864-
// If any of the return types does not conform to the trait, then we can't
865-
// suggest `impl Trait` nor trait objects, it is a type mismatch error.
866-
all_returns_conform_to_trait,
867-
last_ty,
868-
) {
869-
(snippet, last_ty)
870-
} else {
871-
return false;
872-
};
858+
let snippet = if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true) = (
859+
// Verify that we're dealing with a return `dyn Trait`
860+
ret_ty.span.overlaps(span),
861+
&ret_ty.kind,
862+
sm.span_to_snippet(ret_ty.span),
863+
// If any of the return types does not conform to the trait, then we can't
864+
// suggest `impl Trait` nor trait objects, it is a type mismatch error.
865+
all_returns_conform_to_trait,
866+
) {
867+
snippet
868+
} else {
869+
return false;
870+
};
873871
err.code(error_code!(E0746));
874872
err.set_primary_message("return type cannot have an unboxed trait object");
875873
err.children.clear();
@@ -881,13 +879,50 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
881879
#using-trait-objects-that-allow-for-values-of-different-types>";
882880
let has_dyn = snippet.split_whitespace().next().map_or(false, |s| s == "dyn");
883881
let trait_obj = if has_dyn { &snippet[4..] } else { &snippet[..] };
884-
if all_returns_have_same_type {
882+
if count == 0 {
883+
// No return paths. Suggest `-> T`, `-> impl Trait`, and if `Trait` is object safe,
884+
// `-> Box<dyn Trait>`.
885+
err.note(
886+
"currently nothing is being returned, depending on the final implementation \
887+
you could change the return type in different ways",
888+
);
889+
err.span_suggestion(
890+
ret_ty.span,
891+
"you could use some type `T` that is `T: Sized` as the return type if all return \
892+
paths will have the same type",
893+
"T".to_string(),
894+
Applicability::MaybeIncorrect,
895+
);
896+
err.span_suggestion(
897+
ret_ty.span,
898+
&format!(
899+
"you could use `impl {}` as the return type if all return paths will have the \
900+
same type but you want to expose only the trait in the signature",
901+
trait_obj,
902+
),
903+
format!("impl {}", trait_obj),
904+
Applicability::MaybeIncorrect,
905+
);
906+
err.note(impl_trait_msg);
907+
if is_object_safe {
908+
err.span_suggestion(
909+
ret_ty.span,
910+
&format!(
911+
"you could use a boxed trait object if all return paths `impl` trait `{}`",
912+
trait_obj,
913+
),
914+
format!("Box<dyn {}>", trait_obj),
915+
Applicability::MaybeIncorrect,
916+
);
917+
err.note(trait_obj_msg);
918+
}
919+
} else if let (Some(last_ty), true) = (last_ty, all_returns_have_same_type) {
885920
// Suggest `-> impl Trait`.
886921
err.span_suggestion(
887922
ret_ty.span,
888923
&format!(
889-
"return `impl {1}` instead, as all return paths are of type `{}`, \
890-
which implements `{1}`",
924+
"use `impl {1}` as the return type, as all return paths are of type `{}`, \
925+
which implements `{1}`",
891926
last_ty, trait_obj,
892927
),
893928
format!("impl {}", trait_obj),
@@ -925,8 +960,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
925960
}
926961
err.note(trait_obj_msg);
927962
err.note(&format!(
928-
"if all the returned values were of the same type you could use \
929-
`impl {}` as the return type",
963+
"if all the returned values were of the same type you could use `impl {}` as the \
964+
return type",
930965
trait_obj,
931966
));
932967
err.note(impl_trait_msg);

‎src/test/ui/error-codes/E0746.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | fn foo() -> dyn Trait { Struct }
55
| ^^^^^^^^^ doesn't have a size known at compile-time
66
|
77
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
8-
help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
8+
help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
99
|
1010
LL | fn foo() -> impl Trait { Struct }
1111
| ^^^^^^^^^^
@@ -17,7 +17,7 @@ LL | fn bar() -> dyn Trait {
1717
| ^^^^^^^^^ doesn't have a size known at compile-time
1818
|
1919
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
20-
help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
20+
help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
2121
|
2222
LL | fn bar() -> impl Trait {
2323
| ^^^^^^^^^^

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn bap() -> Trait { Struct }
1414
//~^ ERROR E0746
1515
fn ban() -> dyn Trait { Struct }
1616
//~^ ERROR E0746
17-
fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0277
17+
fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0746
1818
// Suggest using `Box<dyn Trait>`
1919
fn bal() -> dyn Trait { //~ ERROR E0746
2020
if true {

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ LL | fn bap() -> Trait { Struct }
4949
| ^^^^^ doesn't have a size known at compile-time
5050
|
5151
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
52-
help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
52+
help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
5353
|
5454
LL | fn bap() -> impl Trait { Struct }
5555
| ^^^^^^^^^^
@@ -61,20 +61,32 @@ LL | fn ban() -> dyn Trait { Struct }
6161
| ^^^^^^^^^ doesn't have a size known at compile-time
6262
|
6363
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
64-
help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait`
64+
help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait`
6565
|
6666
LL | fn ban() -> impl Trait { Struct }
6767
| ^^^^^^^^^^
6868

69-
error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time
69+
error[E0746]: return type cannot have an unboxed trait object
7070
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:17:13
7171
|
7272
LL | fn bak() -> dyn Trait { unimplemented!() }
7373
| ^^^^^^^^^ doesn't have a size known at compile-time
7474
|
75-
= help: the trait `std::marker::Sized` is not implemented for `(dyn Trait + 'static)`
76-
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
77-
= note: the return type of a function must have a statically known size
75+
= note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways
76+
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
77+
= 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>
78+
help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type
79+
|
80+
LL | fn bak() -> T { unimplemented!() }
81+
| ^
82+
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
83+
|
84+
LL | fn bak() -> impl Trait { unimplemented!() }
85+
| ^^^^^^^^^^
86+
help: you could use a boxed trait object if all return paths `impl` trait `Trait`
87+
|
88+
LL | fn bak() -> Box<dyn Trait> { unimplemented!() }
89+
| ^^^^^^^^^^^^^^
7890

7991
error[E0746]: return type cannot have an unboxed trait object
8092
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:19:13
@@ -249,7 +261,7 @@ LL | fn bat() -> dyn Trait {
249261
| ^^^^^^^^^ doesn't have a size known at compile-time
250262
|
251263
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
252-
help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
264+
help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
253265
|
254266
LL | fn bat() -> impl Trait {
255267
| ^^^^^^^^^^
@@ -261,7 +273,7 @@ LL | fn bay() -> dyn Trait {
261273
| ^^^^^^^^^ doesn't have a size known at compile-time
262274
|
263275
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
264-
help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait`
276+
help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
265277
|
266278
LL | fn bay() -> impl Trait {
267279
| ^^^^^^^^^^

‎src/test/ui/issues/issue-18107.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ pub trait AbstractRenderer {}
22

33
fn _create_render(_: &()) ->
44
dyn AbstractRenderer
5-
//~^ ERROR the size for values of type
5+
//~^ ERROR return type cannot have an unboxed trait object
66
{
77
match 0 {
88
_ => unimplemented!()

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,25 @@
1-
error[E0277]: the size for values of type `(dyn AbstractRenderer + 'static)` cannot be known at compilation time
1+
error[E0746]: return type cannot have an unboxed trait object
22
--> $DIR/issue-18107.rs:4:5
33
|
44
LL | dyn AbstractRenderer
55
| ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
66
|
7-
= help: the trait `std::marker::Sized` is not implemented for `(dyn AbstractRenderer + 'static)`
8-
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
9-
= note: the return type of a function must have a statically known size
7+
= note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways
8+
= note: for information on `impl Trait`, see <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
9+
= 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>
10+
help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type
11+
|
12+
LL | T
13+
|
14+
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
15+
|
16+
LL | impl AbstractRenderer
17+
|
18+
help: you could use a boxed trait object if all return paths `impl` trait `AbstractRenderer`
19+
|
20+
LL | Box<dyn AbstractRenderer>
21+
|
1022

1123
error: aborting due to previous error
1224

13-
For more information about this error, try `rustc --explain E0277`.
25+
For more information about this error, try `rustc --explain E0746`.

0 commit comments

Comments
 (0)