Auto merge of rust-lang#89028 - Aaron1011:coercion-cause, r=nagisa

bors · bors · commit 3bca7230ff1e · 2021-09-19T04:34:16.000Z
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.
diff --git 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
@@ -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
@@ -935,11 +935,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr_ty: Ty<'tcx>,
         target: Ty<'tcx>,
         allow_two_phase: AllowTwoPhase,
+        cause: Option<ObligationCause<'tcx>>,
     ) -> 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))?;
 
@@ -1363,7 +1365,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
@@ -134,7 +134,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ) -> (Ty<'tcx>, Option<DiagnosticBuilder<'tcx>>) {
         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
@@ -765,7 +765,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() {
@@ -794,16 +794,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
@@ -15,7 +15,7 @@ note: ...so that the type `Map<<Self as Graph<'a>>::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<<Self as Graph<'a>>::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
@@ -17,7 +17,7 @@ LL |     Box::new(move |_| fut)
    = note: expected `(Pin<Box<dyn Future<Output = A> + Send>>,)`
               found `(Pin<Box<(dyn Future<Output = A> + 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
@@ -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
@@ -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
@@ -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
@@ -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<dyn Foo+'b> {
    |         ^^
-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
@@ -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<dyn SomeTrait + 'c> {
    |                          ^^
-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<dyn SomeTrait + 'a>
diff --git 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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -362,6 +362,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`362`	`362`	`),`
`363`	`363`	`self.cast_ty,`
`364`	`364`	`AllowTwoPhase::No,`
	`365`	`+ None,`
`365`	`366`	`)`
`366`	`367`	`.is_ok()`
`367`	`368`	`{`
`@@ -379,6 +380,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`379`	`380`	`),`
`380`	`381`	`self.cast_ty,`
`381`	`382`	`AllowTwoPhase::No,`
	`383`	`+ None,`
`382`	`384`	`)`
`383`	`385`	`.is_ok()`
`384`	`386`	`{`
`@@ -394,6 +396,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`394`	`396`	`fcx.tcx.mk_ref(reg, TypeAndMut { ty: self.expr_ty, mutbl }),`
`395`	`397`	`self.cast_ty,`
`396`	`398`	`AllowTwoPhase::No,`
	`399`	`+ None,`
`397`	`400`	`)`
`398`	`401`	`.is_ok()`
`399`	`402`	`{`
`@@ -409,6 +412,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`409`	`412`	`),`
`410`	`413`	`self.cast_ty,`
`411`	`414`	`AllowTwoPhase::No,`
	`415`	`+ None,`
`412`	`416`	`)`
`413`	`417`	`.is_ok()`
`414`	`418`	`{`
`@@ -666,6 +670,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`666`	`670`	`self.expr_ty,`
`667`	`671`	`fcx.tcx.mk_fn_ptr(f),`
`668`	`672`	`AllowTwoPhase::No,`
	`673`	`+ None,`
`669`	`674`	`);`
`670`	`675`	`if let Err(TypeError::IntrinsicCast) = res {`
`671`	`676`	`return Err(CastError::IllegalCast);`
`@@ -829,7 +834,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`829`	`834`
`830`	`835`	`// Coerce to a raw pointer so that we generate AddressOf in MIR.`
`831`	`836`	`let array_ptr_type = fcx.tcx.mk_ptr(m_expr);`
`832`		`- fcx.try_coerce(self.expr, self.expr_ty, array_ptr_type, AllowTwoPhase::No)`
	`837`	`+ fcx.try_coerce(self.expr, self.expr_ty, array_ptr_type, AllowTwoPhase::No, None)`
`833`	`838`	`.unwrap_or_else(\|_\| {`
`834`	`839`	`bug!(`
`835`	`840`	`"could not cast from reference to array to pointer to array ({:?} to {:?})",`
`@@ -861,7 +866,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {`
`861`	`866`	`}`
`862`	`867`
`863`	`868`	`fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'tcx>) -> Result<(), ty::error::TypeError<'_>> {`
`864`		`- match fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No) {`
	`869`	`+ match fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No, None) {`
`865`	`870`	`Ok(_) => Ok(()),`
`866`	`871`	`Err(err) => Err(err),`
`867`	`872`	`}`
Original file line number	Diff line number	Diff line change
`@@ -214,7 +214,7 @@ pub(super) fn check_fn<'a, 'tcx>(`
`214`	`214`	`fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);`
`215`	`215`	`} else {`
`216`	`216`	`fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);`
`217`		`- fcx.check_return_expr(&body.value);`
	`217`	`+ fcx.check_return_expr(&body.value, false);`
`218`	`218`	`}`
`219`	`219`	`fcx.in_tail_expr = false;`
`220`	`220`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ LL \| Box::new(move \|_\| fut)`
`17`	`17`	= note: expected `(Pin<Box<dyn Future<Output = A> + Send>>,)`
`18`	`18`	found `(Pin<Box<(dyn Future<Output = A> + Send + 'a)>>,)`
`19`	`19`	`= note: but, the lifetime must be valid for the static lifetime...`
`20`		`-note: ...so that the expression is assignable`
	`20`	`+note: ...so that the types are compatible`
`21`	`21`	`--> $DIR/issue-75777.rs:13:5`
`22`	`22`	`\|`
`23`	`23`	`LL \| Box::new(move \|_\| fut)`
Original file line number	Diff line number	Diff line change
@@ -19,7 +19,7 @@ note: but, the lifetime must be valid for the lifetime `'_` as defined on the im
`19`	`19`	`\|`
`20`	`20`	`LL \| impl Foo<'_> {`
`21`	`21`	`\| ^^`
`22`		`-note: ...so that the expression is assignable`
	`22`	`+note: ...so that the types are compatible`
`23`	`23`	`--> $DIR/issue-55394.rs:9:9`
`24`	`24`	`\|`
`25`	`25`	`LL \| Foo { bar }`
Original file line number	Diff line number	Diff line change
@@ -21,7 +21,7 @@ note: but, the lifetime must be valid for the lifetime `'a` as defined on the im
`21`	`21`	`\|`
`22`	`22`	`LL \| impl<'a> FromBox<'a> for C<'a> {`
`23`	`23`	`\| ^^`
`24`		`-note: ...so that the expression is assignable`
	`24`	`+note: ...so that the types are compatible`
`25`	`25`	`--> $DIR/type-alias-free-regions.rs:17:9`
`26`	`26`	`\|`
`27`	`27`	`LL \| C { f: b }`
@@ -52,7 +52,7 @@ note: but, the lifetime must be valid for the lifetime `'a` as defined on the im
`52`	`52`	`\|`
`53`	`53`	`LL \| impl<'a> FromTuple<'a> for C<'a> {`
`54`	`54`	`\| ^^`
`55`		`-note: ...so that the expression is assignable`
	`55`	`+note: ...so that the types are compatible`
`56`	`56`	`--> $DIR/type-alias-free-regions.rs:27:9`
`57`	`57`	`\|`
`58`	`58`	`LL \| C { f: Box::new(b.0) }`
Original file line number	Diff line number	Diff line change
@@ -69,7 +69,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu
`69`	`69`	`\|`
`70`	`70`	`LL \| fn d<'a,'b>(v: &'a [u8]) -> Box<dyn Foo+'b> {`
`71`	`71`	`\| ^^`
`72`		`-note: ...so that the expression is assignable`
	`72`	`+note: ...so that the types are compatible`
`73`	`73`	`--> $DIR/region-object-lifetime-in-coercion.rs:23:5`
`74`	`74`	`\|`
`75`	`75`	`LL \| Box::new(v)`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on th`
`9`	`9`	`\|`
`10`	`10`	`LL \| with(\|o\| o)`
`11`	`11`	`\| ^^^^^`
`12`		`-note: ...so that the expression is assignable`
	`12`	`+note: ...so that the types are compatible`
`13`	`13`	`--> $DIR/regions-ret-borrowed-1.rs:10:14`
`14`	`14`	`\|`
`15`	`15`	`LL \| with(\|o\| o)`
Original file line number	Diff line number	Diff line change
@@ -36,7 +36,7 @@ note: but, the lifetime must be valid for the lifetime `'b` as defined on the fu
`36`	`36`	`\|`
`37`	`37`	`LL \| fn foo3<'a,'b>(x: &'a mut dyn Dummy) -> &'b mut dyn Dummy {`
`38`	`38`	`\| ^^`
`39`		`-note: ...so that the expression is assignable`
	`39`	`+note: ...so that the types are compatible`
`40`	`40`	`--> $DIR/regions-trait-object-subtyping.rs:15:5`
`41`	`41`	`\|`
`42`	`42`	`LL \| x`