From c8ee33714becbda0f1deddf1befe0383b4aad135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 17 Aug 2020 19:04:27 -0700 Subject: [PATCH 1/6] Use structured suggestion for `impl T` to `Box` --- compiler/rustc_typeck/src/check/coercion.rs | 12 ++--- ...trait-in-return-position-impl-trait.stderr | 5 ++- ...-to-type-err-cause-on-impl-trait-return.rs | 23 ++++------ ...type-err-cause-on-impl-trait-return.stderr | 44 +++++++++++++------ 4 files changed, 50 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index e1913011297..c19bc767563 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -37,7 +37,7 @@ use crate::astconv::AstConv; use crate::check::FnCtxt; -use rustc_errors::{struct_span_err, DiagnosticBuilder}; +use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{Coercion, InferOk, InferResult}; @@ -1523,10 +1523,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { }; if has_impl { if is_object_safe { - err.help(&format!( - "you can instead return a boxed trait object using `Box`", - &snippet[5..] - )); + err.span_suggestion_verbose( + return_sp, + "you could change the return type to be a boxed trait object", + format!("Box", &snippet[5..]), + Applicability::MachineApplicable, + ); } else { err.help(&format!( "if the trait `{}` were object safe, you could return a boxed trait object", diff --git a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr index dd4260fbe4f..a3bf2183255 100644 --- a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr +++ b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr @@ -30,9 +30,12 @@ LL | B | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn cat() -> Box { + | ^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs index a80e5df1a26..6114e8c72a3 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs @@ -2,16 +2,14 @@ fn foo() -> impl std::fmt::Display { if false { return 0i32; } - 1u32 - //~^ ERROR mismatched types + 1u32 //~ ERROR mismatched types } fn bar() -> impl std::fmt::Display { if false { return 0i32; } else { - return 1u32; - //~^ ERROR mismatched types + return 1u32; //~ ERROR mismatched types } } @@ -19,8 +17,7 @@ fn baz() -> impl std::fmt::Display { if false { return 0i32; } else { - 1u32 - //~^ ERROR mismatched types + 1u32 //~ ERROR mismatched types } } @@ -28,22 +25,19 @@ fn qux() -> impl std::fmt::Display { if false { 0i32 } else { - 1u32 - //~^ ERROR `if` and `else` have incompatible types + 1u32 //~ ERROR `if` and `else` have incompatible types } } fn bat() -> impl std::fmt::Display { match 13 { 0 => return 0i32, - _ => 1u32, - //~^ ERROR mismatched types + _ => 1u32, //~ ERROR mismatched types } } fn can() -> impl std::fmt::Display { - match 13 { - //~^ ERROR mismatched types + match 13 { //~ ERROR mismatched types 0 => return 0i32, 1 => 1u32, _ => 2u32, @@ -56,8 +50,9 @@ fn cat() -> impl std::fmt::Display { return 0i32; } _ => { - 1u32 - //~^ ERROR mismatched types + 1u32 //~ ERROR mismatched types + } + } } } } 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 b663cccbeef..4e4d57f35d2 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 @@ -12,12 +12,15 @@ LL | 1u32 | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn foo() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:13:16 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:12:16 | LL | fn bar() -> impl std::fmt::Display { | ---------------------- expected because this return type... @@ -30,12 +33,15 @@ LL | return 1u32; | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn bar() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:22:9 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:20:9 | LL | fn baz() -> impl std::fmt::Display { | ---------------------- expected because this return type... @@ -48,12 +54,15 @@ LL | 1u32 | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn baz() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: `if` and `else` have incompatible types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:31:9 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:9 | LL | / if false { LL | | 0i32 @@ -61,12 +70,11 @@ LL | | 0i32 LL | | } else { LL | | 1u32 | | ^^^^ expected `i32`, found `u32` -LL | | LL | | } | |_____- `if` and `else` have incompatible types error[E0308]: mismatched types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:39:14 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:35:14 | LL | fn bat() -> impl std::fmt::Display { | ---------------------- expected because this return type... @@ -78,17 +86,19 @@ LL | _ => 1u32, | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn bat() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:45:5 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:40:5 | LL | fn can() -> impl std::fmt::Display { | ---------------------- expected because this return type... LL | / match 13 { -LL | | LL | | 0 => return 0i32, | | ---- ...is found to be `i32` here LL | | 1 => 1u32, @@ -98,12 +108,15 @@ LL | | } | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn can() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:59:13 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:53:13 | LL | fn cat() -> impl std::fmt::Display { | ---------------------- expected because this return type... @@ -116,9 +129,12 @@ LL | 1u32 | = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see - = help: you can instead return a boxed trait object using `Box` = note: for information on trait objects, see = help: alternatively, create a new `enum` with a variant for each returned type +help: you could change the return type to be a boxed trait object + | +LL | fn cat() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 7 previous errors From fd9133b9c336e55d12bd5da1c610949473844dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 17 Aug 2020 22:39:46 -0700 Subject: [PATCH 2/6] Suggest boxed trait objects in tail `match` and `if` expressions When encountering a `match` or `if` as a tail expression where the different arms do not have the same type *and* the return type of that `fn` is an `impl Trait`, check whether those arms can implement `Trait` and if so, suggest using boxed trait objects. --- .../src/infer/error_reporting/mod.rs | 66 +++++++++++++- compiler/rustc_middle/src/traits/mod.rs | 3 + compiler/rustc_typeck/src/check/_match.rs | 85 +++++++++++++++++-- compiler/rustc_typeck/src/check/mod.rs | 18 ++++ ...-to-type-err-cause-on-impl-trait-return.rs | 8 +- ...type-err-cause-on-impl-trait-return.stderr | 35 +++++++- 6 files changed, 203 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 1225776db45..e772aacfbf9 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -617,11 +617,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ref prior_arms, last_ty, scrut_hir_id, + suggest_box, + arm_span, .. }) => match source { hir::MatchSource::IfLetDesugar { .. } => { let msg = "`if let` arms have incompatible types"; err.span_label(cause.span, msg); + if let Some(ret_sp) = suggest_box { + self.suggest_boxing_for_return_impl_trait( + err, + ret_sp, + prior_arms.iter().chain(std::iter::once(&arm_span)).map(|s| *s), + ); + } } hir::MatchSource::TryDesugar => { if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { @@ -675,9 +684,23 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); } + if let Some(ret_sp) = suggest_box { + // Get return type span and point to it. + self.suggest_boxing_for_return_impl_trait( + err, + ret_sp, + prior_arms.iter().chain(std::iter::once(&arm_span)).map(|s| *s), + ); + } } }, - ObligationCauseCode::IfExpression(box IfExpressionCause { then, outer, semicolon }) => { + ObligationCauseCode::IfExpression(box IfExpressionCause { + then, + else_sp, + outer, + semicolon, + suggest_box, + }) => { err.span_label(then, "expected because of this"); if let Some(sp) = outer { err.span_label(sp, "`if` and `else` have incompatible types"); @@ -690,11 +713,52 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); } + if let Some(ret_sp) = suggest_box { + self.suggest_boxing_for_return_impl_trait( + err, + ret_sp, + vec![then, else_sp].into_iter(), + ); + } } _ => (), } } + fn suggest_boxing_for_return_impl_trait( + &self, + err: &mut DiagnosticBuilder<'tcx>, + return_sp: Span, + arm_spans: impl Iterator, + ) { + let snippet = self + .tcx + .sess + .source_map() + .span_to_snippet(return_sp) + .unwrap_or_else(|_| "dyn Trait".to_string()); + err.span_suggestion_verbose( + return_sp, + "you could change the return type to be a boxed trait object", + format!("Box", &snippet[5..]), + Applicability::MaybeIncorrect, + ); + let sugg = arm_spans + .flat_map(|sp| { + vec![ + (sp.shrink_to_lo(), "Box::new(".to_string()), + (sp.shrink_to_hi(), ")".to_string()), + ] + .into_iter() + }) + .collect::>(); + err.multipart_suggestion( + "if you change the return type to expect trait objects box the returned expressions", + sugg, + Applicability::MaybeIncorrect, + ); + } + /// Given that `other_ty` is the same as a type argument for `name` in `sub`, populate `value` /// highlighting `name` and every type argument that isn't at `pos` (which is `other_ty`), and /// populate `other_value` with `other_ty`. diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index f86403fa502..71582b7ed73 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -350,13 +350,16 @@ pub struct MatchExpressionArmCause<'tcx> { pub prior_arms: Vec, pub last_ty: Ty<'tcx>, pub scrut_hir_id: hir::HirId, + pub suggest_box: Option, } #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct IfExpressionCause { pub then: Span, + pub else_sp: Span, pub outer: Option, pub semicolon: Option, + pub suggest_box: Option, } #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index afd4413069e..98e53a25879 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -1,12 +1,15 @@ use crate::check::coercion::CoerceMany; use crate::check::{Diverges, Expectation, FnCtxt, Needs}; -use rustc_hir as hir; -use rustc_hir::ExprKind; +use rustc_hir::{self as hir, ExprKind}; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; -use rustc_middle::ty::Ty; +use rustc_infer::traits::Obligation; +use rustc_middle::ty::{self, ToPredicate, Ty}; use rustc_span::Span; -use rustc_trait_selection::traits::ObligationCauseCode; -use rustc_trait_selection::traits::{IfExpressionCause, MatchExpressionArmCause, ObligationCause}; +use rustc_trait_selection::opaque_types::InferCtxtExt as _; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; +use rustc_trait_selection::traits::{ + IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, +}; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn check_match( @@ -14,7 +17,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &'tcx hir::Expr<'tcx>, scrut: &'tcx hir::Expr<'tcx>, arms: &'tcx [hir::Arm<'tcx>], - expected: Expectation<'tcx>, + orig_expected: Expectation<'tcx>, match_src: hir::MatchSource, ) -> Ty<'tcx> { let tcx = self.tcx; @@ -22,7 +25,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { use hir::MatchSource::*; let (source_if, if_no_else, force_scrutinee_bool) = match match_src { IfDesugar { contains_else_clause } => (true, !contains_else_clause, true), - IfLetDesugar { contains_else_clause } => (true, !contains_else_clause, false), + IfLetDesugar { contains_else_clause, .. } => (true, !contains_else_clause, false), WhileDesugar => (false, false, true), _ => (false, false, false), }; @@ -69,7 +72,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // type in that case) let mut all_arms_diverge = Diverges::WarnedAlways; - let expected = expected.adjust_for_branches(self); + let expected = orig_expected.adjust_for_branches(self); let mut coercion = { let coerce_first = match expected { @@ -112,6 +115,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.check_expr_with_expectation(&arm.body, expected) }; all_arms_diverge &= self.diverges.get(); + + // When we have a `match` as a tail expression in a `fn` with a returned `impl Trait` + // we check if the different arms would work with boxed trait objects instead and + // provide a structured suggestion in that case. + let suggest_box = match ( + orig_expected, + self.body_id + .owner + .to_def_id() + .as_local() + .and_then(|id| self.ret_coercion_impl_trait.map(|ty| (id, ty))), + ) { + (Expectation::ExpectHasType(expected), Some((id, ty))) + if self.in_tail_expr && self.can_coerce(arm_ty, expected) => + { + let impl_trait_ret_ty = self.infcx.instantiate_opaque_types( + id, + self.body_id, + self.param_env, + &ty, + arm.body.span, + ); + let mut suggest_box = !impl_trait_ret_ty.obligations.is_empty(); + for o in impl_trait_ret_ty.obligations { + match o.predicate.skip_binders_unchecked() { + ty::PredicateAtom::Trait(t, constness) => { + let pred = ty::PredicateAtom::Trait( + ty::TraitPredicate { + trait_ref: ty::TraitRef { + def_id: t.def_id(), + substs: self.infcx.tcx.mk_substs_trait(arm_ty, &[]), + }, + }, + constness, + ); + let obl = Obligation::new( + o.cause.clone(), + self.param_env, + pred.to_predicate(self.infcx.tcx), + ); + suggest_box &= self.infcx.predicate_must_hold_modulo_regions(&obl); + if !suggest_box { + break; + } + } + _ => {} + } + } + if suggest_box { self.ret_type_span.clone() } else { None } + } + _ => None, + }; + if source_if { let then_expr = &arms[0].body; match (i, if_no_else) { @@ -119,7 +175,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (_, true) => {} // Handled above to avoid duplicated type errors (#60254). (_, _) => { let then_ty = prior_arm_ty.unwrap(); - let cause = self.if_cause(expr.span, then_expr, &arm.body, then_ty, arm_ty); + let cause = self.if_cause( + expr.span, + then_expr, + &arm.body, + then_ty, + arm_ty, + suggest_box, + ); coercion.coerce(self, &cause, &arm.body, arm_ty); } } @@ -142,6 +205,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { prior_arms: other_arms.clone(), last_ty: prior_arm_ty.unwrap(), scrut_hir_id: scrut.hir_id, + suggest_box, }), ), }; @@ -266,6 +330,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { else_expr: &'tcx hir::Expr<'tcx>, then_ty: Ty<'tcx>, else_ty: Ty<'tcx>, + suggest_box: Option, ) -> ObligationCause<'tcx> { let mut outer_sp = if self.tcx.sess.source_map().is_multiline(span) { // The `if`/`else` isn't in one line in the output, include some context to make it @@ -353,8 +418,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { error_sp, ObligationCauseCode::IfExpression(box IfExpressionCause { then: then_sp, + else_sp: error_sp, outer: outer_sp, semicolon: remove_semicolon, + suggest_box, }), ) } diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index cb5f5731aa6..d6abf5f225d 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -570,6 +570,14 @@ pub struct FnCtxt<'a, 'tcx> { /// any). ret_coercion: Option>>, + ret_coercion_impl_trait: Option>, + + ret_type_span: Option, + + /// Used exclusively to reduce cost of advanced evaluation used for + /// more helpful diagnostics. + in_tail_expr: bool, + /// First span of a return site that we find. Used in error messages. ret_coercion_span: RefCell>, @@ -1302,10 +1310,15 @@ fn check_fn<'a, 'tcx>( let hir = tcx.hir(); let declared_ret_ty = fn_sig.output(); + 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); fcx.ret_coercion = Some(RefCell::new(CoerceMany::new(revealed_ret_ty))); + fcx.ret_type_span = Some(decl.output.span()); + if let ty::Opaque(..) = declared_ret_ty.kind() { + fcx.ret_coercion_impl_trait = Some(declared_ret_ty); + } fn_sig = tcx.mk_fn_sig( fn_sig.inputs().iter().cloned(), revealed_ret_ty, @@ -1366,6 +1379,7 @@ fn check_fn<'a, 'tcx>( inherited.typeck_results.borrow_mut().liberated_fn_sigs_mut().insert(fn_id, fn_sig); + fcx.in_tail_expr = true; 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 @@ -1385,6 +1399,7 @@ fn check_fn<'a, 'tcx>( fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType); fcx.check_return_expr(&body.value); } + fcx.in_tail_expr = false; // We insert the deferred_generator_interiors entry after visiting the body. // This ensures that all nested generators appear before the entry of this generator. @@ -3084,6 +3099,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { param_env, err_count_on_creation: inh.tcx.sess.err_count(), ret_coercion: None, + ret_coercion_impl_trait: None, + ret_type_span: None, + in_tail_expr: false, ret_coercion_span: RefCell::new(None), resume_yield_tys: None, ps: RefCell::new(UnsafetyState::function(hir::Unsafety::Normal, hir::CRATE_HIR_ID)), diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs index 6114e8c72a3..1a477e5db99 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs @@ -53,7 +53,13 @@ fn cat() -> impl std::fmt::Display { 1u32 //~ ERROR mismatched types } } - } +} + +fn dog() -> impl std::fmt::Display { + match 13 { + 0 => 0i32, + 1 => 1u32, //~ ERROR `match` arms have incompatible types + _ => 2u32, } } 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 4e4d57f35d2..a198203e245 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 @@ -72,6 +72,17 @@ LL | | 1u32 | | ^^^^ expected `i32`, found `u32` LL | | } | |_____- `if` and `else` have incompatible types + | +help: you could change the return type to be a boxed trait object + | +LL | fn qux() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: if you change the return type to expect trait objects box the returned expressions + | +LL | Box::new(0i32) +LL | } else { +LL | Box::new(1u32) + | error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:35:14 @@ -136,6 +147,28 @@ help: you could change the return type to be a boxed trait object LL | fn cat() -> Box { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error[E0308]: `match` arms have incompatible types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:61:14 + | +LL | / match 13 { +LL | | 0 => 0i32, + | | ---- this is found to be of type `i32` +LL | | 1 => 1u32, + | | ^^^^ expected `i32`, found `u32` +LL | | _ => 2u32, +LL | | } + | |_____- `match` arms have incompatible types + | +help: you could change the return type to be a boxed trait object + | +LL | fn dog() -> Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: if you change the return type to expect trait objects box the returned expressions + | +LL | 0 => Box::new(0i32), +LL | 1 => Box::new(1u32), + | + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0308`. From ff297fafbfb9af47bc75bc7eac9ff76d83fdd49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 18 Aug 2020 16:15:55 -0700 Subject: [PATCH 3/6] Make suggestion have a more targetted underline --- .../rustc_infer/src/infer/error_reporting/mod.rs | 16 ++++++---------- compiler/rustc_typeck/src/check/coercion.rs | 10 ++++++---- ...fe-trait-in-return-position-impl-trait.stderr | 2 +- ...to-type-err-cause-on-impl-trait-return.stderr | 16 ++++++++-------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index e772aacfbf9..fe53ccdbad5 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -70,7 +70,7 @@ use rustc_middle::ty::{ subst::{Subst, SubstsRef}, Region, Ty, TyCtxt, TypeFoldable, }; -use rustc_span::{DesugaringKind, Pos, Span}; +use rustc_span::{BytePos, DesugaringKind, Pos, Span}; use rustc_target::spec::abi; use std::{cmp, fmt}; @@ -731,16 +731,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { return_sp: Span, arm_spans: impl Iterator, ) { - let snippet = self - .tcx - .sess - .source_map() - .span_to_snippet(return_sp) - .unwrap_or_else(|_| "dyn Trait".to_string()); - err.span_suggestion_verbose( - return_sp, + err.multipart_suggestion( "you could change the return type to be a boxed trait object", - format!("Box", &snippet[5..]), + vec![ + (return_sp.with_hi(return_sp.lo() + BytePos(4)), "Box".to_string()), + ], Applicability::MaybeIncorrect, ); let sugg = arm_spans diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index c19bc767563..b669476483b 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -51,7 +51,7 @@ use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Ty, TypeAndMut}; use rustc_session::parse::feature_err; use rustc_span::symbol::sym; -use rustc_span::{self, Span}; +use rustc_span::{self, BytePos, Span}; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits::error_reporting::InferCtxtExt; use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode}; @@ -1523,10 +1523,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { }; if has_impl { if is_object_safe { - err.span_suggestion_verbose( - return_sp, + err.multipart_suggestion( "you could change the return type to be a boxed trait object", - format!("Box", &snippet[5..]), + vec![ + (return_sp.with_hi(return_sp.lo() + BytePos(4)), "Box".to_string()), + ], Applicability::MachineApplicable, ); } else { diff --git a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr index a3bf2183255..9abebeff95a 100644 --- a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr +++ b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr @@ -35,7 +35,7 @@ LL | B help: you could change the return type to be a boxed trait object | LL | fn cat() -> Box { - | ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error: aborting due to 2 previous errors 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 a198203e245..66d9ff307d9 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 @@ -17,7 +17,7 @@ LL | 1u32 help: you could change the return type to be a boxed trait object | LL | fn foo() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:12:16 @@ -38,7 +38,7 @@ LL | return 1u32; help: you could change the return type to be a boxed trait object | LL | fn bar() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:20:9 @@ -59,7 +59,7 @@ LL | 1u32 help: you could change the return type to be a boxed trait object | LL | fn baz() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:9 @@ -76,7 +76,7 @@ LL | | } help: you could change the return type to be a boxed trait object | LL | fn qux() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ help: if you change the return type to expect trait objects box the returned expressions | LL | Box::new(0i32) @@ -102,7 +102,7 @@ LL | _ => 1u32, help: you could change the return type to be a boxed trait object | LL | fn bat() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:40:5 @@ -124,7 +124,7 @@ LL | | } help: you could change the return type to be a boxed trait object | LL | fn can() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:53:13 @@ -145,7 +145,7 @@ LL | 1u32 help: you could change the return type to be a boxed trait object | LL | fn cat() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ error[E0308]: `match` arms have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:61:14 @@ -162,7 +162,7 @@ LL | | } help: you could change the return type to be a boxed trait object | LL | fn dog() -> Box { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ ^ help: if you change the return type to expect trait objects box the returned expressions | LL | 0 => Box::new(0i32), From 5d2a935e6cf25e0bc86354962929ee02513549b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 18 Aug 2020 16:44:45 -0700 Subject: [PATCH 4/6] Make suggestion more complete --- .../src/infer/error_reporting/mod.rs | 2 +- compiler/rustc_typeck/src/check/coercion.rs | 21 +++++++- src/test/ui/impl-trait/equality.stderr | 2 +- ...trait-in-return-position-impl-trait.stderr | 10 +++- ...type-err-cause-on-impl-trait-return.stderr | 54 ++++++++++++++++--- 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index fe53ccdbad5..ca6f4243c28 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -749,7 +749,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }) .collect::>(); err.multipart_suggestion( - "if you change the return type to expect trait objects box the returned expressions", + "if you change the return type to expect trait objects, box the returned expressions", sugg, Applicability::MaybeIncorrect, ); diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index b669476483b..4addee1a4c9 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1459,7 +1459,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { } } if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.borrow().as_ref(), fn_output) { - self.add_impl_trait_explanation(&mut err, fcx, expected, *sp, fn_output); + self.add_impl_trait_explanation(&mut err, cause, fcx, expected, *sp, fn_output); } err } @@ -1467,6 +1467,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fn add_impl_trait_explanation<'a>( &self, err: &mut DiagnosticBuilder<'a>, + cause: &ObligationCause<'tcx>, fcx: &FnCtxt<'a, 'tcx>, expected: Ty<'tcx>, sp: Span, @@ -1531,6 +1532,22 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ], Applicability::MachineApplicable, ); + let sugg = vec![sp, cause.span] + .into_iter() + .flat_map(|sp| { + vec![ + (sp.shrink_to_lo(), "Box::new(".to_string()), + (sp.shrink_to_hi(), ")".to_string()), + ] + .into_iter() + }) + .collect::>(); + err.multipart_suggestion( + "if you change the return type to expect trait objects, box the returned \ + expressions", + sugg, + Applicability::MaybeIncorrect, + ); } else { err.help(&format!( "if the trait `{}` were object safe, you could return a boxed trait object", @@ -1539,7 +1556,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { } err.note(trait_obj_msg); } - err.help("alternatively, create a new `enum` with a variant for each returned type"); + err.help("you could instead 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 { diff --git a/src/test/ui/impl-trait/equality.stderr b/src/test/ui/impl-trait/equality.stderr index 9667a9785dc..cdaa61ac323 100644 --- a/src/test/ui/impl-trait/equality.stderr +++ b/src/test/ui/impl-trait/equality.stderr @@ -23,7 +23,7 @@ LL | 0_u32 = note: for information on `impl Trait`, see = help: if the trait `Foo` were object safe, you could return a boxed trait object = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead 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/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr index 9abebeff95a..66043267f91 100644 --- a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr +++ b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr @@ -14,7 +14,7 @@ LL | B = note: for information on `impl Trait`, see = help: if the trait `NotObjectSafe` were object safe, you could return a boxed trait object = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type error[E0308]: mismatched types --> $DIR/object-unsafe-trait-in-return-position-impl-trait.rs:43:5 @@ -31,11 +31,17 @@ LL | B = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn cat() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | return Box::new(A); +LL | } +LL | Box::new(B) + | error: aborting due to 2 previous errors 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 66d9ff307d9..4265381eb40 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 @@ -13,11 +13,17 @@ LL | 1u32 = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn foo() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | return Box::new(0i32); +LL | } +LL | Box::new(1u32) + | error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:12:16 @@ -34,11 +40,17 @@ LL | return 1u32; = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn bar() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | return Box::new(0i32); +LL | } else { +LL | return Box::new(1u32); + | error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:20:9 @@ -55,11 +67,17 @@ LL | 1u32 = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn baz() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | return Box::new(0i32); +LL | } else { +LL | Box::new(1u32) + | error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:9 @@ -77,7 +95,7 @@ help: you could change the return type to be a boxed trait object | LL | fn qux() -> Box { | ^^^^^^^ ^ -help: if you change the return type to expect trait objects box the returned expressions +help: if you change the return type to expect trait objects, box the returned expressions | LL | Box::new(0i32) LL | } else { @@ -98,11 +116,16 @@ LL | _ => 1u32, = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn bat() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | 0 => return Box::new(0i32), +LL | _ => Box::new(1u32), + | error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:40:5 @@ -120,11 +143,19 @@ LL | | } = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn can() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | Box::new(match 13 { +LL | 0 => return Box::new(0i32), +LL | 1 => 1u32, +LL | _ => 2u32, +LL | }) + | error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:53:13 @@ -141,11 +172,18 @@ LL | 1u32 = note: to return `impl Trait`, all returned values must be of the same type = note: for information on `impl Trait`, see = note: for information on trait objects, see - = help: alternatively, create a new `enum` with a variant for each returned type + = help: you could instead create a new `enum` with a variant for each returned type help: you could change the return type to be a boxed trait object | LL | fn cat() -> Box { | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | return Box::new(0i32); +LL | } +LL | _ => { +LL | Box::new(1u32) + | error[E0308]: `match` arms have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:61:14 @@ -163,7 +201,7 @@ help: you could change the return type to be a boxed trait object | LL | fn dog() -> Box { | ^^^^^^^ ^ -help: if you change the return type to expect trait objects box the returned expressions +help: if you change the return type to expect trait objects, box the returned expressions | LL | 0 => Box::new(0i32), LL | 1 => Box::new(1u32), From dc53cfea7eb75b941e05e45fc7d8c412fd2a1bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 22 Aug 2020 17:24:26 -0700 Subject: [PATCH 5/6] Add test cases and address review comments --- .../src/infer/error_reporting/mod.rs | 10 +- compiler/rustc_middle/src/traits/mod.rs | 4 +- compiler/rustc_typeck/src/check/_match.rs | 16 +-- ...-to-type-err-cause-on-impl-trait-return.rs | 35 ++++++ ...type-err-cause-on-impl-trait-return.stderr | 110 +++++++++++++++++- 5 files changed, 156 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index ca6f4243c28..bcfcee23d13 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -617,14 +617,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ref prior_arms, last_ty, scrut_hir_id, - suggest_box, + opt_suggest_box_span, arm_span, .. }) => match source { hir::MatchSource::IfLetDesugar { .. } => { let msg = "`if let` arms have incompatible types"; err.span_label(cause.span, msg); - if let Some(ret_sp) = suggest_box { + if let Some(ret_sp) = opt_suggest_box_span { self.suggest_boxing_for_return_impl_trait( err, ret_sp, @@ -684,7 +684,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); } - if let Some(ret_sp) = suggest_box { + if let Some(ret_sp) = opt_suggest_box_span { // Get return type span and point to it. self.suggest_boxing_for_return_impl_trait( err, @@ -699,7 +699,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { else_sp, outer, semicolon, - suggest_box, + opt_suggest_box_span, }) => { err.span_label(then, "expected because of this"); if let Some(sp) = outer { @@ -713,7 +713,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); } - if let Some(ret_sp) = suggest_box { + if let Some(ret_sp) = opt_suggest_box_span { self.suggest_boxing_for_return_impl_trait( err, ret_sp, diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 71582b7ed73..9db3b57a5c2 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -350,7 +350,7 @@ pub struct MatchExpressionArmCause<'tcx> { pub prior_arms: Vec, pub last_ty: Ty<'tcx>, pub scrut_hir_id: hir::HirId, - pub suggest_box: Option, + pub opt_suggest_box_span: Option, } #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -359,7 +359,7 @@ pub struct IfExpressionCause { pub else_sp: Span, pub outer: Option, pub semicolon: Option, - pub suggest_box: Option, + pub opt_suggest_box_span: Option, } #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 98e53a25879..08cf178c813 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -119,13 +119,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // When we have a `match` as a tail expression in a `fn` with a returned `impl Trait` // we check if the different arms would work with boxed trait objects instead and // provide a structured suggestion in that case. - let suggest_box = match ( + let opt_suggest_box_span = match ( orig_expected, - self.body_id - .owner - .to_def_id() - .as_local() - .and_then(|id| self.ret_coercion_impl_trait.map(|ty| (id, ty))), + self.ret_coercion_impl_trait.map(|ty| (self.body_id.owner, ty)), ) { (Expectation::ExpectHasType(expected), Some((id, ty))) if self.in_tail_expr && self.can_coerce(arm_ty, expected) => @@ -181,7 +177,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &arm.body, then_ty, arm_ty, - suggest_box, + opt_suggest_box_span, ); coercion.coerce(self, &cause, &arm.body, arm_ty); } @@ -205,7 +201,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { prior_arms: other_arms.clone(), last_ty: prior_arm_ty.unwrap(), scrut_hir_id: scrut.hir_id, - suggest_box, + opt_suggest_box_span, }), ), }; @@ -330,7 +326,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { else_expr: &'tcx hir::Expr<'tcx>, then_ty: Ty<'tcx>, else_ty: Ty<'tcx>, - suggest_box: Option, + opt_suggest_box_span: Option, ) -> ObligationCause<'tcx> { let mut outer_sp = if self.tcx.sess.source_map().is_multiline(span) { // The `if`/`else` isn't in one line in the output, include some context to make it @@ -421,7 +417,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { else_sp: error_sp, outer: outer_sp, semicolon: remove_semicolon, - suggest_box, + opt_suggest_box_span, }), ) } diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs index 1a477e5db99..fa7664a83ee 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs @@ -63,4 +63,39 @@ fn dog() -> impl std::fmt::Display { } } +fn hat() -> dyn std::fmt::Display { //~ ERROR return type cannot have an unboxed trait object + match 13 { + 0 => { + return 0i32; + } + _ => { + 1u32 + } + } +} + +fn pug() -> dyn std::fmt::Display { //~ ERROR return type cannot have an unboxed trait object + match 13 { + 0 => 0i32, + 1 => 1u32, //~ ERROR `match` arms have incompatible types + _ => 2u32, + } +} + +fn man() -> dyn std::fmt::Display { //~ ERROR return type cannot have an unboxed trait object + if false { + 0i32 + } else { + 1u32 //~ ERROR `if` and `else` have incompatible types + } +} + +fn apt() -> impl std::fmt::Display { + if let Some(42) = Some(42) { + 0i32 + } else { + 1u32 //~ ERROR `if` and `else` have incompatible types + } +} + fn main() {} 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 4265381eb40..eb4dc45c8a9 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 @@ -207,6 +207,112 @@ LL | 0 => Box::new(0i32), LL | 1 => Box::new(1u32), | -error: aborting due to 8 previous errors +error[E0308]: `if` and `else` have incompatible types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:97:9 + | +LL | / if let Some(42) = Some(42) { +LL | | 0i32 + | | ---- expected because of this +LL | | } else { +LL | | 1u32 + | | ^^^^ expected `i32`, found `u32` +LL | | } + | |_____- `if` and `else` have incompatible types + | +help: you could change the return type to be a boxed trait object + | +LL | fn apt() -> Box { + | ^^^^^^^ ^ +help: if you change the return type to expect trait objects, box the returned expressions + | +LL | Box::new(0i32) +LL | } else { +LL | Box::new(1u32) + | -For more information about this error, try `rustc --explain E0308`. +error[E0746]: return type cannot have an unboxed trait object + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:66:13 + | +LL | fn hat() -> dyn std::fmt::Display { + | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on trait objects, see + = note: if all the returned values were of the same type you could use `impl std::fmt::Display` as the return type + = note: for information on `impl Trait`, see + = note: you can create a new `enum` with a variant for each returned type +help: return a boxed trait object instead + | +LL | fn hat() -> Box { +LL | match 13 { +LL | 0 => { +LL | return Box::new(0i32); +LL | } +LL | _ => { + ... + +error[E0308]: `match` arms have incompatible types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:80:14 + | +LL | / match 13 { +LL | | 0 => 0i32, + | | ---- this is found to be of type `i32` +LL | | 1 => 1u32, + | | ^^^^ expected `i32`, found `u32` +LL | | _ => 2u32, +LL | | } + | |_____- `match` arms have incompatible types + +error[E0746]: return type cannot have an unboxed trait object + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:77:13 + | +LL | fn pug() -> dyn std::fmt::Display { + | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on trait objects, see + = note: if all the returned values were of the same type you could use `impl std::fmt::Display` as the return type + = note: for information on `impl Trait`, see + = note: you can create a new `enum` with a variant for each returned type +help: return a boxed trait object instead + | +LL | fn pug() -> Box { +LL | match 13 { +LL | 0 => Box::new(0i32), +LL | 1 => Box::new(1u32), +LL | _ => Box::new(2u32), + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:89:9 + | +LL | / if false { +LL | | 0i32 + | | ---- expected because of this +LL | | } else { +LL | | 1u32 + | | ^^^^ expected `i32`, found `u32` +LL | | } + | |_____- `if` and `else` have incompatible types + +error[E0746]: return type cannot have an unboxed trait object + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:85:13 + | +LL | fn man() -> dyn std::fmt::Display { + | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on trait objects, see + = note: if all the returned values were of the same type you could use `impl std::fmt::Display` as the return type + = note: for information on `impl Trait`, see + = note: you can create a new `enum` with a variant for each returned type +help: return a boxed trait object instead + | +LL | fn man() -> Box { +LL | if false { +LL | Box::new(0i32) +LL | } else { +LL | Box::new(1u32) + | + +error: aborting due to 14 previous errors + +Some errors have detailed explanations: E0308, E0746. +For more information about an error, try `rustc --explain E0308`. From c6f2ddf1cb1e328245da59a4fb42f90403389a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 11 Sep 2020 17:20:56 -0700 Subject: [PATCH 6/6] Fix rebase and add comments --- compiler/rustc_typeck/src/check/_match.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 08cf178c813..836a4ff78c7 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -6,7 +6,7 @@ use rustc_infer::traits::Obligation; use rustc_middle::ty::{self, ToPredicate, Ty}; use rustc_span::Span; use rustc_trait_selection::opaque_types::InferCtxtExt as _; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, }; @@ -153,12 +153,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); suggest_box &= self.infcx.predicate_must_hold_modulo_regions(&obl); if !suggest_box { + // We've encountered some obligation that didn't hold, so the + // return expression can't just be boxed. We don't need to + // evaluate the rest of the obligations. break; } } _ => {} } } + // If all the obligations hold (or there are no obligations) the tail expression + // we can suggest to return a boxed trait object instead of an opaque type. if suggest_box { self.ret_type_span.clone() } else { None } } _ => None,