Skip to content

Commit

Permalink
Replace local variables signifying "done" or "loop break", use Contro…
Browse files Browse the repository at this point in the history
…lFlow #12830
  • Loading branch information
blyxyas committed Jul 20, 2024
1 parent cf4270d commit 4e222c6
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 140 deletions.
39 changes: 20 additions & 19 deletions clippy_lints/src/if_let_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;
use std::ops::ControlFlow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -44,21 +45,23 @@ declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]);

impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
let mut arm_visit = ArmVisitor { found_mutex: None, cx };
let mut op_visit = OppVisitor { found_mutex: None, cx };
let mut arm_visit = ArmVisitor { cx };
let mut op_visit = OppVisitor { cx };
if let Some(higher::IfLet {
let_expr,
if_then,
if_else: Some(if_else),
..
}) = higher::IfLet::hir(cx, expr)
{
op_visit.visit_expr(let_expr);
if let Some(op_mutex) = op_visit.found_mutex {
arm_visit.visit_expr(if_then);
arm_visit.visit_expr(if_else);
let found_op_mutex = op_visit.visit_expr(let_expr).break_value();
if let Some(op_mutex) = found_op_mutex {
let mut found_mutex = arm_visit.visit_expr(if_then).break_value();
if found_mutex.is_none() {
found_mutex = arm_visit.visit_expr(if_else).break_value();
};

if let Some(arm_mutex) = arm_visit.found_mutex_if_same_as(op_mutex) {
if let Some(arm_mutex) = arm_visit.found_mutex_if_same_as(op_mutex, found_mutex) {
let diag = |diag: &mut Diag<'_, ()>| {
diag.span_label(
op_mutex.span,
Expand All @@ -85,39 +88,37 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {

/// Checks if `Mutex::lock` is called in the `if let` expr.
pub struct OppVisitor<'a, 'tcx> {
found_mutex: Option<&'tcx Expr<'tcx>>,
cx: &'a LateContext<'tcx>,
}

impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
type Result = ControlFlow<&'tcx Expr<'tcx>>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<&'tcx Expr<'tcx>> {
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
self.found_mutex = Some(mutex);
return;
return ControlFlow::Break(mutex);
}
visit::walk_expr(self, expr);
visit::walk_expr(self, expr)
}
}

/// Checks if `Mutex::lock` is called in any of the branches.
pub struct ArmVisitor<'a, 'tcx> {
found_mutex: Option<&'tcx Expr<'tcx>>,
cx: &'a LateContext<'tcx>,
}

impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
type Result = ControlFlow<&'tcx Expr<'tcx>>;
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<&'tcx Expr<'tcx>> {
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
self.found_mutex = Some(mutex);
return;
return ControlFlow::Break(mutex);
}
visit::walk_expr(self, expr);
visit::walk_expr(self, expr)
}
}

impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
fn found_mutex_if_same_as(&self, op_mutex: &Expr<'_>) -> Option<&Expr<'_>> {
self.found_mutex.and_then(|arm_mutex| {
fn found_mutex_if_same_as(&self, op_mutex: &Expr<'_>, found_mutex: Option<&'tcx Expr<'tcx>>) -> Option<&Expr<'_>> {
found_mutex.and_then(|arm_mutex| {
SpanlessEq::new(self.cx)
.eq_expr(op_mutex, arm_mutex)
.then_some(arm_mutex)
Expand Down
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;
return 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
34 changes: 16 additions & 18 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,17 +55,16 @@ 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;
}
};
}

if !unwrap_arg.span.eq_ctxt(map_span) {
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
Loading

0 comments on commit 4e222c6

Please sign in to comment.