diff --git a/src/librustc/traits/error_reporting/mod.rs b/src/librustc/traits/error_reporting/mod.rs index 17d7b75a7f7..db3173989ac 100644 --- a/src/librustc/traits/error_reporting/mod.rs +++ b/src/librustc/traits/error_reporting/mod.rs @@ -25,7 +25,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; -use rustc_hir::intravisit::Visitor; use rustc_span::source_map::SourceMap; use rustc_span::{ExpnKind, Span, DUMMY_SP}; use std::fmt; @@ -1411,34 +1410,3 @@ pub fn suggest_constraining_type_param( } false } - -/// Collect all the returned expressions within the input expression. -/// Used to point at the return spans when we want to suggest some change to them. -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); - } -} diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index c2f562b4bc7..7c1b1041c34 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -563,157 +563,159 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { 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), .. + let (sig, body_id) = 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(); - let is_object_safe; - match ty.kind { - ty::Dynamic(predicates, _) => { - // The `dyn Trait` is not object safe, do not suggest `Box`. - is_object_safe = predicates.principal_def_id().map_or(true, |def_id| { - !object_safety_violations(self.tcx, def_id).is_empty() - }) - } - // We only want to suggest `impl Trait` to `dyn Trait`s. - // For example, `fn foo() -> str` needs to be filtered out. - _ => return false, + (sig, body_id) + } else { + return false; + }; + let body = hir.body(*body_id); + let trait_ref = self.resolve_vars_if_possible(trait_ref); + let ty = trait_ref.skip_binder().self_ty(); + let is_object_safe = match ty.kind { + ty::Dynamic(predicates, _) => { + // If the `dyn Trait` is not object safe, do not suggest `Box`. + predicates + .principal_def_id() + .map_or(true, |def_id| object_safety_violations(self.tcx, def_id).is_empty()) } + // We only want to suggest `impl Trait` to `dyn Trait`s. + // For example, `fn foo() -> str` needs to be filtered out. + _ => return false, + }; - let ret_ty = if let hir::FunctionRetTy::Return(ret_ty) = sig.decl.output { - ret_ty - } else { - return false; - }; + let ret_ty = if let hir::FunctionRetTy::Return(ret_ty) = sig.decl.output { + ret_ty + } else { + 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`. + // 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`. - // 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); + // 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(); + let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); - 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); - let param_env = ty::ParamEnv::empty(); - if let ty::Dynamic(predicates, _) = &ty_ret_ty.kind { - for expr in &visitor.0 { - if let Some(returned_ty) = tables.node_type_opt(expr.hir_id) { - all_returns_have_same_type &= - Some(returned_ty) == last_ty || last_ty.is_none(); - last_ty = Some(returned_ty); - for predicate in predicates.iter() { - let pred = predicate.with_self_ty(self.tcx, returned_ty); - let obl = Obligation::new(cause.clone(), param_env, pred); - all_returns_conform_to_trait &= self.predicate_may_hold(&obl); - } + 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); + let param_env = ty::ParamEnv::empty(); + if let ty::Dynamic(predicates, _) = &ty_ret_ty.kind { + for expr in &visitor.0 { + if let Some(returned_ty) = tables.node_type_opt(expr.hir_id) { + all_returns_have_same_type &= + Some(returned_ty) == last_ty || last_ty.is_none(); + last_ty = Some(returned_ty); + for predicate in predicates.iter() { + let pred = predicate.with_self_ty(self.tcx, returned_ty); + let obl = Obligation::new(cause.clone(), param_env, pred); + all_returns_conform_to_trait &= self.predicate_may_hold(&obl); } } } - } else { - // We still want to verify whether all the return types conform to each other. - for expr in &visitor.0 { - let returned_ty = tables.node_type_opt(expr.hir_id); - all_returns_have_same_type &= last_ty == returned_ty || last_ty.is_none(); - last_ty = returned_ty; - } } + } else { + // We still want to verify whether all the return types conform to each other. + for expr in &visitor.0 { + let returned_ty = tables.node_type_opt(expr.hir_id); + all_returns_have_same_type &= last_ty == returned_ty || last_ty.is_none(); + last_ty = returned_ty; + } + } - 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, - self.tcx.sess.source_map().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; - }; - err.code(error_code!(E0746)); - err.set_primary_message("return type cannot have an unboxed trait object"); - 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 { - // Suggest `-> impl Trait`. - err.span_suggestion( - ret_ty.span, - &format!( - "return `impl {1}` instead, as all return paths are of type `{}`, \ - which implements `{1}`", - last_ty, trait_obj, - ), - format!("impl {}", trait_obj), - Applicability::MachineApplicable, - ); - err.note(impl_trait_msg); + 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, + self.tcx.sess.source_map().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 { - if is_object_safe { - // Suggest `-> Box` and `Box::new(returned_value)`. - // Get all the return values and collect their span and suggestion. - 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::>(); - // Add the suggestion for the return type. - suggestions.push(( - ret_ty.span, - format!("Box<{}{}>", if has_dyn { "" } else { "dyn " }, snippet), - )); - err.multipart_suggestion( + return false; + }; + err.code(error_code!(E0746)); + err.set_primary_message("return type cannot have an unboxed trait object"); + 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 { + // Suggest `-> impl Trait`. + err.span_suggestion( + ret_ty.span, + &format!( + "return `impl {1}` instead, as all return paths are of type `{}`, \ + which implements `{1}`", + last_ty, trait_obj, + ), + format!("impl {}", trait_obj), + Applicability::MachineApplicable, + ); + err.note(impl_trait_msg); + } else { + if is_object_safe { + // Suggest `-> Box` and `Box::new(returned_value)`. + // Get all the return values and collect their span and suggestion. + 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::>(); + // Add the suggestion for the return type. + suggestions.push(( + ret_ty.span, + format!("Box<{}{}>", if has_dyn { "" } else { "dyn " }, snippet), + )); + err.multipart_suggestion( "return a trait object instead", - suggestions, - Applicability::MaybeIncorrect, - ); - } else { - err.note(&format!( - "if trait `{}` was object safe, you could return a trait object", - trait_obj, - )); - } + suggestions, + Applicability::MaybeIncorrect, + ); + } else { err.note(&format!( - "if all the returned values were of the same type you could use \ - `impl {}` as the return type", + "if trait `{}` was object safe, you could return a trait object", trait_obj, )); - err.note(impl_trait_msg); - err.note(trait_obj_msg); - err.note("you can create a new `enum` with a variant for each returned type"); } - return true; + 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", + trait_obj, + )); + err.note(impl_trait_msg); + err.note("you can create a new `enum` with a variant for each returned type"); } - false + true } crate fn point_at_returns_when_relevant( @@ -1686,6 +1688,8 @@ pub fn suggest_constraining_type_param( false } +/// Collect all the returned expressions within the input expression. +/// Used to point at the return spans when we want to suggest some change to them. struct ReturnsVisitor<'v>(Vec<&'v hir::Expr<'v>>); impl<'v> Visitor<'v> for ReturnsVisitor<'v> { diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index b4998d4486f..2e5da2b0382 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -1171,7 +1171,7 @@ impl<'tcx> ObligationCause<'tcx> { } } -impl<'tcx> ObligationCauseCode<'tcx> { +impl ObligationCauseCode<'_> { // Return the base obligation, ignoring derived obligations. pub fn peel_derives(&self) -> &Self { let mut base_cause = self; diff --git a/src/librustc_error_codes/error_codes/E0746.md b/src/librustc_error_codes/error_codes/E0746.md index 2df27bcf0bf..041061f3380 100644 --- a/src/librustc_error_codes/error_codes/E0746.md +++ b/src/librustc_error_codes/error_codes/E0746.md @@ -12,8 +12,8 @@ impl T for S { fn bar(&self) {} } -// Having the trait `T` as return type is invalid because bare traits do not -// have a statically known size: +// Having the trait `T` as return type is invalid because +// bare trait objects do not have a statically known size: fn foo() -> dyn T { S(42) } @@ -32,15 +32,15 @@ If there is a single type involved, you can use [`impl Trait`]: # fn bar(&self) {} # } // The compiler will select `S(usize)` as the materialized return type of this -// function, but callers will only be able to access associated items from `T`. +// function, but callers will only know that the return type implements `T`. fn foo() -> impl T { S(42) } ``` If there are multiple types involved, the only way you care to interact with -them is through the trait's interface and having to rely on dynamic dispatch is -acceptable, then you can use [trait objects] with `Box`, or other container +them is through the trait's interface, and having to rely on dynamic dispatch +is acceptable, then you can use [trait objects] with `Box`, or other container types like `Rc` or `Arc`: ``` diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 8aa2cb50342..768e532fa3b 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -68,7 +68,7 @@ use rustc_error_codes::*; use rustc_errors::{struct_span_err, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::DefId; -use rustc_span; +use rustc_span::{self, Span}; use rustc_span::symbol::sym; use rustc_target::spec::abi::Abi; use smallvec::{smallvec, SmallVec}; @@ -1352,41 +1352,50 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { } } if let (Some(sp), Some(return_sp)) = (fcx.ret_coercion_span.borrow().as_ref(), return_sp) { - err.span_label(return_sp, "expected because this return type..."); - err.span_label( *sp, format!( - "...is found to be `{}` here", - fcx.resolve_vars_with_obligations(expected), - )); - err.note("to return `impl Trait`, all returned values must be of the same type"); - let snippet = fcx - .tcx - .sess - .source_map() - .span_to_snippet(return_sp) - .unwrap_or_else(|_| "dyn Trait".to_string()); - let mut snippet_iter = snippet.split_whitespace(); - let has_impl = snippet_iter.next().map_or(false, |s| s == "impl"); - if has_impl { - err.help(&format!( - "you can instead return a trait object using `Box`", - &snippet[5..] - )); - } - err.help("alternatively, create a new `enum` with a variant for each returned type"); - let impl_trait_msg = "for information on `impl Trait`, see \ - "; - let trait_obj_msg = "for information on trait objects, see \ - "; - err.note(impl_trait_msg); - if has_impl { - err.note(trait_obj_msg); - } + self.add_impl_trait_explanation(&mut err, fcx, expected, *sp, return_sp); } err } + fn add_impl_trait_explanation<'a>( + &self, + err: &mut DiagnosticBuilder<'a>, + fcx: &FnCtxt<'a, 'tcx>, + expected: Ty<'tcx>, + sp: Span, + return_sp: Span, + ) { + err.span_label(return_sp, "expected because this return type..."); + err.span_label( + sp, + format!("...is found to be `{}` here", fcx.resolve_vars_with_obligations(expected)), + ); + let impl_trait_msg = "for information on `impl Trait`, see \ + "; + let trait_obj_msg = "for information on trait objects, see \ + "; + err.note("to return `impl Trait`, all returned values must be of the same type"); + err.note(impl_trait_msg); + let snippet = fcx + .tcx + .sess + .source_map() + .span_to_snippet(return_sp) + .unwrap_or_else(|_| "dyn Trait".to_string()); + let mut snippet_iter = snippet.split_whitespace(); + let has_impl = snippet_iter.next().map_or(false, |s| s == "impl"); + if has_impl { + err.help(&format!( + "you can instead return a trait object using `Box`", + &snippet[5..] + )); + err.note(trait_obj_msg); + } + err.help("alternatively, create a new `enum` with a variant for each returned type"); + } + fn is_return_ty_unsized(&self, fcx: &FnCtxt<'a, 'tcx>, blk_id: hir::HirId) -> bool { if let Some((fn_decl, _)) = fcx.get_fn_decl(blk_id) { if let hir::FunctionRetTy::Return(ty) = fn_decl.output { 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 index 3d0707c0916..ac101f8f3ce 100644 --- 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 @@ -82,11 +82,18 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bal() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | - = note: if trait `Trait` was object safe, you could return a trait object + = note: for information on trait objects, see = note: if all the returned values were of the same type you could use `impl Trait` as the return type = note: for information on `impl Trait`, see - = note: for information on trait objects, see = note: you can create a new `enum` with a variant for each returned type +help: return a trait object instead + | +LL | fn bal() -> Box { +LL | if true { +LL | return Box::new(Struct); +LL | } +LL | Box::new(42) + | error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:27:13 diff --git a/src/test/ui/impl-trait/equality.stderr b/src/test/ui/impl-trait/equality.stderr index ceb32dd4cd3..a399fadbc5d 100644 --- a/src/test/ui/impl-trait/equality.stderr +++ b/src/test/ui/impl-trait/equality.stderr @@ -11,10 +11,10 @@ LL | 0_u32 | ^^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0277]: cannot add `impl Foo` to `u32` --> $DIR/equality.rs:24:11 diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr index 87daab5ca7a..9859c73b7b1 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr @@ -11,10 +11,10 @@ LL | 1u32 | ^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:13:16 @@ -29,10 +29,10 @@ LL | return 1u32; | ^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:22:9 @@ -47,10 +47,10 @@ LL | 1u32 | ^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:31:9 @@ -77,10 +77,10 @@ LL | _ => 1u32, | ^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:45:5 @@ -97,10 +97,10 @@ LL | | } | |_____^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:59:13 @@ -115,10 +115,10 @@ LL | 1u32 | ^^^^ expected `i32`, found `u32` | = note: to return `impl Trait`, all returned values must be of the same type - = help: you can instead return a trait object using `Box` - = help: alternatively, create a new `enum` with a variant for each returned type = note: for information on `impl Trait`, see + = help: you can instead return a trait object using `Box` = note: for information on trait objects, see + = help: alternatively, create a new `enum` with a variant for each returned type error: aborting due to 7 previous errors