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

Prevent infinite (exponential) recursion in only_used_in_recursion #8691

Merged
merged 1 commit into from
Apr 12, 2022
+16 −31
Merged

Prevent infinite (exponential) recursion in only_used_in_recursion #8691

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
47 changes: 16 additions & 31 deletions clippy_lints/src/only_used_in_recursion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
use rustc_hir::intravisit::{walk_expr, walk_stmt, FnKind, Visitor};
use rustc_hir::{
Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment,
QPath, Stmt, StmtKind, TyKind, UnOp,
Expand Down Expand Up @@ -145,7 +145,8 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
is_method: matches!(kind, FnKind::Method(..)),
has_self,
ty_res,
ty_ctx: cx.tcx,
tcx: cx.tcx,
visited_exprs: FxHashSet::default(),
};

visitor.visit_expr(&body.value);
Expand Down Expand Up @@ -206,19 +207,13 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
}

pub fn is_primitive(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true,
ty::Ref(_, t, _) => is_primitive(*t),
_ => false,
}
let ty = ty.peel_refs();
ty.is_primitive() || ty.is_str()
}

pub fn is_array(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Array(..) | ty::Slice(..) => true,
ty::Ref(_, t, _) => is_array(*t),
_ => false,
}
let ty = ty.peel_refs();
ty.is_array() || ty.is_array_slice()
}

/// This builds the graph of side effect.
Expand Down Expand Up @@ -250,40 +245,30 @@ pub struct SideEffectVisit<'tcx> {
is_method: bool,
has_self: bool,
ty_res: &'tcx TypeckResults<'tcx>,
ty_ctx: TyCtxt<'tcx>,
tcx: TyCtxt<'tcx>,
visited_exprs: FxHashSet<HirId>,
}

impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
b.stmts.iter().for_each(|stmt| {
self.visit_stmt(stmt);
self.ret_vars.clear();
});
walk_list!(self, visit_expr, b.expr);
}

fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
match s.kind {
StmtKind::Local(Local {
pat, init: Some(init), ..
}) => {
self.visit_pat_expr(pat, init, false);
self.ret_vars.clear();
},
StmtKind::Item(i) => {
let item = self.ty_ctx.hir().item(i);
self.visit_item(item);
self.ret_vars.clear();
},
StmtKind::Expr(e) | StmtKind::Semi(e) => {
self.visit_expr(e);
self.ret_vars.clear();
StmtKind::Item(_) | StmtKind::Expr(_) | StmtKind::Semi(_) => {
walk_stmt(self, s);
},
StmtKind::Local(_) => {},
}
self.ret_vars.clear();
}

fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if !self.visited_exprs.insert(ex.hir_id) {
return;
}
match ex.kind {
ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example for missing tests: This whole arm could be removed completely and the tests keep passing, even though this changes the semantics of the lint.

self.ret_vars = exprs
Expand All @@ -307,7 +292,7 @@ impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms),
// since analysing the closure is not easy, just set all variables in it to side-effect
ExprKind::Closure(_, _, body_id, _, _) => {
let body = self.ty_ctx.hir().body(body_id);
let body = self.tcx.hir().body(body_id);
self.visit_body(body);
let vars = std::mem::take(&mut self.ret_vars);
self.add_side_effect(vars);
Expand Down