Auto merge of #12830 - blyxyas:more-controlflow, r=y21,xFredNet

Use ControlFlow in more places

Now, instead of manually using variables in visitors to signify that a visit is "done" and that the visitor should stop traversing. We use the trait type "Result" to signify this (in relevant places).

I'll schedule a perf run, I don't think it will be much of a difference, but every bit of performance is welcomed :)

changelog: Improve performance, less memory use in visitors

Fixes #12829
r? `@y21`

clippy_lints/src/lib.rs

@@ -1,6 +1,7 @@
 #![feature(array_windows)]
 #![feature(binary_heap_into_iter_sorted)]
 #![feature(box_patterns)]
+#![feature(control_flow_enum)]
 #![feature(f128)]
 #![feature(f16)]
 #![feature(if_let_guard)]

clippy_lints/src/lifetimes.rs

@@ -22,6 +22,7 @@ use rustc_session::declare_lint_pass;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::symbol::{kw, Ident, Symbol};
 use rustc_span::Span;
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -380,11 +381,8 @@ fn could_use_elision<'tcx>(
             return None;
         }
 
-        let mut checker = BodyLifetimeChecker {
+        let mut checker = BodyLifetimeChecker;
-            lifetimes_used_in_body: false,
+        if checker.visit_expr(body.value).is_break() {
-        };
-        checker.visit_expr(body.value);
-        if checker.lifetimes_used_in_body {
             return None;
         }
     }
@@ -694,15 +692,15 @@ fn report_extra_impl_lifetimes<'tcx>(cx: &LateContext<'tcx>, impl_: &'tcx Impl<'
     }
 }
 
-struct BodyLifetimeChecker {
+struct BodyLifetimeChecker;
-    lifetimes_used_in_body: bool,
-}
 
 impl<'tcx> Visitor<'tcx> for BodyLifetimeChecker {
+    type Result = ControlFlow<()>;
     // for lifetimes as parameters of generics
-    fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) {
+    fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) -> ControlFlow<()> {
         if !lifetime.is_anonymous() && lifetime.ident.name != kw::StaticLifetime {
-            self.lifetimes_used_in_body = true;
+            return ControlFlow::Break(());
         }
+        ControlFlow::Continue(())
     }
 }

clippy_lints/src/loops/mut_range_bound.rs

@@ -8,6 +8,7 @@ use rustc_lint::LateContext;
 use rustc_middle::mir::FakeReadCause;
 use rustc_middle::ty;
 use rustc_span::Span;
+use std::ops::ControlFlow;
 
 pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
     if let Some(higher::Range {
@@ -114,7 +115,6 @@ impl MutatePairDelegate<'_, '_> {
 struct BreakAfterExprVisitor {
     hir_id: HirId,
     past_expr: bool,
-    past_candidate: bool,
     break_after_expr: bool,
 }
 
@@ -123,7 +123,6 @@ impl BreakAfterExprVisitor {
         let mut visitor = BreakAfterExprVisitor {
             hir_id,
             past_expr: false,
-            past_candidate: false,
             break_after_expr: false,
         };
 
@@ -135,21 +134,19 @@ impl BreakAfterExprVisitor {
 }
 
 impl<'tcx> Visitor<'tcx> for BreakAfterExprVisitor {
-    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+    type Result = ControlFlow<()>;
-        if self.past_candidate {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
-            return;
-        }
-
         if expr.hir_id == self.hir_id {
             self.past_expr = true;
+            ControlFlow::Continue(())
         } else if self.past_expr {
             if matches!(&expr.kind, ExprKind::Break(..)) {
                 self.break_after_expr = true;
             }
 
-            self.past_candidate = true;
+            ControlFlow::Break(())
         } else {
-            intravisit::walk_expr(self, expr);
+            intravisit::walk_expr(self, expr)
         }
     }
 }

clippy_lints/src/loops/while_immutable_condition.rs

@@ -7,6 +7,7 @@ use rustc_hir::def_id::DefIdMap;
 use rustc_hir::intravisit::{walk_expr, Visitor};
 use rustc_hir::{Expr, ExprKind, HirIdSet, QPath};
 use rustc_lint::LateContext;
+use std::ops::ControlFlow;
 
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
     if constant(cx, cx.typeck_results(), cond).is_some() {
@@ -35,11 +36,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
         };
     let mutable_static_in_cond = var_visitor.def_ids.items().any(|(_, v)| *v);
 
-    let mut has_break_or_return_visitor = HasBreakOrReturnVisitor {
+    let mut has_break_or_return_visitor = HasBreakOrReturnVisitor;
-        has_break_or_return: false,
+    let has_break_or_return = has_break_or_return_visitor.visit_expr(expr).is_break();
-    };
-    has_break_or_return_visitor.visit_expr(expr);
-    let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
 
     if no_cond_variable_mutated && !mutable_static_in_cond {
         span_lint_and_then(
@@ -59,25 +57,19 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
     }
 }
 
-struct HasBreakOrReturnVisitor {
+struct HasBreakOrReturnVisitor;
-    has_break_or_return: bool,
-}
 
 impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
-    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+    type Result = ControlFlow<()>;
-        if self.has_break_or_return {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<()> {
-            return;
-        }
-
         match expr.kind {
             ExprKind::Ret(_) | ExprKind::Break(_, _) => {
-                self.has_break_or_return = true;
+                return ControlFlow::Break(());
-                return;
             },
             _ => {},
         }
 
-        walk_expr(self, expr);
+        walk_expr(self, expr)
     }
 }
 

clippy_lints/src/methods/option_map_unwrap_or.rs

@@ -10,6 +10,7 @@ use rustc_hir::{ExprKind, HirId, Node, PatKind, Path, QPath};
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
 use rustc_span::{sym, Span};
+use std::ops::ControlFlow;
 
 use super::MAP_UNWRAP_OR;
 
@@ -54,15 +55,14 @@ pub(super) fn check<'tcx>(
             let mut reference_visitor = ReferenceVisitor {
                 cx,
                 identifiers: unwrap_visitor.identifiers,
-                found_reference: false,
                 unwrap_or_span: unwrap_arg.span,
             };
 
             let map = cx.tcx.hir();
             let body = map.body_owned_by(map.enclosing_body_owner(expr.hir_id));
-            reference_visitor.visit_body(body);
 
-            if reference_visitor.found_reference {
+            // Visit the body, and return if we've found a reference
+            if reference_visitor.visit_body(body).is_break() {
                 return;
             }
         }
@@ -151,17 +151,16 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrapVisitor<'a, 'tcx> {
 struct ReferenceVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     identifiers: FxHashSet<HirId>,
-    found_reference: bool,
     unwrap_or_span: Span,
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for ReferenceVisitor<'a, 'tcx> {
     type NestedFilter = nested_filter::All;
-    fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) {
+    type Result = ControlFlow<()>;
+    fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> {
         // If we haven't found a reference yet, check if this references
         // one of the locals that was moved in the `unwrap_or` argument.
         // We are only interested in exprs that appear before the `unwrap_or` call.
-        if !self.found_reference {
         if expr.span < self.unwrap_or_span
             && let ExprKind::Path(ref path) = expr.kind
             && let QPath::Resolved(_, path) = path
@@ -170,10 +169,9 @@ impl<'a, 'tcx> Visitor<'tcx> for ReferenceVisitor<'a, 'tcx> {
             && let PatKind::Binding(_, local_id, ..) = pat.kind
             && self.identifiers.contains(&local_id)
         {
-                self.found_reference = true;
+            return ControlFlow::Break(());
-            }
-            rustc_hir::intravisit::walk_expr(self, expr);
         }
+        rustc_hir::intravisit::walk_expr(self, expr)
     }
 
     fn nested_visit_map(&mut self) -> Self::Map {

clippy_lints/src/redundant_closure_call.rs

@@ -14,6 +14,7 @@ use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
 use rustc_span::ExpnKind;
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -42,24 +43,15 @@ declare_clippy_lint! {
 declare_lint_pass!(RedundantClosureCall => [REDUNDANT_CLOSURE_CALL]);
 
 // Used to find `return` statements or equivalents e.g., `?`
-struct ReturnVisitor {
+struct ReturnVisitor;
-    found_return: bool,
-}
-
-impl ReturnVisitor {
-    #[must_use]
-    fn new() -> Self {
-        Self { found_return: false }
-    }
-}
 
 impl<'tcx> Visitor<'tcx> for ReturnVisitor {
-    fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) {
+    type Result = ControlFlow<()>;
+    fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> ControlFlow<()> {
         if let ExprKind::Ret(_) | ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) = ex.kind {
-            self.found_return = true;
+            return ControlFlow::Break(());
-        } else {
-            hir_visit::walk_expr(self, ex);
         }
+        hir_visit::walk_expr(self, ex)
     }
 }
 
@@ -101,9 +93,8 @@ fn find_innermost_closure<'tcx>(
     while let ExprKind::Closure(closure) = expr.kind
         && let body = cx.tcx.hir().body(closure.body)
         && {
-            let mut visitor = ReturnVisitor::new();
+            let mut visitor = ReturnVisitor;
-            visitor.visit_expr(body.value);
+            !visitor.visit_expr(body.value).is_break()
-            !visitor.found_return
         }
         && steps > 0
     {

clippy_lints/src/unconditional_recursion.rs

@@ -16,6 +16,7 @@ use rustc_session::impl_lint_pass;
 use rustc_span::symbol::{kw, Ident};
 use rustc_span::{sym, Span};
 use rustc_trait_selection::error_reporting::traits::suggestions::ReturnsVisitor;
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -276,7 +277,6 @@ struct CheckCalls<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     map: Map<'tcx>,
     implemented_ty_id: DefId,
-    found_default_call: bool,
     method_span: Span,
 }
 
@@ -285,16 +285,14 @@ where
     'tcx: 'a,
 {
     type NestedFilter = nested_filter::OnlyBodies;
+    type Result = ControlFlow<()>;
 
     fn nested_visit_map(&mut self) -> Self::Map {
         self.map
     }
 
-    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
-        if self.found_default_call {
+        walk_expr(self, expr)?;
-            return;
-        }
-        walk_expr(self, expr);
 
         if let ExprKind::Call(f, _) = expr.kind
             && let ExprKind::Path(qpath) = f.kind
@@ -303,9 +301,10 @@ where
             && let Some(trait_def_id) = self.cx.tcx.trait_of_item(method_def_id)
             && self.cx.tcx.is_diagnostic_item(sym::Default, trait_def_id)
         {
-            self.found_default_call = true;
             span_error(self.cx, self.method_span, expr);
+            return ControlFlow::Break(());
         }
+        ControlFlow::Continue(())
     }
 }
 
@@ -383,7 +382,6 @@ impl UnconditionalRecursion {
                 cx,
                 map: cx.tcx.hir(),
                 implemented_ty_id,
-                found_default_call: false,
                 method_span,
             };
             walk_body(&mut c, body);

clippy_lints/src/unused_peekable.rs

@@ -8,6 +8,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter::OnlyBodies;
 use rustc_session::declare_lint_pass;
 use rustc_span::sym;
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -70,15 +71,16 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable {
                     return;
                 }
 
-                for stmt in &block.stmts[idx..] {
+                let mut found_peek_call = block.stmts[idx..].iter().any(|stmt| vis.visit_stmt(stmt).is_break());
-                    vis.visit_stmt(stmt);
+
+                if !found_peek_call
+                    && let Some(expr) = block.expr
+                    && vis.visit_expr(expr).is_break()
+                {
+                    found_peek_call = true;
                 }
 
-                if let Some(expr) = block.expr {
+                if !found_peek_call {
-                    vis.visit_expr(expr);
-                }
-
-                if !vis.found_peek_call {
                     span_lint_hir_and_then(
                         cx,
                         UNUSED_PEEKABLE,
@@ -98,31 +100,23 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable {
 struct PeekableVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     expected_hir_id: HirId,
-    found_peek_call: bool,
 }
 
 impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
     fn new(cx: &'a LateContext<'tcx>, expected_hir_id: HirId) -> Self {
-        Self {
+        Self { cx, expected_hir_id }
-            cx,
-            expected_hir_id,
-            found_peek_call: false,
-        }
     }
 }
 
 impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> {
     type NestedFilter = OnlyBodies;
+    type Result = ControlFlow<()>;
 
     fn nested_visit_map(&mut self) -> Self::Map {
         self.cx.tcx.hir()
     }
 
-    fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
+    fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> ControlFlow<()> {
-        if self.found_peek_call {
-            return;
-        }
-
         if path_to_local_id(ex, self.expected_hir_id) {
             for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) {
                 match node {
@@ -137,14 +131,14 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> {
                                     && func_did == into_iter_did
                                 {
                                     // Probably a for loop desugar, stop searching
-                                    return;
+                                    return ControlFlow::Continue(());
                                 }
 
                                 if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) {
-                                    self.found_peek_call = true;
+                                    return ControlFlow::Break(());
                                 }
 
-                                return;
+                                return ControlFlow::Continue(());
                             },
                             // Catch anything taking a Peekable mutably
                             ExprKind::MethodCall(
@@ -162,16 +156,14 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> {
                                 if matches!(method_name, "peek" | "peek_mut" | "next_if" | "next_if_eq")
                                     && arg_is_mut_peekable(self.cx, self_arg)
                                 {
-                                    self.found_peek_call = true;
+                                    return ControlFlow::Break(());
-                                    return;
                                 }
 
                                 // foo.some_method() excluding Iterator methods
                                 if remaining_args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg))
                                     && !is_trait_method(self.cx, expr, sym::Iterator)
                                 {
-                                    self.found_peek_call = true;
+                                    return ControlFlow::Break(());
-                                    return;
                                 }
 
                                 // foo.by_ref(), keep checking for `peek`
@@ -179,41 +171,42 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> {
                                     continue;
                                 }
 
-                                return;
+                                return ControlFlow::Continue(());
                             },
                             ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => {
                             },
-                            ExprKind::AddrOf(_, Mutability::Not, _) => return,
+                            ExprKind::AddrOf(_, Mutability::Not, _) => return ControlFlow::Continue(()),
                             _ => {
-                                self.found_peek_call = true;
+                                return ControlFlow::Break(());
-                                return;
                             },
                         }
                     },
                     Node::LetStmt(LetStmt { init: Some(init), .. }) => {
                         if arg_is_mut_peekable(self.cx, init) {
-                            self.found_peek_call = true;
+                            return ControlFlow::Break(());
                         }
 
-                        return;
+                        return ControlFlow::Continue(());
                     },
                     Node::Stmt(stmt) => {
                         match stmt.kind {
-                            StmtKind::Let(_) | StmtKind::Item(_) => self.found_peek_call = true,
+                            StmtKind::Let(_) | StmtKind::Item(_) => {
+                                return ControlFlow::Break(());
+                            },
                             StmtKind::Expr(_) | StmtKind::Semi(_) => {},
                         }
 
-                        return;
+                        return ControlFlow::Continue(());
                     },
                     Node::Block(_) | Node::ExprField(_) => {},
                     _ => {
-                        return;
+                        return ControlFlow::Continue(());
                     },
                 }
             }
         }
 
-        walk_expr(self, ex);
+        walk_expr(self, ex)
     }
 }
 

clippy_utils/src/visitors.rs

@@ -109,23 +109,36 @@ pub fn for_each_expr_without_closures<'tcx, B, C: Continue>(
         res: Option<B>,
     }
     impl<'tcx, B, C: Continue, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B, C>> Visitor<'tcx> for V<B, F> {
-        fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) {
+        type Result = ControlFlow<()>;
+
+        fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) -> ControlFlow<()> {
             if self.res.is_some() {
-                return;
+                return ControlFlow::Break(());
             }
             match (self.f)(e) {
                 ControlFlow::Continue(c) if c.descend() => walk_expr(self, e),
-                ControlFlow::Break(b) => self.res = Some(b),
+                ControlFlow::Break(b) => {
-                ControlFlow::Continue(_) => (),
+                    self.res = Some(b);
+                    ControlFlow::Break(())
+                },
+                ControlFlow::Continue(_) => ControlFlow::Continue(()),
             }
         }
 
         // Avoid unnecessary `walk_*` calls.
-        fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) {}
+        fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) -> ControlFlow<()> {
-        fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) {}
+            ControlFlow::Continue(())
-        fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) {}
+        }
+        fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) -> ControlFlow<()> {
+            ControlFlow::Continue(())
+        }
+        fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) -> ControlFlow<()> {
+            ControlFlow::Continue(())
+        }
         // Avoid monomorphising all `visit_*` functions.
-        fn visit_nested_item(&mut self, _: ItemId) {}
+        fn visit_nested_item(&mut self, _: ItemId) -> ControlFlow<()> {
+            ControlFlow::Continue(())
+        }
     }
     let mut v = V { f, res: None };
     node.visit(&mut v);