mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure tail expression will have a Ty for E0746

Browse Source 
When the return type is `!Sized` we look for all the returned
expressions in the body to fetch their types and provide a reasonable
suggestion. The tail expression of the body is normally evaluated after
checking whether the return type is `Sized`. Changing the order of the
evaluation produces undesirable knock down effects, so we detect the
specific case that newcomers are likely to encounter ,returning a single
bare trait object, and only in that case we evaluate the tail
expression's type so that the suggestion will be accurate.
This commit is contained in:
Esteban Küber 2020-04-11 11:31:54 -07:00
parent d3c96f03b5
commit e536257061
6 changed files with 113 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/librustc_error_codes/error_codes/E0746.md
View File

@ -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);
}

121
src/librustc_trait_selection/traits/error_reporting/suggestions.rs
View File

@ -13,7 +13,7 @@ use rustc_hir::intravisit::Visitor;
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 @@ 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, count) = ret_types.clone().fold(
(None, true, 0),
|(last_ty, mut same, count): (std::option::Option<Ty<'_>>, bool, usize), 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, count + 1)
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 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
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)
})
})
}) || count == 0
}
_ => false,
}
@ -861,7 +878,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&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.
// suggest `impl Trait` nor trait objects: it is a type mismatch error.
all_returns_conform_to_trait,
) {
snippet
@ -879,43 +896,15 @@ 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 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(
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,
"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,
trait_obj,
is_object_safe,
);
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(
@ -1851,3 +1840,39 @@ impl NextTypeParamName for &[hir::GenericParam<'_>] {
.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,
);
}
}

21
src/librustc_typeck/check/mod.rs
View File

@ -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.

2
src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs
View File

@ -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> {

23
src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
View File

@ -72,18 +72,15 @@ error[E0746]: return type cannot have an unboxed trait object
LL | fn bak() -> dyn Trait { unimplemented!() }
| ^^^^^^^^^ doesn't have a size known at compile-time
|
= 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
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: 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
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: you could use a boxed trait object if all return paths `impl` trait `Trait`
help: use a boxed trait object if all return paths implement trait `Trait`
|
LL | fn bak() -> Box<dyn Trait> { unimplemented!() }
| ^^^^^^^^^^^^^^
@ -107,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
|
@ -278,7 +287,7 @@ help: use `impl Trait` as the return type, as all return paths are of type `{int
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`.

9
src/test/ui/issues/issue-18107.stderr
View File

@ -4,18 +4,15 @@ error[E0746]: return type cannot have an unboxed trait object
LL | dyn AbstractRenderer
| ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= 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
help: use some type `T` that is `T: Sized` as the return type if all return paths 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
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: you could use a boxed trait object if all return paths `impl` trait `AbstractRenderer`
help: use a boxed trait object if all return paths implement trait `AbstractRenderer`
|
LL | Box<dyn AbstractRenderer>
|