[infinite_loop]: give a result type to LoopVisitor

Author: J-ZhengLi

clippy_lints/src/loops/infinite_loop.rs

@@ -4,7 +4,6 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{fn_def_id, is_from_proc_macro, is_lint_allowed};
 use hir::intravisit::{walk_expr, Visitor};
 use hir::{ClosureKind, Expr, ExprKind, FnRetTy, FnSig, ItemKind, Node, Ty, TyKind};
-use rustc_ast::Label;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LintContext};
@@ -17,7 +16,7 @@ pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &Expr<'tcx>,
     loop_block: &'tcx hir::Block<'_>,
-    label: Option<Label>,
+    hir_id: hir::HirId,
 ) {
     if is_lint_allowed(cx, INFINITE_LOOP, expr.hir_id) {
         return;
@@ -33,15 +32,11 @@ pub(super) fn check<'tcx>(
 
     let mut loop_visitor = LoopVisitor {
         cx,
-        label,
-        is_finite: false,
-        loop_depth: 0,
+        hir_id,
+        nested_loop_count: 0,
     };
-    loop_visitor.visit_block(loop_block);
-
-    let is_finite_loop = loop_visitor.is_finite;
-
-    if !is_finite_loop {
+    let is_finite = loop_visitor.visit_block(loop_block).is_break();
+    if !is_finite {
         span_lint_and_then(cx, INFINITE_LOOP, expr.span, "infinite loop detected", |diag| {
             if let Some(span) = parent_fn_ret.sugg_span() {
                 // Check if the type span is after a `->` or not.
@@ -106,40 +101,43 @@ fn get_parent_fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option
 
 struct LoopVisitor<'hir, 'tcx> {
     cx: &'hir LateContext<'tcx>,
-    label: Option<Label>,
-    loop_depth: usize,
-    is_finite: bool,
+    // `HirId` for the entry (outer most) loop.
+    hir_id: hir::HirId,
+    nested_loop_count: usize,
 }
 
 impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
-    fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
+    type Result = ControlFlow<()>;
+
+    fn visit_expr(&mut self, ex: &'hir Expr<'_>) -> Self::Result {
         match &ex.kind {
-            ExprKind::Break(hir::Destination { label, .. }, ..) => {
-                // Assuming breaks the loop when `loop_depth` is 0,
-                // as it could only means this `break` breaks current loop or any of its upper loop.
-                // Or, the depth is not zero but the label is matched.
-                if self.loop_depth == 0 || (label.is_some() && *label == self.label) {
-                    self.is_finite = true;
+            ExprKind::Break(hir::Destination { target_id, .. }, ..) => {
+                // When there are no nested loops and we've encountered a `break`, meaning it's either breaking the
+                // current loop or any of the parent loop, assuming that this loop is not infinite.
+                if self.nested_loop_count == 0
+                    || matches!(target_id, Ok(target_loop_hid) if *target_loop_hid == self.hir_id)
+                {
+                    return ControlFlow::Break(());
                 }
             },
-            ExprKind::Ret(..) => self.is_finite = true,
+            ExprKind::Ret(..) => return ControlFlow::Break(()),
             ExprKind::Loop(..) => {
-                self.loop_depth += 1;
-                walk_expr(self, ex);
-                self.loop_depth = self.loop_depth.saturating_sub(1);
+                self.nested_loop_count += 1;
+                let cf = walk_expr(self, ex);
+                self.nested_loop_count = self.nested_loop_count.saturating_sub(1);
+                return cf;
             },
             _ => {
                 // Calls to a function that never return
                 if let Some(did) = fn_def_id(self.cx, ex) {
                     let fn_ret_ty = self.cx.tcx.fn_sig(did).skip_binder().output().skip_binder();
                     if fn_ret_ty.is_never() {
-                        self.is_finite = true;
-                        return;
+                        return ControlFlow::Break(());
                     }
                 }
-                walk_expr(self, ex);
             },
         }
+        walk_expr(self, ex)
     }
 }
 

clippy_lints/src/loops/mod.rs

@@ -786,11 +786,11 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
         // check for `loop { if let {} else break }` that could be `while let`
         // (also matches an explicit "match" instead of "if let")
         // (even if the "match" or "if let" is used for declaration)
-        if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
+        if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind {
             // also check for empty `loop {}` statements, skipping those in #[panic_handler]
             empty_loop::check(cx, expr, block);
             while_let_loop::check(cx, expr, block);
-            infinite_loop::check(cx, expr, block, label);
+            infinite_loop::check(cx, expr, block, expr.hir_id);
         }
 
         while_let_on_iterator::check(cx, expr);