Skip to content

Commit 24fb393

Browse files
JohnTitorJohnTitor
authored
Rollup merge of #70998 - estebank:suggest-impl-trait-empty-fn, r=varkor
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. Closes #38376.
2 parents 45d050c + e536257 commit 24fb393

File tree

8 files changed

+161
-55
lines changed

8 files changed

+161
-55
lines changed

‎src/librustc_error_codes/error_codes/E0746.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ Return types cannot be `dyn Trait`s as they must be `Sized`.
22

33
Erroneous code example:
44

5-
```compile_fail,E0277
6-
# // FIXME: after E0746 is in beta, change the above
5+
```compile_fail,E0746
76
trait T {
87
fn bar(&self);
98
}

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

Lines changed: 92 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_hir::intravisit::Visitor;
1313
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
1414
use rustc_middle::ty::TypeckTables;
1515
use rustc_middle::ty::{
16-
self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
16+
self, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
1717
};
1818
use rustc_span::symbol::{kw, sym, Symbol};
1919
use rustc_span::{MultiSpan, Span, DUMMY_SP};
@@ -826,12 +826,28 @@ 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, only_never_return) = ret_types.clone().fold(
830+
(None, true, true),
831+
|(last_ty, mut same, only_never_return): (std::option::Option<Ty<'_>>, bool, bool),
832+
ty| {
832833
let ty = self.resolve_vars_if_possible(&ty);
833-
same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error;
834-
(Some(ty), same)
834+
same &=
835+
ty.kind != ty::Error
836+
&& last_ty.map_or(true, |last_ty| {
837+
// FIXME: ideally we would use `can_coerce` here instead, but `typeck` comes
838+
// *after* in the dependency graph.
839+
match (&ty.kind, &last_ty.kind) {
840+
(Infer(InferTy::IntVar(_)), Infer(InferTy::IntVar(_)))
841+
| (Infer(InferTy::FloatVar(_)), Infer(InferTy::FloatVar(_)))
842+
| (Infer(InferTy::FreshIntTy(_)), Infer(InferTy::FreshIntTy(_)))
843+
| (
844+
Infer(InferTy::FreshFloatTy(_)),
845+
Infer(InferTy::FreshFloatTy(_)),
846+
) => true,
847+
_ => ty == last_ty,
848+
}
849+
});
850+
(Some(ty), same, only_never_return && matches!(ty.kind, ty::Never))
835851
},
836852
);
837853
let all_returns_conform_to_trait =
@@ -840,13 +856,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
840856
ty::Dynamic(predicates, _) => {
841857
let cause = ObligationCause::misc(ret_ty.span, ret_ty.hir_id);
842858
let param_env = ty::ParamEnv::empty();
843-
ret_types.all(|returned_ty| {
844-
predicates.iter().all(|predicate| {
845-
let pred = predicate.with_self_ty(self.tcx, returned_ty);
846-
let obl = Obligation::new(cause.clone(), param_env, pred);
847-
self.predicate_may_hold(&obl)
859+
only_never_return
860+
|| ret_types.all(|returned_ty| {
861+
predicates.iter().all(|predicate| {
862+
let pred = predicate.with_self_ty(self.tcx, returned_ty);
863+
let obl = Obligation::new(cause.clone(), param_env, pred);
864+
self.predicate_may_hold(&obl)
865+
})
848866
})
849-
})
850867
}
851868
_ => false,
852869
}
@@ -855,21 +872,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
855872
};
856873

857874
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-
};
875+
let snippet = if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true) = (
876+
// Verify that we're dealing with a return `dyn Trait`
877+
ret_ty.span.overlaps(span),
878+
&ret_ty.kind,
879+
sm.span_to_snippet(ret_ty.span),
880+
// If any of the return types does not conform to the trait, then we can't
881+
// suggest `impl Trait` nor trait objects: it is a type mismatch error.
882+
all_returns_conform_to_trait,
883+
) {
884+
snippet
885+
} else {
886+
return false;
887+
};
873888
err.code(error_code!(E0746));
874889
err.set_primary_message("return type cannot have an unboxed trait object");
875890
err.children.clear();
@@ -881,13 +896,22 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
881896
#using-trait-objects-that-allow-for-values-of-different-types>";
882897
let has_dyn = snippet.split_whitespace().next().map_or(false, |s| s == "dyn");
883898
let trait_obj = if has_dyn { &snippet[4..] } else { &snippet[..] };
884-
if all_returns_have_same_type {
899+
if only_never_return {
900+
// No return paths, probably using `panic!()` or similar.
901+
// Suggest `-> T`, `-> impl Trait`, and if `Trait` is object safe, `-> Box<dyn Trait>`.
902+
suggest_trait_object_return_type_alternatives(
903+
err,
904+
ret_ty.span,
905+
trait_obj,
906+
is_object_safe,
907+
);
908+
} else if let (Some(last_ty), true) = (last_ty, all_returns_have_same_type) {
885909
// Suggest `-> impl Trait`.
886910
err.span_suggestion(
887911
ret_ty.span,
888912
&format!(
889-
"return `impl {1}` instead, as all return paths are of type `{}`, \
890-
which implements `{1}`",
913+
"use `impl {1}` as the return type, as all return paths are of type `{}`, \
914+
which implements `{1}`",
891915
last_ty, trait_obj,
892916
),
893917
format!("impl {}", trait_obj),
@@ -925,8 +949,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
925949
}
926950
err.note(trait_obj_msg);
927951
err.note(&format!(
928-
"if all the returned values were of the same type you could use \
929-
`impl {}` as the return type",
952+
"if all the returned values were of the same type you could use `impl {}` as the \
953+
return type",
930954
trait_obj,
931955
));
932956
err.note(impl_trait_msg);
@@ -1813,3 +1837,39 @@ impl NextTypeParamName for &[hir::GenericParam<'_>] {
18131837
.to_string()
18141838
}
18151839
}
1840+
1841+
fn suggest_trait_object_return_type_alternatives(
1842+
err: &mut DiagnosticBuilder<'tcx>,
1843+
ret_ty: Span,
1844+
trait_obj: &str,
1845+
is_object_safe: bool,
1846+
) {
1847+
err.span_suggestion(
1848+
ret_ty,
1849+
"use some type `T` that is `T: Sized` as the return type if all return paths have the \
1850+
same type",
1851+
"T".to_string(),
1852+
Applicability::MaybeIncorrect,
1853+
);
1854+
err.span_suggestion(
1855+
ret_ty,
1856+
&format!(
1857+
"use `impl {}` as the return type if all return paths have the same type but you \
1858+
want to expose only the trait in the signature",
1859+
trait_obj,
1860+
),
1861+
format!("impl {}", trait_obj),
1862+
Applicability::MaybeIncorrect,
1863+
);
1864+
if is_object_safe {
1865+
err.span_suggestion(
1866+
ret_ty,
1867+
&format!(
1868+
"use a boxed trait object if all return paths implement trait `{}`",
1869+
trait_obj,
1870+
),
1871+
format!("Box<dyn {}>", trait_obj),
1872+
Applicability::MaybeIncorrect,
1873+
);
1874+
}
1875+
}

‎src/librustc_typeck/check/mod.rs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,7 +1305,6 @@ fn check_fn<'a, 'tcx>(
13051305
let hir = tcx.hir();
13061306

13071307
let declared_ret_ty = fn_sig.output();
1308-
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
13091308
let revealed_ret_ty =
13101309
fcx.instantiate_opaque_types_from_value(fn_id, &declared_ret_ty, decl.output.span());
13111310
debug!("check_fn: declared_ret_ty: {}, revealed_ret_ty: {}", declared_ret_ty, revealed_ret_ty);
@@ -1374,7 +1373,25 @@ fn check_fn<'a, 'tcx>(
13741373

13751374
inherited.tables.borrow_mut().liberated_fn_sigs_mut().insert(fn_id, fn_sig);
13761375

1377-
fcx.check_return_expr(&body.value);
1376+
if let ty::Dynamic(..) = declared_ret_ty.kind {
1377+
// FIXME: We need to verify that the return type is `Sized` after the return expression has
1378+
// been evaluated so that we have types available for all the nodes being returned, but that
1379+
// requires the coerced evaluated type to be stored. Moving `check_return_expr` before this
1380+
// causes unsized errors caused by the `declared_ret_ty` to point at the return expression,
1381+
// while keeping the current ordering we will ignore the tail expression's type because we
1382+
// don't know it yet. We can't do `check_expr_kind` while keeping `check_return_expr`
1383+
// because we will trigger "unreachable expression" lints unconditionally.
1384+
// Because of all of this, we perform a crude check to know whether the simplest `!Sized`
1385+
// case that a newcomer might make, returning a bare trait, and in that case we populate
1386+
// the tail expression's type so that the suggestion will be correct, but ignore all other
1387+
// possible cases.
1388+
fcx.check_expr(&body.value);
1389+
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
1390+
tcx.sess.delay_span_bug(decl.output.span(), "`!Sized` return type");
1391+
} else {
1392+
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
1393+
fcx.check_return_expr(&body.value);
1394+
}
13781395

13791396
// We insert the deferred_generator_interiors entry after visiting the body.
13801397
// This ensures that all nested generators appear before the entry of this generator.

‎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: 2 additions & 2 deletions
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 {
@@ -26,7 +26,7 @@ fn bax() -> dyn Trait { //~ ERROR E0746
2626
if true {
2727
Struct
2828
} else {
29-
42
29+
42 //~ ERROR `if` and `else` have incompatible types
3030
}
3131
}
3232
fn bam() -> Box<dyn Trait> {

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

Lines changed: 30 additions & 9 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,29 @@ 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+
help: use some type `T` that is `T: Sized` as the return type if all return paths have the same type
76+
|
77+
LL | fn bak() -> T { unimplemented!() }
78+
| ^
79+
help: use `impl Trait` as the return type if all return paths have the same type but you want to expose only the trait in the signature
80+
|
81+
LL | fn bak() -> impl Trait { unimplemented!() }
82+
| ^^^^^^^^^^
83+
help: use a boxed trait object if all return paths implement trait `Trait`
84+
|
85+
LL | fn bak() -> Box<dyn Trait> { unimplemented!() }
86+
| ^^^^^^^^^^^^^^
7887

7988
error[E0746]: return type cannot have an unboxed trait object
8089
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:19:13
@@ -95,6 +104,18 @@ LL | }
95104
LL | Box::new(42)
96105
|
97106

107+
error[E0308]: `if` and `else` have incompatible types
108+
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:9
109+
|
110+
LL | / if true {
111+
LL | | Struct
112+
| | ------ expected because of this
113+
LL | | } else {
114+
LL | | 42
115+
| | ^^ expected struct `Struct`, found integer
116+
LL | | }
117+
| |_____- `if` and `else` have incompatible types
118+
98119
error[E0746]: return type cannot have an unboxed trait object
99120
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:25:13
100121
|
@@ -249,7 +270,7 @@ LL | fn bat() -> dyn Trait {
249270
| ^^^^^^^^^ doesn't have a size known at compile-time
250271
|
251272
= 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`
273+
help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
253274
|
254275
LL | fn bat() -> impl Trait {
255276
| ^^^^^^^^^^
@@ -261,12 +282,12 @@ LL | fn bay() -> dyn Trait {
261282
| ^^^^^^^^^ doesn't have a size known at compile-time
262283
|
263284
= 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`
285+
help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait`
265286
|
266287
LL | fn bay() -> impl Trait {
267288
| ^^^^^^^^^^
268289

269-
error: aborting due to 19 previous errors
290+
error: aborting due to 20 previous errors
270291

271292
Some errors have detailed explanations: E0277, E0308, E0746.
272293
For more information about an error, try `rustc --explain E0277`.

‎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: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
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+
help: use some type `T` that is `T: Sized` as the return type if all return paths have the same type
8+
|
9+
LL | T
10+
|
11+
help: use `impl AbstractRenderer` as the return type if all return paths have the same type but you want to expose only the trait in the signature
12+
|
13+
LL | impl AbstractRenderer
14+
|
15+
help: use a boxed trait object if all return paths implement trait `AbstractRenderer`
16+
|
17+
LL | Box<dyn AbstractRenderer>
18+
|
1019

1120
error: aborting due to previous error
1221

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

0 commit comments

Comments
 (0)