Rollup merge of #100094 - lyming2007:issue-98982, r=estebank

Detect type mismatch due to loop that might never iterate

When loop as tail expression causes a miss match type E0308 error, recursively get the return statement and add diagnostic information on it.

Author: matthiaskrgr

compiler/rustc_typeck/src/check/coercion.rs

@@ -38,10 +38,12 @@
 use crate::astconv::AstConv;
 use crate::check::FnCtxt;
 use rustc_errors::{
-    struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
+    struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
 };
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
+use rustc_hir::intravisit::{self, Visitor};
+use rustc_hir::Expr;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::infer::{Coercion, InferOk, InferResult};
 use rustc_infer::traits::{Obligation, TraitEngine, TraitEngineExt};
@@ -87,6 +89,19 @@ impl<'a, 'tcx> Deref for Coerce<'a, 'tcx> {
 
 type CoerceResult<'tcx> = InferResult<'tcx, (Vec<Adjustment<'tcx>>, Ty<'tcx>)>;
 
+struct CollectRetsVisitor<'tcx> {
+    ret_exprs: Vec<&'tcx hir::Expr<'tcx>>,
+}
+
+impl<'tcx> Visitor<'tcx> for CollectRetsVisitor<'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+        if let hir::ExprKind::Ret(_) = expr.kind {
+            self.ret_exprs.push(expr);
+        }
+        intravisit::walk_expr(self, expr);
+    }
+}
+
 /// Coercing a mutable reference to an immutable works, while
 /// coercing `&T` to `&mut T` should be forbidden.
 fn coerce_mutbls<'tcx>(
@@ -1481,6 +1496,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
 
                 let mut err;
                 let mut unsized_return = false;
+                let mut visitor = CollectRetsVisitor { ret_exprs: vec![] };
                 match *cause.code() {
                     ObligationCauseCode::ReturnNoExpression => {
                         err = struct_span_err!(
@@ -1506,6 +1522,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                         if !fcx.tcx.features().unsized_locals {
                             unsized_return = self.is_return_ty_unsized(fcx, blk_id);
                         }
+                        if let Some(expression) = expression
+                            && let hir::ExprKind::Loop(loop_blk, ..) = expression.kind {
+                              intravisit::walk_block(& mut visitor, loop_blk);
+                        }
                     }
                     ObligationCauseCode::ReturnValue(id) => {
                         err = self.report_return_mismatched_types(
@@ -1551,12 +1571,39 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                     );
                 }
 
+                if visitor.ret_exprs.len() > 0 && let Some(expr) = expression {
+                    self.note_unreachable_loop_return(&mut err, &expr, &visitor.ret_exprs);
+                }
                 err.emit_unless(unsized_return);
 
                 self.final_ty = Some(fcx.tcx.ty_error());
             }
         }
     }
+    fn note_unreachable_loop_return<'a>(
+        &self,
+        err: &mut DiagnosticBuilder<'a, ErrorGuaranteed>,
+        expr: &hir::Expr<'tcx>,
+        ret_exprs: &Vec<&'tcx hir::Expr<'tcx>>,
+    ) {
+        let hir::ExprKind::Loop(_, _, _, loop_span) = expr.kind else { return;};
+        let mut span: MultiSpan = vec![loop_span].into();
+        span.push_span_label(loop_span, "this might have zero elements to iterate on".to_string());
+        for ret_expr in ret_exprs {
+            span.push_span_label(
+                ret_expr.span,
+                "if the loop doesn't execute, this value would never get returned".to_string(),
+            );
+        }
+        err.span_note(
+            span,
+            "the function expects a value to always be returned, but loops might run zero times",
+        );
+        err.help(
+            "return a value for the case when the loop has zero elements to iterate on, or \
+           consider changing the return type to account for that possibility",
+        );
+    }
 
     fn report_return_mismatched_types<'a>(
         &self,

src/test/ui/for-loop-while/break-while-condition.stderr

@@ -31,6 +31,14 @@ LL | |             }
    |
    = note:   expected type `!`
            found unit type `()`
+note: the function expects a value to always be returned, but loops might run zero times
+  --> $DIR/break-while-condition.rs:24:13
+   |
+LL |             while false {
+   |             ^^^^^^^^^^^ this might have zero elements to iterate on
+LL |                 return
+   |                 ------ if the loop doesn't execute, this value would never get returned
+   = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility
 
 error: aborting due to 3 previous errors
 

src/test/ui/typeck/issue-98982.rs

@@ -0,0 +1,9 @@
+fn foo() -> i32 {
+    for i in 0..0 {
+    //~^ ERROR: mismatched types [E0308]
+        return i;
+    }
+    //~| help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility
+}
+
+fn main() {}

src/test/ui/typeck/issue-98982.stderr

@@ -0,0 +1,24 @@
+error[E0308]: mismatched types
+  --> $DIR/issue-98982.rs:2:5
+   |
+LL |   fn foo() -> i32 {
+   |               --- expected `i32` because of return type
+LL | /     for i in 0..0 {
+LL | |
+LL | |         return i;
+LL | |     }
+   | |_____^ expected `i32`, found `()`
+   |
+note: the function expects a value to always be returned, but loops might run zero times
+  --> $DIR/issue-98982.rs:2:5
+   |
+LL |     for i in 0..0 {
+   |     ^^^^^^^^^^^^^ this might have zero elements to iterate on
+LL |
+LL |         return i;
+   |         -------- if the loop doesn't execute, this value would never get returned
+   = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.