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 14550690e63e..4ea7a8694c07 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 1fd1253e5277..54e4eb476881 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 6fe96e4cc27b..e47c7e64ab55 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 a86db2d31b3d..722b110ed610 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 f4e3c8e0d9f7..0c011b85b068 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 ffe3bb737ad6..952159ffc3bf 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 16249a33c2fd..25562f6347e6 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 36721f923f7d..dbc478e5b4c8 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 6498ecfbe6f9..dbb63b71af8a 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 79ded5fc875a..ee1a46125722 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 1c8840f540e7..852ca0f21b16 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 0cce89215d36..bf29c76a0f0a 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 b24db1df18b0..44bd88e01a26 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 bba968cfde43..b5b54bc3c8b7 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 4b93ca0ae673..debae47d16d0 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 7478b53bd3cc..f16dfdd6e8c7 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