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 14550690e63..4ea7a8694c0 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 1fd1253e527..54e4eb47688 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 6fe96e4cc27..e47c7e64ab5 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 a86db2d31b3..722b110ed61 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 f4e3c8e0d9f..0c011b85b06 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 ffe3bb737ad..952159ffc3b 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 16249a33c2f..25562f6347e 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 36721f923f7..dbc478e5b4c 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 6498ecfbe6f..dbb63b71af8 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 79ded5fc875..ee1a4612572 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 1c8840f540e..852ca0f21b1 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 0cce89215d3..bf29c76a0f0 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 b24db1df18b..44bd88e01a2 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 bba968cfde4..b5b54bc3c8b 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 4b93ca0ae67..debae47d16d 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 7478b53bd3c..f16dfdd6e8c 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