Auto merge of #116071 - estebank:issue-115905, r=compiler-errors

Point at cause of expectation of `break` value when possible

When encountering a type error within the value of a `break` statement, climb the HIR tree to identify if the expectation comes from an assignment or a return type (if the loop is the tail expression of a `fn`).

Fix #115905.

Author: bors

Diff for: compiler/rustc_hir_typeck/src/_match.rs

@@ -2,6 +2,7 @@ use crate::coercion::{AsCoercionSite, CoerceMany};
 use crate::{Diverges, Expectation, FnCtxt, Needs};
 use rustc_errors::Diagnostic;
 use rustc_hir::{self as hir, ExprKind};
+use rustc_hir_pretty::ty_to_string;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::traits::Obligation;
 use rustc_middle::ty::{self, Ty};
@@ -252,7 +253,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     {
         // If this `if` expr is the parent's function return expr,
         // the cause of the type coercion is the return type, point at it. (#25228)
-        let ret_reason = self.maybe_get_coercion_reason(then_expr.hir_id, span);
+        let hir_id = self.tcx.hir().parent_id(self.tcx.hir().parent_id(then_expr.hir_id));
+        let ret_reason = self.maybe_get_coercion_reason(hir_id, span);
         let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse);
         let mut error = false;
         coercion.coerce_forced_unit(
@@ -275,11 +277,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         error
     }
 
-    fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, sp: Span) -> Option<(Span, String)> {
-        let node = {
-            let rslt = self.tcx.hir().parent_id(self.tcx.hir().parent_id(hir_id));
-            self.tcx.hir().get(rslt)
-        };
+    pub fn maybe_get_coercion_reason(
+        &self,
+        hir_id: hir::HirId,
+        sp: Span,
+    ) -> Option<(Span, String)> {
+        let node = self.tcx.hir().get(hir_id);
         if let hir::Node::Block(block) = node {
             // check that the body's parent is an fn
             let parent = self.tcx.hir().get_parent(self.tcx.hir().parent_id(block.hir_id));
@@ -289,9 +292,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 // check that the `if` expr without `else` is the fn body's expr
                 if expr.span == sp {
                     return self.get_fn_decl(hir_id).and_then(|(_, fn_decl, _)| {
-                        let span = fn_decl.output.span();
-                        let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok()?;
-                        Some((span, format!("expected `{snippet}` because of this return type")))
+                        let (ty, span) = match fn_decl.output {
+                            hir::FnRetTy::DefaultReturn(span) => ("()".to_string(), span),
+                            hir::FnRetTy::Return(ty) => (ty_to_string(ty), ty.span),
+                        };
+                        Some((span, format!("expected `{ty}` because of this return type")))
                     });
                 }
             }

Diff for: compiler/rustc_hir_typeck/src/demand.rs

@@ -83,6 +83,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
 
         self.annotate_expected_due_to_let_ty(err, expr, error);
+        self.annotate_loop_expected_due_to_inference(err, expr, error);
 
         // FIXME(#73154): For now, we do leak check when coercing function
         // pointers in typeck, instead of only during borrowck. This can lead
@@ -527,6 +528,67 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         false
     }
 
+    // When encountering a type error on the value of a `break`, try to point at the reason for the
+    // expected type.
+    fn annotate_loop_expected_due_to_inference(
+        &self,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'_>,
+        error: Option<TypeError<'tcx>>,
+    ) {
+        let Some(TypeError::Sorts(ExpectedFound { expected, .. })) = error else {
+            return;
+        };
+        let mut parent_id = self.tcx.hir().parent_id(expr.hir_id);
+        loop {
+            // Climb the HIR tree to see if the current `Expr` is part of a `break;` statement.
+            let Some(hir::Node::Expr(parent)) = self.tcx.hir().find(parent_id) else {
+                break;
+            };
+            parent_id = self.tcx.hir().parent_id(parent.hir_id);
+            let hir::ExprKind::Break(destination, _) = parent.kind else {
+                continue;
+            };
+            let mut parent_id = parent.hir_id;
+            loop {
+                // Climb the HIR tree to find the (desugared) `loop` this `break` corresponds to.
+                let parent = match self.tcx.hir().find(parent_id) {
+                    Some(hir::Node::Expr(&ref parent)) => {
+                        parent_id = self.tcx.hir().parent_id(parent.hir_id);
+                        parent
+                    }
+                    Some(hir::Node::Stmt(hir::Stmt {
+                        hir_id,
+                        kind: hir::StmtKind::Semi(&ref parent) | hir::StmtKind::Expr(&ref parent),
+                        ..
+                    })) => {
+                        parent_id = self.tcx.hir().parent_id(*hir_id);
+                        parent
+                    }
+                    Some(hir::Node::Block(hir::Block { .. })) => {
+                        parent_id = self.tcx.hir().parent_id(parent_id);
+                        parent
+                    }
+                    _ => break,
+                };
+                if let hir::ExprKind::Loop(_, label, _, span) = parent.kind
+                    && destination.label == label
+                {
+                    if let Some((reason_span, message)) =
+                        self.maybe_get_coercion_reason(parent_id, parent.span)
+                    {
+                        err.span_label(reason_span, message);
+                        err.span_label(
+                            span,
+                            format!("this loop is expected to be of type `{expected}`"),
+                        );
+                    }
+                    break;
+                }
+            }
+        }
+    }
+
     fn annotate_expected_due_to_let_ty(
         &self,
         err: &mut Diagnostic,

Diff for: tests/ui/loops/loop-break-value.rs

@@ -95,4 +95,7 @@ fn main() {
         break LOOP;
         //~^ ERROR cannot find value `LOOP` in this scope
     }
+    loop { // point at the return type
+        break 2; //~ ERROR mismatched types
+    }
 }

Diff for: tests/ui/loops/loop-break-value.stderr

@@ -148,12 +148,21 @@ LL |             break 123;
 error[E0308]: mismatched types
   --> $DIR/loop-break-value.rs:16:15
    |
+LL |     let _: i32 = loop {
+   |         -        ---- this loop is expected to be of type `i32`
+   |         |
+   |         expected because of this assignment
 LL |         break "asdf";
    |               ^^^^^^ expected `i32`, found `&str`
 
 error[E0308]: mismatched types
   --> $DIR/loop-break-value.rs:21:31
    |
+LL |     let _: i32 = 'outer_loop: loop {
+   |         -                     ---- this loop is expected to be of type `i32`
+   |         |
+   |         expected because of this assignment
+LL |         loop {
 LL |             break 'outer_loop "nope";
    |                               ^^^^^^ expected `i32`, found `&str`
 
@@ -187,7 +196,18 @@ LL |         break;
    |         expected integer, found `()`
    |         help: give it a value of the expected type: `break value`
 
-error: aborting due to 17 previous errors; 1 warning emitted
+error[E0308]: mismatched types
+  --> $DIR/loop-break-value.rs:99:15
+   |
+LL | fn main() {
+   |           - expected `()` because of this return type
+...
+LL |     loop { // point at the return type
+   |     ---- this loop is expected to be of type `()`
+LL |         break 2;
+   |               ^ expected `()`, found integer
+
+error: aborting due to 18 previous errors; 1 warning emitted
 
 Some errors have detailed explanations: E0308, E0425, E0571.
 For more information about an error, try `rustc --explain E0308`.