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.

Sign up for GitHub

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

Swap Visitors to early exit, instead of storing poison flag #13654

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Swap Visitors to early exit, instead of storing poison flag #13654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 7 additions & 9 deletions clippy_lints/src/copies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,24 +540,22 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
.iter()
.filter(|&&(_, name)| !name.as_str().starts_with('_'))
.any(|&(_, name)| {
let mut walker = ContainsName {
name,
result: false,
cx,
};
let mut walker = ContainsName { name, cx };

// Scan block
block
let mut res = block
.stmts
.iter()
.filter(|stmt| !ignore_span.overlaps(stmt.span))
.for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
.try_for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));

if let Some(expr) = block.expr {
intravisit::walk_expr(&mut walker, expr);
if res.is_continue() {
res = intravisit::walk_expr(&mut walker, expr);
}
}

walker.result
res.is_break()
})
})
}
Expand Down
38 changes: 19 additions & 19 deletions clippy_lints/src/derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::ControlFlow;

use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy};
use clippy_utils::{has_non_exhaustive_attr, is_lint_allowed, match_def_path, paths};
Expand Down Expand Up @@ -371,9 +373,8 @@ fn check_unsafe_derive_deserialize<'tcx>(
ty: Ty<'tcx>,
) {
fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
let mut visitor = UnsafeVisitor { cx, has_unsafe: false };
walk_item(&mut visitor, item);
visitor.has_unsafe
let mut visitor = UnsafeVisitor { cx };
walk_item(&mut visitor, item).is_break()
}

if let Some(trait_def_id) = trait_ref.trait_def_id()
Expand Down Expand Up @@ -406,38 +407,37 @@ fn check_unsafe_derive_deserialize<'tcx>(

struct UnsafeVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
has_unsafe: bool,
}

impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = nested_filter::All;

fn visit_fn(&mut self, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body_id: BodyId, _: Span, id: LocalDefId) {
if self.has_unsafe {
return;
}

fn visit_fn(
&mut self,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'_>,
body_id: BodyId,
_: Span,
id: LocalDefId,
) -> Self::Result {
if let Some(header) = kind.header()
&& header.safety == Safety::Unsafe
{
self.has_unsafe = true;
ControlFlow::Break(())
} else {
walk_fn(self, kind, decl, body_id, id)
}

walk_fn(self, kind, decl, body_id, id);
}

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.has_unsafe {
return;
}

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> Self::Result {
if let ExprKind::Block(block, _) = expr.kind {
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
self.has_unsafe = true;
return ControlFlow::Break(());
}
}

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

fn nested_visit_map(&mut self) -> Self::Map {
Expand Down
29 changes: 16 additions & 13 deletions clippy_lints/src/from_over_into.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::ControlFlow;

use clippy_config::Conf;
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_then;
Expand Down Expand Up @@ -115,43 +117,46 @@ impl<'tcx> LateLintPass<'tcx> for FromOverInto {
}

/// Finds the occurrences of `Self` and `self`
///
/// Returns `ControlFlow::break` if any of the `self`/`Self` usages were from an expansion, or the
/// body contained a binding already named `val`.
struct SelfFinder<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
/// Occurrences of `Self`
upper: Vec<Span>,
/// Occurrences of `self`
lower: Vec<Span>,
/// If any of the `self`/`Self` usages were from an expansion, or the body contained a binding
/// already named `val`
invalid: bool,
}

impl<'tcx> Visitor<'tcx> for SelfFinder<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = OnlyBodies;

fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}

fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) {
fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) -> Self::Result {
for segment in path.segments {
match segment.ident.name {
kw::SelfLower => self.lower.push(segment.ident.span),
kw::SelfUpper => self.upper.push(segment.ident.span),
_ => continue,
}

self.invalid |= segment.ident.span.from_expansion();
if segment.ident.span.from_expansion() {
return ControlFlow::Break(());
}
}

if !self.invalid {
walk_path(self, path);
}
walk_path(self, path)
}

fn visit_name(&mut self, name: Symbol) {
fn visit_name(&mut self, name: Symbol) -> Self::Result {
if name == sym::val {
self.invalid = true;
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
}
}
Expand Down Expand Up @@ -209,11 +214,9 @@ fn convert_to_from(
cx,
upper: Vec::new(),
lower: Vec::new(),
invalid: false,
};
finder.visit_expr(body.value);

if finder.invalid {
if finder.visit_expr(body.value).is_break() {
return None;
}

Expand Down
15 changes: 8 additions & 7 deletions clippy_lints/src/loops/while_immutable_condition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
cx,
ids: HirIdSet::default(),
def_ids: DefIdMap::default(),
skip: false,
};
var_visitor.visit_expr(cond);
if var_visitor.skip {
if var_visitor.visit_expr(cond).is_break() {
return;
}
let used_in_condition = &var_visitor.ids;
Expand Down Expand Up @@ -81,7 +79,6 @@ struct VarCollectorVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
ids: HirIdSet,
def_ids: DefIdMap<bool>,
skip: bool,
}

impl<'tcx> VarCollectorVisitor<'_, 'tcx> {
Expand All @@ -104,11 +101,15 @@ impl<'tcx> VarCollectorVisitor<'_, 'tcx> {
}

impl<'tcx> Visitor<'tcx> for VarCollectorVisitor<'_, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
type Result = ControlFlow<()>;
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) -> Self::Result {
match ex.kind {
ExprKind::Path(_) => self.insert_def_id(ex),
ExprKind::Path(_) => {
self.insert_def_id(ex);
ControlFlow::Continue(())
},
// If there is any function/method call… we just stop analysis
ExprKind::Call(..) | ExprKind::MethodCall(..) => self.skip = true,
ExprKind::Call(..) | ExprKind::MethodCall(..) => ControlFlow::Break(()),

_ => walk_expr(self, ex),
}
Expand Down
60 changes: 31 additions & 29 deletions clippy_lints/src/loops/while_let_on_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::ControlFlow;

use super::WHILE_LET_ON_ITERATOR;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
Expand Down Expand Up @@ -204,35 +206,32 @@ fn uses_iter<'tcx>(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tc
struct V<'a, 'b, 'tcx> {
cx: &'a LateContext<'tcx>,
iter_expr: &'b IterExpr,
uses_iter: bool,
}
impl<'tcx> Visitor<'tcx> for V<'_, '_, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.uses_iter {
// return
} else if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
self.uses_iter = true;
type Result = ControlFlow<()>;
fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
ControlFlow::Break(())
} else if let (e, true) = skip_fields_and_path(e) {
if let Some(e) = e {
self.visit_expr(e);
self.visit_expr(e)
} else {
ControlFlow::Continue(())
}
} else if let ExprKind::Closure(&Closure { body: id, .. }) = e.kind {
if is_res_used(self.cx, self.iter_expr.path, id) {
self.uses_iter = true;
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
} else {
walk_expr(self, e);
walk_expr(self, e)
}
}
}

let mut v = V {
cx,
iter_expr,
uses_iter: false,
};
v.visit_expr(container);
v.uses_iter
let mut v = V { cx, iter_expr };
v.visit_expr(container).is_break()
}

#[expect(clippy::too_many_lines)]
Expand All @@ -242,34 +241,38 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
iter_expr: &'b IterExpr,
loop_id: HirId,
after_loop: bool,
used_iter: bool,
}
impl<'tcx> Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> {
type NestedFilter = OnlyBodies;
type Result = ControlFlow<()>;
fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}

fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.used_iter {
return;
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
if self.after_loop {
if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
self.used_iter = true;
ControlFlow::Break(())
} else if let (e, true) = skip_fields_and_path(e) {
if let Some(e) = e {
self.visit_expr(e);
self.visit_expr(e)
} else {
ControlFlow::Continue(())
}
} else if let ExprKind::Closure(&Closure { body: id, .. }) = e.kind {
self.used_iter = is_res_used(self.cx, self.iter_expr.path, id);
if is_res_used(self.cx, self.iter_expr.path, id) {
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
} else {
walk_expr(self, e);
walk_expr(self, e)
}
} else if self.loop_id == e.hir_id {
self.after_loop = true;
ControlFlow::Continue(())
} else {
walk_expr(self, e);
walk_expr(self, e)
}
}
}
Expand Down Expand Up @@ -347,9 +350,8 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
iter_expr,
loop_id: loop_expr.hir_id,
after_loop: false,
used_iter: false,
};
v.visit_expr(cx.tcx.hir().body(cx.enclosing_body.unwrap()).value);
v.used_iter
v.visit_expr(cx.tcx.hir().body(cx.enclosing_body.unwrap()).value)
.is_break()
}
}
30 changes: 16 additions & 14 deletions clippy_lints/src/matches/match_str_case_mismatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::ControlFlow;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::ty::is_type_lang_item;
use rustc_ast::ast::LitKind;
Expand All @@ -23,11 +25,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arm
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(scrutinee).kind()
&& let ty::Str = ty.kind()
{
let mut visitor = MatchExprVisitor { cx, case_method: None };

visitor.visit_expr(scrutinee);

if let Some(case_method) = visitor.case_method {
let mut visitor = MatchExprVisitor { cx };
if let ControlFlow::Break(case_method) = visitor.visit_expr(scrutinee) {
if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) {
lint(cx, &case_method, bad_case_span, bad_case_sym.as_str());
}
Expand All @@ -37,30 +36,33 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arm

struct MatchExprVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
case_method: Option<CaseMethod>,
}

impl<'tcx> Visitor<'tcx> for MatchExprVisitor<'_, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
match ex.kind {
ExprKind::MethodCall(segment, receiver, [], _) if self.case_altered(segment.ident.as_str(), receiver) => {},
_ => walk_expr(self, ex),
type Result = ControlFlow<CaseMethod>;
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) -> Self::Result {
if let ExprKind::MethodCall(segment, receiver, [], _) = ex.kind {
let result = self.case_altered(segment.ident.as_str(), receiver);
if result.is_break() {
return result;
}
}

walk_expr(self, ex)
}
}

impl MatchExprVisitor<'_, '_> {
fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> bool {
fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> ControlFlow<CaseMethod> {
if let Some(case_method) = get_case_method(segment_ident) {
let ty = self.cx.typeck_results().expr_ty(receiver).peel_refs();

if is_type_lang_item(self.cx, ty, LangItem::String) || ty.kind() == &ty::Str {
self.case_method = Some(case_method);
return true;
return ControlFlow::Break(case_method);
}
}

false
ControlFlow::Continue(())
}
}

Expand Down
Loading