Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ControlFlow in more places #12830

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
18 changes: 8 additions & 10 deletions clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -380,11 +381,8 @@ fn could_use_elision<'tcx>(
return None;
}

let mut checker = BodyLifetimeChecker {
lifetimes_used_in_body: false,
};
checker.visit_expr(body.value);
if checker.lifetimes_used_in_body {
let mut checker = BodyLifetimeChecker;
if checker.visit_expr(body.value).is_break() {
return None;
}
}
Expand Down Expand Up @@ -694,15 +692,15 @@ fn report_extra_impl_lifetimes<'tcx>(cx: &LateContext<'tcx>, impl_: &'tcx Impl<'
}
}

struct BodyLifetimeChecker {
lifetimes_used_in_body: bool,
}
struct BodyLifetimeChecker;

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(())
}
}
15 changes: 6 additions & 9 deletions clippy_lints/src/loops/mut_range_bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -114,7 +115,6 @@ impl MutatePairDelegate<'_, '_> {
struct BreakAfterExprVisitor {
hir_id: HirId,
past_expr: bool,
past_candidate: bool,
break_after_expr: bool,
}

Expand All @@ -123,7 +123,6 @@ impl BreakAfterExprVisitor {
let mut visitor = BreakAfterExprVisitor {
hir_id,
past_expr: false,
past_candidate: false,
break_after_expr: false,
};

Expand All @@ -135,21 +134,19 @@ impl BreakAfterExprVisitor {
}

impl<'tcx> Visitor<'tcx> for BreakAfterExprVisitor {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if self.past_candidate {
return;
}

type Result = ControlFlow<()>;
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
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)
}
}
}
24 changes: 8 additions & 16 deletions clippy_lints/src/loops/while_immutable_condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
has_break_or_return: false,
};
has_break_or_return_visitor.visit_expr(expr);
let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor;
let has_break_or_return = has_break_or_return_visitor.visit_expr(expr).is_break();

if no_cond_variable_mutated && !mutable_static_in_cond {
span_lint_and_then(
Expand All @@ -59,25 +57,19 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
}
}

struct HasBreakOrReturnVisitor {
has_break_or_return: bool,
}
struct HasBreakOrReturnVisitor;

impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.has_break_or_return {
return;
}

type Result = ControlFlow<()>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<()> {
match expr.kind {
ExprKind::Ret(_) | ExprKind::Break(_, _) => {
self.has_break_or_return = true;
return;
return ControlFlow::Break(());
},
_ => {},
}

walk_expr(self, expr);
walk_expr(self, expr)
}
}

Expand Down
32 changes: 15 additions & 17 deletions clippy_lints/src/methods/option_map_unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -151,29 +151,27 @@ 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
&& let Res::Local(local_id) = path.res
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
&& let PatKind::Binding(_, local_id, ..) = pat.kind
&& self.identifiers.contains(&local_id)
{
self.found_reference = true;
}
rustc_hir::intravisit::walk_expr(self, expr);
if expr.span < self.unwrap_or_span
&& let ExprKind::Path(ref path) = expr.kind
&& let QPath::Resolved(_, path) = path
&& let Res::Local(local_id) = path.res
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
&& let PatKind::Binding(_, local_id, ..) = pat.kind
&& self.identifiers.contains(&local_id)
{
return ControlFlow::Break(());
}
rustc_hir::intravisit::walk_expr(self, expr)
}

fn nested_visit_map(&mut self) -> Self::Map {
Expand Down
25 changes: 8 additions & 17 deletions clippy_lints/src/redundant_closure_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
found_return: bool,
}

impl ReturnVisitor {
#[must_use]
fn new() -> Self {
Self { found_return: false }
}
}
struct ReturnVisitor;

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;
} else {
hir_visit::walk_expr(self, ex);
return ControlFlow::Break(());
}
hir_visit::walk_expr(self, ex)
}
}

Expand Down Expand Up @@ -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();
visitor.visit_expr(body.value);
!visitor.found_return
let mut visitor = ReturnVisitor;
!visitor.visit_expr(body.value).is_break()
}
&& steps > 0
{
Expand Down
14 changes: 6 additions & 8 deletions clippy_lints/src/unconditional_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}

Expand All @@ -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>) {
if self.found_default_call {
return;
}
walk_expr(self, expr);
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
walk_expr(self, expr)?;

if let ExprKind::Call(f, _) = expr.kind
&& let ExprKind::Path(qpath) = f.kind
Expand All @@ -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(())
}
}

Expand Down Expand Up @@ -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);
Expand Down
Loading