From 31cdd8cdd2a20dbdeae3d3f79d5f9080d431a87d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 16 Sep 2021 16:53:40 -0500 Subject: [PATCH] Propagate coercion cause into `try_coerce` Currently, `coerce_inner` discards its `ObligationCause` when calling `try_coerce`. This interfers with other diagnostc improvements I'm working on, since we will lose the original span by the time the actual coercion occurs. Additionally, we now use the span of the trailing expression (rather than the span of the entire function) when performing a coercion in `check_return_expr`. This currently has no visible effect on any of the unit tests, but will unblock future diagnostic improvements. --- compiler/rustc_typeck/src/check/cast.rs | 9 ++++++-- compiler/rustc_typeck/src/check/check.rs | 2 +- compiler/rustc_typeck/src/check/coercion.rs | 12 ++++++++-- compiler/rustc_typeck/src/check/demand.rs | 2 +- compiler/rustc_typeck/src/check/expr.rs | 22 ++++++++++++++++--- src/test/ui/issues/issue-55796.stderr | 4 ++-- src/test/ui/issues/issue-75777.stderr | 2 +- src/test/ui/nll/issue-55394.stderr | 2 +- .../ui/nll/type-alias-free-regions.stderr | 4 ++-- .../object-lifetime-default-elision.stderr | 4 ++-- .../region-object-lifetime-in-coercion.stderr | 2 +- ...-close-over-type-parameter-multiple.stderr | 2 +- .../ui/regions/regions-creating-enums4.stderr | 2 +- .../ui/regions/regions-ret-borrowed-1.stderr | 2 +- .../ui/regions/regions-ret-borrowed.stderr | 2 +- .../regions-trait-object-subtyping.stderr | 2 +- 16 files changed, 52 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_typeck/src/check/cast.rs b/compiler/rustc_typeck/src/check/cast.rs index 14550690e63e0..4ea7a8694c075 100644 --- a/compiler/rustc_typeck/src/check/cast.rs +++ b/compiler/rustc_typeck/src/check/cast.rs @@ -362,6 +362,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { ), self.cast_ty, AllowTwoPhase::No, + None, ) .is_ok() { @@ -379,6 +380,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { ), self.cast_ty, AllowTwoPhase::No, + None, ) .is_ok() { @@ -394,6 +396,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { fcx.tcx.mk_ref(reg, TypeAndMut { ty: self.expr_ty, mutbl }), self.cast_ty, AllowTwoPhase::No, + None, ) .is_ok() { @@ -409,6 +412,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { ), self.cast_ty, AllowTwoPhase::No, + None, ) .is_ok() { @@ -666,6 +670,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { self.expr_ty, fcx.tcx.mk_fn_ptr(f), AllowTwoPhase::No, + None, ); if let Err(TypeError::IntrinsicCast) = res { return Err(CastError::IllegalCast); @@ -829,7 +834,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { // Coerce to a raw pointer so that we generate AddressOf in MIR. let array_ptr_type = fcx.tcx.mk_ptr(m_expr); - fcx.try_coerce(self.expr, self.expr_ty, array_ptr_type, AllowTwoPhase::No) + fcx.try_coerce(self.expr, self.expr_ty, array_ptr_type, AllowTwoPhase::No, None) .unwrap_or_else(|_| { bug!( "could not cast from reference to array to pointer to array ({:?} to {:?})", @@ -861,7 +866,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { } fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'tcx>) -> Result<(), ty::error::TypeError<'_>> { - match fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No) { + match fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No, None) { Ok(_) => Ok(()), Err(err) => Err(err), } diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 1fd1253e5277d..54e4eb4768813 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -214,7 +214,7 @@ pub(super) fn check_fn<'a, 'tcx>( fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType); } else { fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType); - fcx.check_return_expr(&body.value); + fcx.check_return_expr(&body.value, false); } fcx.in_tail_expr = false; diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 6fe96e4cc27b2..e47c7e64ab552 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -941,11 +941,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr_ty: Ty<'tcx>, target: Ty<'tcx>, allow_two_phase: AllowTwoPhase, + cause: Option>, ) -> RelateResult<'tcx, Ty<'tcx>> { let source = self.resolve_vars_with_obligations(expr_ty); debug!("coercion::try({:?}: {:?} -> {:?})", expr, source, target); - let cause = self.cause(expr.span, ObligationCauseCode::ExprAssignable); + let cause = + cause.unwrap_or_else(|| self.cause(expr.span, ObligationCauseCode::ExprAssignable)); let coerce = Coerce::new(self, cause, allow_two_phase); let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?; @@ -1369,7 +1371,13 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { // Special-case the first expression we are coercing. // To be honest, I'm not entirely sure why we do this. // We don't allow two-phase borrows, see comment in try_find_coercion_lub for why - fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No) + fcx.try_coerce( + expression, + expression_ty, + self.expected_ty, + AllowTwoPhase::No, + Some(cause.clone()), + ) } else { match self.expressions { Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub( diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index a86db2d31b3d2..722b110ed6108 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -134,7 +134,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> (Ty<'tcx>, Option>) { let expected = self.resolve_vars_with_obligations(expected); - let e = match self.try_coerce(expr, checked_ty, expected, allow_two_phase) { + let e = match self.try_coerce(expr, checked_ty, expected, allow_two_phase, None) { Ok(ty) => return (ty, None), Err(e) => e, }; diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index f4e3c8e0d9f7f..0c011b85b0685 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -747,7 +747,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if self.ret_coercion_span.get().is_none() { self.ret_coercion_span.set(Some(e.span)); } - self.check_return_expr(e); + self.check_return_expr(e, true); } else { let mut coercion = self.ret_coercion.as_ref().unwrap().borrow_mut(); if self.ret_coercion_span.get().is_none() { @@ -776,16 +776,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.types.never } - pub(super) fn check_return_expr(&self, return_expr: &'tcx hir::Expr<'tcx>) { + /// `explicit_return` is `true` if we're checkng an explicit `return expr`, + /// and `false` if we're checking a trailing expression. + pub(super) fn check_return_expr( + &self, + return_expr: &'tcx hir::Expr<'tcx>, + explicit_return: bool, + ) { let ret_coercion = self.ret_coercion.as_ref().unwrap_or_else(|| { span_bug!(return_expr.span, "check_return_expr called outside fn body") }); let ret_ty = ret_coercion.borrow().expected_ty(); let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty); + let mut span = return_expr.span; + // Use the span of the trailing expression for our cause, + // not the span of the entire function + if !explicit_return { + if let ExprKind::Block(body, _) = return_expr.kind { + if let Some(last_expr) = body.expr { + span = last_expr.span; + } + } + } ret_coercion.borrow_mut().coerce( self, - &self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)), + &self.cause(span, ObligationCauseCode::ReturnValue(return_expr.hir_id)), return_expr, return_expr_ty, ); diff --git a/src/test/ui/issues/issue-55796.stderr b/src/test/ui/issues/issue-55796.stderr index ffe3bb737ad6a..952159ffc3bfe 100644 --- a/src/test/ui/issues/issue-55796.stderr +++ b/src/test/ui/issues/issue-55796.stderr @@ -15,7 +15,7 @@ note: ...so that the type `Map<>::EdgesIter, [closure@$DIR/iss LL | Box::new(self.out_edges(u).map(|e| e.target())) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: but, the lifetime must be valid for the static lifetime... -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/issue-55796.rs:18:9 | LL | Box::new(self.out_edges(u).map(|e| e.target())) @@ -40,7 +40,7 @@ note: ...so that the type `Map<>::EdgesIter, [closure@$DIR/iss LL | Box::new(self.in_edges(u).map(|e| e.target())) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: but, the lifetime must be valid for the static lifetime... -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/issue-55796.rs:23:9 | LL | Box::new(self.in_edges(u).map(|e| e.target())) diff --git a/src/test/ui/issues/issue-75777.stderr b/src/test/ui/issues/issue-75777.stderr index 16249a33c2fd0..25562f6347e67 100644 --- a/src/test/ui/issues/issue-75777.stderr +++ b/src/test/ui/issues/issue-75777.stderr @@ -17,7 +17,7 @@ LL | Box::new(move |_| fut) = note: expected `(Pin + Send>>,)` found `(Pin + Send + 'a)>>,)` = note: but, the lifetime must be valid for the static lifetime... -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/issue-75777.rs:13:5 | LL | Box::new(move |_| fut) diff --git a/src/test/ui/nll/issue-55394.stderr b/src/test/ui/nll/issue-55394.stderr index 36721f923f7da..dbc478e5b4c87 100644 --- a/src/test/ui/nll/issue-55394.stderr +++ b/src/test/ui/nll/issue-55394.stderr @@ -19,7 +19,7 @@ note: but, the lifetime must be valid for the lifetime `'_` as defined on the im | LL | impl Foo<'_> { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/issue-55394.rs:9:9 | LL | Foo { bar } diff --git a/src/test/ui/nll/type-alias-free-regions.stderr b/src/test/ui/nll/type-alias-free-regions.stderr index 6498ecfbe6f9b..dbb63b71af8a2 100644 --- a/src/test/ui/nll/type-alias-free-regions.stderr +++ b/src/test/ui/nll/type-alias-free-regions.stderr @@ -21,7 +21,7 @@ note: but, the lifetime must be valid for the lifetime `'a` as defined on the im | LL | impl<'a> FromBox<'a> for C<'a> { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/type-alias-free-regions.rs:17:9 | LL | C { f: b } @@ -52,7 +52,7 @@ note: but, the lifetime must be valid for the lifetime `'a` as defined on the im | LL | impl<'a> FromTuple<'a> for C<'a> { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/type-alias-free-regions.rs:27:9 | LL | C { f: Box::new(b.0) } diff --git a/src/test/ui/object-lifetime/object-lifetime-default-elision.stderr b/src/test/ui/object-lifetime/object-lifetime-default-elision.stderr index 79ded5fc875a2..ee1a461257228 100644 --- a/src/test/ui/object-lifetime/object-lifetime-default-elision.stderr +++ b/src/test/ui/object-lifetime/object-lifetime-default-elision.stderr @@ -19,7 +19,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu | LL | fn load3<'a,'b>(ss: &'a dyn SomeTrait) -> &'b dyn SomeTrait { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/object-lifetime-default-elision.rs:71:5 | LL | ss @@ -48,7 +48,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu | LL | fn load3<'a,'b>(ss: &'a dyn SomeTrait) -> &'b dyn SomeTrait { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/object-lifetime-default-elision.rs:71:5 | LL | ss diff --git a/src/test/ui/regions/region-object-lifetime-in-coercion.stderr b/src/test/ui/regions/region-object-lifetime-in-coercion.stderr index 1c8840f540e76..852ca0f21b166 100644 --- a/src/test/ui/regions/region-object-lifetime-in-coercion.stderr +++ b/src/test/ui/regions/region-object-lifetime-in-coercion.stderr @@ -69,7 +69,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu | LL | fn d<'a,'b>(v: &'a [u8]) -> Box { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/region-object-lifetime-in-coercion.rs:23:5 | LL | Box::new(v) diff --git a/src/test/ui/regions/regions-close-over-type-parameter-multiple.stderr b/src/test/ui/regions/regions-close-over-type-parameter-multiple.stderr index 0cce89215d364..bf29c76a0f0a8 100644 --- a/src/test/ui/regions/regions-close-over-type-parameter-multiple.stderr +++ b/src/test/ui/regions/regions-close-over-type-parameter-multiple.stderr @@ -19,7 +19,7 @@ note: but, the lifetime must be valid for the lifetime `'c` as defined on the fu | LL | fn make_object_bad<'a,'b,'c,A:SomeTrait+'a+'b>(v: A) -> Box { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/regions-close-over-type-parameter-multiple.rs:20:5 | LL | box v as Box diff --git a/src/test/ui/regions/regions-creating-enums4.stderr b/src/test/ui/regions/regions-creating-enums4.stderr index b24db1df18b0a..44bd88e01a267 100644 --- a/src/test/ui/regions/regions-creating-enums4.stderr +++ b/src/test/ui/regions/regions-creating-enums4.stderr @@ -21,7 +21,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu | LL | fn mk_add_bad2<'a,'b>(x: &'a Ast<'a>, y: &'a Ast<'a>, z: &Ast) -> Ast<'b> { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/regions-creating-enums4.rs:7:5 | LL | Ast::Add(x, y) diff --git a/src/test/ui/regions/regions-ret-borrowed-1.stderr b/src/test/ui/regions/regions-ret-borrowed-1.stderr index bba968cfde43c..b5b54bc3c8b73 100644 --- a/src/test/ui/regions/regions-ret-borrowed-1.stderr +++ b/src/test/ui/regions/regions-ret-borrowed-1.stderr @@ -9,7 +9,7 @@ note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on th | LL | with(|o| o) | ^^^^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/regions-ret-borrowed-1.rs:10:14 | LL | with(|o| o) diff --git a/src/test/ui/regions/regions-ret-borrowed.stderr b/src/test/ui/regions/regions-ret-borrowed.stderr index 4b93ca0ae6734..debae47d16d0b 100644 --- a/src/test/ui/regions/regions-ret-borrowed.stderr +++ b/src/test/ui/regions/regions-ret-borrowed.stderr @@ -9,7 +9,7 @@ note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on th | LL | with(|o| o) | ^^^^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/regions-ret-borrowed.rs:13:14 | LL | with(|o| o) diff --git a/src/test/ui/regions/regions-trait-object-subtyping.stderr b/src/test/ui/regions/regions-trait-object-subtyping.stderr index 7478b53bd3ccc..f16dfdd6e8c77 100644 --- a/src/test/ui/regions/regions-trait-object-subtyping.stderr +++ b/src/test/ui/regions/regions-trait-object-subtyping.stderr @@ -36,7 +36,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu | LL | fn foo3<'a,'b>(x: &'a mut dyn Dummy) -> &'b mut dyn Dummy { | ^^ -note: ...so that the expression is assignable +note: ...so that the types are compatible --> $DIR/regions-trait-object-subtyping.rs:15:5 | LL | x