Auto merge of #13654 - GnomedDev:early-exit-visitors, r=Alexendoo

Swap Visitors to early exit, instead of storing poison flag

I noticed that a bunch of visitors had a `poison` or `success` field, when they could just be returning `ControlFlow`.

changelog: none
This commit is contained in:
bors 2024-11-05 17:39:32 +00:00
commit 6631a2c1b7
9 changed files with 148 additions and 158 deletions

View File

@ -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()
})
})
}

View File

@ -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};
@ -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()
@ -406,38 +407,37 @@ fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
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 {

View File

@ -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;
@ -115,25 +117,26 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
}
/// 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),
@ -141,17 +144,19 @@ fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) {
_ => 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(())
}
}
}
@ -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;
}

View File

@ -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;
@ -81,7 +79,6 @@ struct VarCollectorVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
ids: HirIdSet,
def_ids: DefIdMap<bool>,
skip: bool,
}
impl<'tcx> VarCollectorVisitor<'_, 'tcx> {
@ -104,11 +101,15 @@ fn insert_def_id(&mut self, ex: &'tcx Expr<'_>) {
}
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),
}

View File

@ -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;
@ -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)]
@ -242,34 +241,38 @@ struct AfterLoopVisitor<'a, 'b, 'tcx> {
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)
}
}
}
@ -347,9 +350,8 @@ fn visit_expr(&mut self, e: &'tcx 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()
}
}

View File

@ -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;
@ -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());
}
@ -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(())
}
}

View File

@ -1338,15 +1338,17 @@ pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> {
pub struct ContainsName<'a, 'tcx> {
pub cx: &'a LateContext<'tcx>,
pub name: Symbol,
pub result: bool,
}
impl<'tcx> Visitor<'tcx> for ContainsName<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = nested_filter::OnlyBodies;
fn visit_name(&mut self, name: Symbol) {
fn visit_name(&mut self, name: Symbol) -> Self::Result {
if self.name == name {
self.result = true;
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
}
@ -1357,13 +1359,8 @@ fn nested_visit_map(&mut self) -> Self::Map {
/// Checks if an `Expr` contains a certain name.
pub fn contains_name<'tcx>(name: Symbol, expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> bool {
let mut cn = ContainsName {
name,
result: false,
cx,
};
cn.visit_expr(expr);
cn.result
let mut cn = ContainsName { cx, name };
cn.visit_expr(expr).is_break()
}
/// Returns `true` if `expr` contains a return expression

View File

@ -109,34 +109,28 @@ fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) {
pub struct BindingUsageFinder<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
binding_ids: Vec<HirId>,
usage_found: bool,
}
impl<'a, 'tcx> BindingUsageFinder<'a, 'tcx> {
pub fn are_params_used(cx: &'a LateContext<'tcx>, body: &'tcx hir::Body<'tcx>) -> bool {
let mut finder = BindingUsageFinder {
cx,
binding_ids: ParamBindingIdCollector::collect_binding_hir_ids(body),
usage_found: false,
};
finder.visit_body(body);
finder.usage_found
finder.visit_body(body).is_break()
}
}
impl<'tcx> Visitor<'tcx> for BindingUsageFinder<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = nested_filter::OnlyBodies;
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if !self.usage_found {
intravisit::walk_expr(self, expr);
}
}
fn visit_path(&mut self, path: &hir::Path<'tcx>, _: HirId) {
fn visit_path(&mut self, path: &hir::Path<'tcx>, _: HirId) -> Self::Result {
if let Res::Local(id) = path.res {
if self.binding_ids.contains(&id) {
self.usage_found = true;
return ControlFlow::Break(());
}
}
ControlFlow::Continue(())
}
fn nested_visit_map(&mut self) -> Self::Map {

View File

@ -324,17 +324,15 @@ pub fn is_local_used<'tcx>(cx: &LateContext<'tcx>, visitable: impl Visitable<'tc
pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
is_const: bool,
}
impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = intravisit::nested_filter::None;
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if !self.is_const {
return;
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
match e.kind {
ExprKind::ConstBlock(_) => return,
ExprKind::ConstBlock(_) => return ControlFlow::Continue(()),
ExprKind::Call(
&Expr {
kind: ExprKind::Path(ref p),
@ -394,37 +392,34 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
| ExprKind::Type(..) => (),
_ => {
self.is_const = false;
return;
return ControlFlow::Break(());
},
}
walk_expr(self, e);
walk_expr(self, e)
}
}
let mut v = V { cx, is_const: true };
v.visit_expr(e);
v.is_const
let mut v = V { cx };
v.visit_expr(e).is_continue()
}
/// Checks if the given expression performs an unsafe operation outside of an unsafe block.
pub fn is_expr_unsafe<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
is_unsafe: bool,
}
impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
type NestedFilter = nested_filter::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.is_unsafe {
return;
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
match e.kind {
ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_unsafe_ptr() => {
self.is_unsafe = true;
ControlFlow::Break(())
},
ExprKind::MethodCall(..)
if self
@ -435,13 +430,13 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
self.cx.tcx.fn_sig(id).skip_binder().safety() == Safety::Unsafe
}) =>
{
self.is_unsafe = true;
ControlFlow::Break(())
},
ExprKind::Call(func, _) => match *self.cx.typeck_results().expr_ty(func).peel_refs().kind() {
ty::FnDef(id, _) if self.cx.tcx.fn_sig(id).skip_binder().safety() == Safety::Unsafe => {
self.is_unsafe = true;
ControlFlow::Break(())
},
ty::FnPtr(_, hdr) if hdr.safety == Safety::Unsafe => self.is_unsafe = true,
ty::FnPtr(_, hdr) if hdr.safety == Safety::Unsafe => ControlFlow::Break(()),
_ => walk_expr(self, e),
},
ExprKind::Path(ref p)
@ -451,56 +446,54 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
.opt_def_id()
.map_or(false, |id| self.cx.tcx.is_mutable_static(id)) =>
{
self.is_unsafe = true;
ControlFlow::Break(())
},
_ => walk_expr(self, e),
}
}
fn visit_block(&mut self, b: &'tcx Block<'_>) {
if !matches!(b.rules, BlockCheckMode::UnsafeBlock(_)) {
walk_block(self, b);
fn visit_block(&mut self, b: &'tcx Block<'_>) -> Self::Result {
if matches!(b.rules, BlockCheckMode::UnsafeBlock(_)) {
ControlFlow::Continue(())
} else {
walk_block(self, b)
}
}
fn visit_nested_item(&mut self, id: ItemId) {
if let ItemKind::Impl(i) = &self.cx.tcx.hir().item(id).kind {
self.is_unsafe = i.safety == Safety::Unsafe;
fn visit_nested_item(&mut self, id: ItemId) -> Self::Result {
if let ItemKind::Impl(i) = &self.cx.tcx.hir().item(id).kind
&& i.safety == Safety::Unsafe
{
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
}
}
let mut v = V { cx, is_unsafe: false };
v.visit_expr(e);
v.is_unsafe
let mut v = V { cx };
v.visit_expr(e).is_break()
}
/// Checks if the given expression contains an unsafe block
pub fn contains_unsafe_block<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
struct V<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
found_unsafe: bool,
}
impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
type Result = ControlFlow<()>;
type NestedFilter = nested_filter::OnlyBodies;
fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}
fn visit_block(&mut self, b: &'tcx Block<'_>) {
if self.found_unsafe {
return;
}
fn visit_block(&mut self, b: &'tcx Block<'_>) -> Self::Result {
if b.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
self.found_unsafe = true;
return;
ControlFlow::Break(())
} else {
walk_block(self, b)
}
walk_block(self, b);
}
}
let mut v = V {
cx,
found_unsafe: false,
};
v.visit_expr(e);
v.found_unsafe
let mut v = V { cx };
v.visit_expr(e).is_break()
}
/// Runs the given function for each sub-expression producing the final value consumed by the parent