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.
This commit is contained in:
Yuki Okushi
GitHub
commit
24fb393207
@ -2,8 +2,7 @@ Return types cannot be `dyn Trait`s as they must be `Sized`.
|
||||
|
||||
Erroneous code example:
|
||||
|
||||
```compile_fail,E0277
|
||||
# // FIXME: after E0746 is in beta, change the above
|
||||
```compile_fail,E0746
|
||||
trait T {
|
||||
fn bar(&self);
|
||||
}
|
||||
|
||||
@ -13,7 +13,7 @@
|
||||
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
|
||||
use rustc_middle::ty::TypeckTables;
|
||||
use rustc_middle::ty::{
|
||||
self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
|
||||
self, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
|
||||
};
|
||||
use rustc_span::symbol::{kw, sym, Symbol};
|
||||
use rustc_span::{MultiSpan, Span, DUMMY_SP};
|
||||
@ -826,12 +826,28 @@ fn suggest_impl_trait(
|
||||
.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, only_never_return) = ret_types.clone().fold(
|
||||
(None, true, true),
|
||||
|(last_ty, mut same, only_never_return): (std::option::Option<Ty<'_>>, bool, bool),
|
||||
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)
|
||||
same &=
|
||||
ty.kind != ty::Error
|
||||
&& last_ty.map_or(true, |last_ty| {
|
||||
// FIXME: ideally we would use `can_coerce` here instead, but `typeck` comes
|
||||
// *after* in the dependency graph.
|
||||
match (&ty.kind, &last_ty.kind) {
|
||||
(Infer(InferTy::IntVar(_)), Infer(InferTy::IntVar(_)))
|
||||
| (Infer(InferTy::FloatVar(_)), Infer(InferTy::FloatVar(_)))
|
||||
| (Infer(InferTy::FreshIntTy(_)), Infer(InferTy::FreshIntTy(_)))
|
||||
| (
|
||||
Infer(InferTy::FreshFloatTy(_)),
|
||||
Infer(InferTy::FreshFloatTy(_)),
|
||||
) => true,
|
||||
_ => ty == last_ty,
|
||||
}
|
||||
});
|
||||
(Some(ty), same, only_never_return && matches!(ty.kind, ty::Never))
|
||||
},
|
||||
);
|
||||
let all_returns_conform_to_trait =
|
||||
@ -840,13 +856,14 @@ fn suggest_impl_trait(
|
||||
ty::Dynamic(predicates, _) => {
|
||||
let cause = ObligationCause::misc(ret_ty.span, ret_ty.hir_id);
|
||||
let param_env = ty::ParamEnv::empty();
|
||||
ret_types.all(|returned_ty| {
|
||||
predicates.iter().all(|predicate| {
|
||||
let pred = predicate.with_self_ty(self.tcx, returned_ty);
|
||||
let obl = Obligation::new(cause.clone(), param_env, pred);
|
||||
self.predicate_may_hold(&obl)
|
||||
only_never_return
|
||||
|| ret_types.all(|returned_ty| {
|
||||
predicates.iter().all(|predicate| {
|
||||
let pred = predicate.with_self_ty(self.tcx, returned_ty);
|
||||
let obl = Obligation::new(cause.clone(), param_env, pred);
|
||||
self.predicate_may_hold(&obl)
|
||||
})
|
||||
})
|
||||
})
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
@ -855,21 +872,19 @@ fn suggest_impl_trait(
|
||||
};
|
||||
|
||||
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 +896,22 @@ fn suggest_impl_trait(
|
||||
#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 only_never_return {
|
||||
// No return paths, probably using `panic!()` or similar.
|
||||
// Suggest `-> T`, `-> impl Trait`, and if `Trait` is object safe, `-> Box<dyn Trait>`.
|
||||
suggest_trait_object_return_type_alternatives(
|
||||
err,
|
||||
ret_ty.span,
|
||||
trait_obj,
|
||||
is_object_safe,
|
||||
);
|
||||
} 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 +949,8 @@ fn suggest_impl_trait(
|
||||
}
|
||||
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);
|
||||
@ -1813,3 +1837,39 @@ fn next_type_param_name(&self, name: Option<&str>) -> String {
|
||||
.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
fn suggest_trait_object_return_type_alternatives(
|
||||
err: &mut DiagnosticBuilder<'tcx>,
|
||||
ret_ty: Span,
|
||||
trait_obj: &str,
|
||||
is_object_safe: bool,
|
||||
) {
|
||||
err.span_suggestion(
|
||||
ret_ty,
|
||||
"use some type `T` that is `T: Sized` as the return type if all return paths have the \
|
||||
same type",
|
||||
"T".to_string(),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
err.span_suggestion(
|
||||
ret_ty,
|
||||
&format!(
|
||||
"use `impl {}` as the return type if all return paths have the same type but you \
|
||||
want to expose only the trait in the signature",
|
||||
trait_obj,
|
||||
),
|
||||
format!("impl {}", trait_obj),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
if is_object_safe {
|
||||
err.span_suggestion(
|
||||
ret_ty,
|
||||
&format!(
|
||||
"use a boxed trait object if all return paths implement trait `{}`",
|
||||
trait_obj,
|
||||
),
|
||||
format!("Box<dyn {}>", trait_obj),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1305,7 +1305,6 @@ fn check_fn<'a, 'tcx>(
|
||||
let hir = tcx.hir();
|
||||
|
||||
let declared_ret_ty = fn_sig.output();
|
||||
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
|
||||
let revealed_ret_ty =
|
||||
fcx.instantiate_opaque_types_from_value(fn_id, &declared_ret_ty, decl.output.span());
|
||||
debug!("check_fn: declared_ret_ty: {}, revealed_ret_ty: {}", declared_ret_ty, revealed_ret_ty);
|
||||
@ -1374,7 +1373,25 @@ fn check_fn<'a, 'tcx>(
|
||||
|
||||
inherited.tables.borrow_mut().liberated_fn_sigs_mut().insert(fn_id, fn_sig);
|
||||
|
||||
fcx.check_return_expr(&body.value);
|
||||
if let ty::Dynamic(..) = declared_ret_ty.kind {
|
||||
// FIXME: We need to verify that the return type is `Sized` after the return expression has
|
||||
// been evaluated so that we have types available for all the nodes being returned, but that
|
||||
// requires the coerced evaluated type to be stored. Moving `check_return_expr` before this
|
||||
// causes unsized errors caused by the `declared_ret_ty` to point at the return expression,
|
||||
// while keeping the current ordering we will ignore the tail expression's type because we
|
||||
// don't know it yet. We can't do `check_expr_kind` while keeping `check_return_expr`
|
||||
// because we will trigger "unreachable expression" lints unconditionally.
|
||||
// Because of all of this, we perform a crude check to know whether the simplest `!Sized`
|
||||
// case that a newcomer might make, returning a bare trait, and in that case we populate
|
||||
// the tail expression's type so that the suggestion will be correct, but ignore all other
|
||||
// possible cases.
|
||||
fcx.check_expr(&body.value);
|
||||
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
|
||||
tcx.sess.delay_span_bug(decl.output.span(), "`!Sized` return type");
|
||||
} else {
|
||||
fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
|
||||
fcx.check_return_expr(&body.value);
|
||||
}
|
||||
|
||||
// We insert the deferred_generator_interiors entry after visiting the body.
|
||||
// This ensures that all nested generators appear before the entry of this generator.
|
||||
|
||||
@ -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 {
|
||||
| ^^^^^^^^^^
|
||||
|
||||
@ -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 {
|
||||
@ -26,7 +26,7 @@ fn bax() -> dyn Trait { //~ ERROR E0746
|
||||
if true {
|
||||
Struct
|
||||
} else {
|
||||
42
|
||||
42 //~ ERROR `if` and `else` have incompatible types
|
||||
}
|
||||
}
|
||||
fn bam() -> Box<dyn Trait> {
|
||||
|
||||
@ -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,29 @@ 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
|
||||
help: use some type `T` that is `T: Sized` as the return type if all return paths have the same type
|
||||
|
|
||||
LL | fn bak() -> T { unimplemented!() }
|
||||
| ^
|
||||
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
|
||||
|
|
||||
LL | fn bak() -> impl Trait { unimplemented!() }
|
||||
| ^^^^^^^^^^
|
||||
help: use a boxed trait object if all return paths implement 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
|
||||
@ -95,6 +104,18 @@ LL | }
|
||||
LL | Box::new(42)
|
||||
|
|
||||
|
||||
error[E0308]: `if` and `else` have incompatible types
|
||||
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:9
|
||||
|
|
||||
LL | / if true {
|
||||
LL | | Struct
|
||||
| | ------ expected because of this
|
||||
LL | | } else {
|
||||
LL | | 42
|
||||
| | ^^ expected struct `Struct`, found integer
|
||||
LL | | }
|
||||
| |_____- `if` and `else` have incompatible types
|
||||
|
||||
error[E0746]: return type cannot have an unboxed trait object
|
||||
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:25:13
|
||||
|
|
||||
@ -249,7 +270,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,12 +282,12 @@ 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 {
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: aborting due to 19 previous errors
|
||||
error: aborting due to 20 previous errors
|
||||
|
||||
Some errors have detailed explanations: E0277, E0308, E0746.
|
||||
For more information about an error, try `rustc --explain E0277`.
|
||||
|
||||
@ -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!()
|
||||
|
||||
@ -1,13 +1,22 @@
|
||||
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
|
||||
help: use some type `T` that is `T: Sized` as the return type if all return paths have the same type
|
||||
|
|
||||
LL | T
|
||||
|
|
||||
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
|
||||
|
|
||||
LL | impl AbstractRenderer
|
||||
|
|
||||
help: use a boxed trait object if all return paths implement 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`.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user