From 6fd564112f1ec00f6f8a56e8a3577dd255639131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 13 Jan 2020 13:13:12 -0800 Subject: [PATCH] Specific error for unsized `dyn Trait` return type Suggest `impl Trait` when possible, and `Box` otherwise. --- src/librustc/traits/error_reporting.rs | 201 +++++++++++++++++- src/librustc/traits/mod.rs | 11 + src/librustc_error_codes/error_codes.rs | 1 + src/test/ui/error-codes/E0746.rs | 17 ++ src/test/ui/error-codes/E0746.stderr | 62 ++++++ .../dyn-trait-return-should-be-impl-trait.rs | 36 ++++ ...n-trait-return-should-be-impl-trait.stderr | 186 ++++++++++++++++ 7 files changed, 512 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/error-codes/E0746.rs create mode 100644 src/test/ui/error-codes/E0746.stderr create mode 100644 src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs create mode 100644 src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 7f151af7abe..8f771658e40 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -1,3 +1,4 @@ +// ignore-tidy-filelength use super::{ ConstEvalFailure, EvaluationResult, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes, ObjectSafetyViolation, Obligation, ObligationCause, @@ -22,9 +23,12 @@ use crate::ty::TypeckTables; use crate::ty::{self, AdtKind, DefIdTree, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, Style}; +use rustc_errors::{ + error_code, pluralize, struct_span_err, Applicability, DiagnosticBuilder, Style, +}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; +use rustc_hir::intravisit::Visitor; use rustc_hir::Node; use rustc_span::source_map::SourceMap; use rustc_span::symbol::{kw, sym}; @@ -758,7 +762,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { )), Some( "the question mark operation (`?`) implicitly performs a \ - conversion on the error value using the `From` trait" + conversion on the error value using the `From` trait" .to_owned(), ), ) @@ -835,6 +839,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.suggest_remove_reference(&obligation, &mut err, &trait_ref); self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref); self.note_version_mismatch(&mut err, &trait_ref); + if self.suggest_impl_trait(&mut err, span, &obligation, &trait_ref) { + err.emit(); + return; + } // Try to report a help message if !trait_ref.has_infer_types() @@ -1696,6 +1704,195 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } + fn suggest_impl_trait( + &self, + err: &mut DiagnosticBuilder<'tcx>, + span: Span, + obligation: &PredicateObligation<'tcx>, + trait_ref: &ty::Binder>, + ) -> bool { + if let ObligationCauseCode::SizedReturnType = obligation.cause.code.peel_derives() { + } else { + return false; + } + + let hir = self.tcx.hir(); + let parent_node = hir.get_parent_node(obligation.cause.body_id); + let node = hir.find(parent_node); + if let Some(hir::Node::Item(hir::Item { + kind: hir::ItemKind::Fn(sig, _, body_id), .. + })) = node + { + let body = hir.body(*body_id); + let trait_ref = self.resolve_vars_if_possible(trait_ref); + let ty = trait_ref.skip_binder().self_ty(); + if let ty::Dynamic(..) = ty.kind { + } else { + // We only want to suggest `impl Trait` to `dyn Trait`s. + // For example, `fn foo() -> str` needs to be filtered out. + return false; + } + // Use `TypeVisitor` instead of the output type directly to find the span of `ty` for + // cases like `fn foo() -> (dyn Trait, i32) {}`. + // Recursively look for `TraitObject` types and if there's only one, use that span to + // suggest `impl Trait`. + + struct ReturnsVisitor<'v>(Vec<&'v hir::Expr<'v>>); + + impl<'v> Visitor<'v> for ReturnsVisitor<'v> { + type Map = rustc::hir::map::Map<'v>; + + fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap<'_, Self::Map> { + hir::intravisit::NestedVisitorMap::None + } + + fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { + match ex.kind { + hir::ExprKind::Ret(Some(ex)) => self.0.push(ex), + _ => {} + } + hir::intravisit::walk_expr(self, ex); + } + + fn visit_body(&mut self, body: &'v hir::Body<'v>) { + if body.generator_kind().is_none() { + if let hir::ExprKind::Block(block, None) = body.value.kind { + if let Some(expr) = block.expr { + self.0.push(expr); + } + } + } + hir::intravisit::walk_body(self, body); + } + } + + // Visit to make sure there's a single `return` type to suggest `impl Trait`, + // otherwise suggest using `Box` or an enum. + let mut visitor = ReturnsVisitor(vec![]); + visitor.visit_body(&body); + + let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); + + if let hir::FunctionRetTy::Return(ret_ty) = sig.decl.output { + let mut all_returns_conform_to_trait = true; + let mut all_returns_have_same_type = true; + let mut last_ty = None; + if let Some(ty_ret_ty) = tables.node_type_opt(ret_ty.hir_id) { + let cause = ObligationCause::misc(ret_ty.span, ret_ty.hir_id); + if let ty::Dynamic(predicates, _) = &ty_ret_ty.kind { + for predicate in predicates.iter() { + for expr in &visitor.0 { + if let Some(returned_ty) = tables.node_type_opt(expr.hir_id) { + if let Some(ty) = last_ty { + all_returns_have_same_type &= ty == returned_ty; + } + last_ty = Some(returned_ty); + + let param_env = ty::ParamEnv::empty(); + let pred = predicate.with_self_ty(self.tcx, returned_ty); + let obligation = + Obligation::new(cause.clone(), param_env, pred); + all_returns_conform_to_trait &= + self.predicate_may_hold(&obligation); + } + } + } + } + } else { + // We still want to verify whether all the return types conform to each other. + for expr in &visitor.0 { + if let Some(returned_ty) = tables.node_type_opt(expr.hir_id) { + if let Some(ty) = last_ty { + all_returns_have_same_type &= ty == returned_ty; + } + last_ty = Some(returned_ty); + } + } + } + + if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true, Some(last_ty)) = ( + ret_ty.span.overlaps(span), + &ret_ty.kind, + self.tcx.sess.source_map().span_to_snippet(ret_ty.span), + all_returns_conform_to_trait, + last_ty, + ) { + err.code = Some(error_code!(E0746)); + err.set_primary_message( + "return type cannot have a bare trait because it must be `Sized`", + ); + err.children.clear(); + let impl_trait_msg = "for information on `impl Trait`, see \ + "; + let trait_obj_msg = "for information on trait objects, see \ + "; + 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 { + err.span_suggestion( + ret_ty.span, + &format!( + "you can use the `impl Trait` feature \ + in the return type because all the return paths are of type \ + `{}`, which implements `dyn {}`", + last_ty, trait_obj, + ), + format!("impl {}", trait_obj), + Applicability::MaybeIncorrect, + ); + err.note(impl_trait_msg); + } else { + let mut suggestions = visitor + .0 + .iter() + .map(|expr| { + ( + expr.span, + format!( + "Box::new({})", + self.tcx + .sess + .source_map() + .span_to_snippet(expr.span) + .unwrap() + ), + ) + }) + .collect::>(); + suggestions.push(( + ret_ty.span, + format!("Box<{}{}>", if has_dyn { "" } else { "dyn " }, snippet), + )); + err.multipart_suggestion( + "if the performance implications are acceptable, you can return a \ + trait object", + suggestions, + Applicability::MaybeIncorrect, + ); + err.span_help( + visitor.0.iter().map(|expr| expr.span).collect::>(), + &format!( + "if all the returned values were of the same type you could use \ + `impl {}` as the return type", + trait_obj, + ), + ); + err.help( + "alternatively, you can always create a new `enum` with a variant \ + for each returned type", + ); + err.note(impl_trait_msg); + err.note(trait_obj_msg); + } + return true; + } + } + } + false + } + /// Given some node representing a fn-like thing in the HIR map, /// returns a span and `ArgKind` information that describes the /// arguments it expects. This can be supplied to diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 31de5409fc8..f68711c0620 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -1171,6 +1171,17 @@ impl<'tcx> ObligationCause<'tcx> { } } +impl<'tcx> ObligationCauseCode<'tcx> { + pub fn peel_derives(&self) -> &Self { + match self { + BuiltinDerivedObligation(cause) | ImplDerivedObligation(cause) => { + cause.parent_code.peel_derives() + } + _ => self, + } + } +} + impl<'tcx, N> Vtable<'tcx, N> { pub fn nested_obligations(self) -> Vec { match self { diff --git a/src/librustc_error_codes/error_codes.rs b/src/librustc_error_codes/error_codes.rs index 272147e28a4..c17cb7dd9f1 100644 --- a/src/librustc_error_codes/error_codes.rs +++ b/src/librustc_error_codes/error_codes.rs @@ -608,4 +608,5 @@ E0745: include_str!("./error_codes/E0745.md"), E0726, // non-explicit (not `'_`) elided lifetime in unsupported position E0727, // `async` generators are not yet supported E0739, // invalid track_caller application/syntax + E0746, // `dyn Trait` return type } diff --git a/src/test/ui/error-codes/E0746.rs b/src/test/ui/error-codes/E0746.rs new file mode 100644 index 00000000000..ad257b01e1b --- /dev/null +++ b/src/test/ui/error-codes/E0746.rs @@ -0,0 +1,17 @@ +struct Struct; +trait Trait {} +impl Trait for Struct {} +impl Trait for u32 {} + +fn foo() -> dyn Trait { Struct } +//~^ ERROR E0746 +//~| ERROR E0308 + +fn bar() -> dyn Trait { //~ ERROR E0746 + if true { + return 0; //~ ERROR E0308 + } + 42 //~ ERROR E0308 +} + +fn main() {} diff --git a/src/test/ui/error-codes/E0746.stderr b/src/test/ui/error-codes/E0746.stderr new file mode 100644 index 00000000000..baafcd27c29 --- /dev/null +++ b/src/test/ui/error-codes/E0746.stderr @@ -0,0 +1,62 @@ +error[E0308]: mismatched types + --> $DIR/E0746.rs:6:25 + | +LL | fn foo() -> dyn Trait { Struct } + | --------- ^^^^^^ expected trait `Trait`, found struct `Struct` + | | + | expected `(dyn Trait + 'static)` because of return type + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/E0746.rs:6:13 + | +LL | fn foo() -> dyn Trait { Struct } + | ^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on `impl Trait`, see +help: you can use the `impl Trait` feature in the return type because all the return paths are of type `Struct`, which implements `dyn Trait` + | +LL | fn foo() -> impl Trait { Struct } + | ^^^^^^^^^^ + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/E0746.rs:10:13 + | +LL | fn bar() -> dyn Trait { + | ^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on `impl Trait`, see +help: you can use the `impl Trait` feature in the return type because all the return paths are of type `{integer}`, which implements `dyn Trait` + | +LL | fn bar() -> impl Trait { + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/E0746.rs:12:16 + | +LL | fn bar() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +LL | if true { +LL | return 0; + | ^ expected trait `Trait`, found integer + | + = note: expected trait object `(dyn Trait + 'static)` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/E0746.rs:14:5 + | +LL | fn bar() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +... +LL | 42 + | ^^ expected trait `Trait`, found integer + | + = note: expected trait object `(dyn Trait + 'static)` + found type `{integer}` + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0308`. 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 new file mode 100644 index 00000000000..80168ca8257 --- /dev/null +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs @@ -0,0 +1,36 @@ +#![allow(bare_trait_objects)] +struct Struct; +trait Trait {} +impl Trait for Struct {} +impl Trait for u32 {} + +fn fuz() -> (usize, Trait) { (42, Struct) } +//~^ ERROR E0277 +//~| ERROR E0308 +fn bar() -> (usize, dyn Trait) { (42, Struct) } +//~^ ERROR E0277 +//~| ERROR E0308 +fn bap() -> Trait { Struct } +//~^ ERROR E0746 +//~| ERROR E0308 +fn ban() -> dyn Trait { Struct } +//~^ ERROR E0746 +//~| ERROR E0308 +fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0277 +// Suggest using `Box` +fn bal() -> dyn Trait { //~ ERROR E0746 + if true { + return Struct; //~ ERROR E0308 + } + 42 //~ ERROR E0308 +} + +// Suggest using `impl Trait` +fn bat() -> dyn Trait { //~ ERROR E0746 + if true { + return 0; //~ ERROR E0308 + } + 42 //~ ERROR E0308 +} + +fn main() {} 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 new file mode 100644 index 00000000000..ce4c141a0af --- /dev/null +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr @@ -0,0 +1,186 @@ +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:7:35 + | +LL | fn fuz() -> (usize, Trait) { (42, Struct) } + | ^^^^^^ expected trait `Trait`, found struct `Struct` + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:7:13 + | +LL | fn fuz() -> (usize, Trait) { (42, Struct) } + | ^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: within `(usize, (dyn Trait + 'static))`, the trait `std::marker::Sized` is not implemented for `(dyn Trait + 'static)` + = note: to learn more, visit + = note: required because it appears within the type `(usize, (dyn Trait + 'static))` + = note: the return type of a function must have a statically known size + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:10:39 + | +LL | fn bar() -> (usize, dyn Trait) { (42, Struct) } + | ^^^^^^ expected trait `Trait`, found struct `Struct` + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:10:13 + | +LL | fn bar() -> (usize, dyn Trait) { (42, Struct) } + | ^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: within `(usize, (dyn Trait + 'static))`, the trait `std::marker::Sized` is not implemented for `(dyn Trait + 'static)` + = note: to learn more, visit + = note: required because it appears within the type `(usize, (dyn Trait + 'static))` + = note: the return type of a function must have a statically known size + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:13:21 + | +LL | fn bap() -> Trait { Struct } + | ----- ^^^^^^ expected trait `Trait`, found struct `Struct` + | | + | expected `(dyn Trait + 'static)` because of return type + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:13:13 + | +LL | fn bap() -> Trait { Struct } + | ^^^^^ doesn't have a size known at compile-time + | + = note: for information on `impl Trait`, see +help: you can use the `impl Trait` feature in the return type because all the return paths are of type `Struct`, which implements `dyn Trait` + | +LL | fn bap() -> impl Trait { Struct } + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:16:25 + | +LL | fn ban() -> dyn Trait { Struct } + | --------- ^^^^^^ expected trait `Trait`, found struct `Struct` + | | + | expected `(dyn Trait + 'static)` because of return type + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:16:13 + | +LL | fn ban() -> dyn Trait { Struct } + | ^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on `impl Trait`, see +help: you can use the `impl Trait` feature in the return type because all the return paths are of type `Struct`, which implements `dyn Trait` + | +LL | fn ban() -> impl Trait { Struct } + | ^^^^^^^^^^ + +error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:19: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 + = note: the return type of a function must have a statically known size + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:23:16 + | +LL | fn bal() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +LL | if true { +LL | return Struct; + | ^^^^^^ expected trait `Trait`, found struct `Struct` + | + = note: expected trait object `(dyn Trait + 'static)` + found struct `Struct` + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:21:13 + | +LL | fn bal() -> dyn Trait { + | ^^^^^^^^^ doesn't have a size known at compile-time + | +help: if all the returned values were of the same type you could use `impl Trait` as the return type + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:25:5 + | +LL | return Struct; + | ^^^^^^ +LL | } +LL | 42 + | ^^ + = help: alternatively, you can always create a new `enum` with a variant for each returned type + = note: for information on `impl Trait`, see + = note: for information on trait objects, see +help: if the performance implications are acceptable, you can return a trait object + | +LL | fn bal() -> Box { +LL | if true { +LL | return Box::new(Struct); +LL | } +LL | Box::new(42) + | + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:25:5 + | +LL | fn bal() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +... +LL | 42 + | ^^ expected trait `Trait`, found integer + | + = note: expected trait object `(dyn Trait + 'static)` + found type `{integer}` + +error[E0746]: return type cannot have a bare trait because it must be `Sized` + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:13 + | +LL | fn bat() -> dyn Trait { + | ^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on `impl Trait`, see +help: you can use the `impl Trait` feature in the return type because all the return paths are of type `{integer}`, which implements `dyn Trait` + | +LL | fn bat() -> impl Trait { + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:31:16 + | +LL | fn bat() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +LL | if true { +LL | return 0; + | ^ expected trait `Trait`, found integer + | + = note: expected trait object `(dyn Trait + 'static)` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:33:5 + | +LL | fn bat() -> dyn Trait { + | --------- expected `(dyn Trait + 'static)` because of return type +... +LL | 42 + | ^^ expected trait `Trait`, found integer + | + = note: expected trait object `(dyn Trait + 'static)` + found type `{integer}` + +error: aborting due to 15 previous errors + +Some errors have detailed explanations: E0277, E0308. +For more information about an error, try `rustc --explain E0277`.