Skip to content

[beta] Clippy backport ICE/infinite loop fix #96740

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

Closed
wants to merge 2 commits into from
Closed
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: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
LintId::of(no_effect::NO_EFFECT),
LintId::of(no_effect::UNNECESSARY_OPERATION),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(precedence::PRECEDENCE),
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(mutex_atomic::MUTEX_INTEGER),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),
Expand Down
56 changes: 24 additions & 32 deletions src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use std::collections::VecDeque;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_lint_allowed;
use itertools::{izip, Itertools};
use rustc_ast::{walk_list, Label, Mutability};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
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 All @@ -33,6 +34,9 @@ declare_clippy_lint! {
/// and the assigned variables are also only in recursion, it is useless.
///
/// ### Known problems
/// Too many code paths in the linting code are currently untested and prone to produce false
/// positives or are prone to have performance implications.
///
/// In some cases, this would not catch all useless arguments.
///
/// ```rust
Expand Down Expand Up @@ -85,7 +89,7 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.60.0"]
pub ONLY_USED_IN_RECURSION,
complexity,
nursery,
"arguments that is only used in recursion can be removed"
}
declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
Expand All @@ -100,6 +104,9 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
_: Span,
id: HirId,
) {
if is_lint_allowed(cx, ONLY_USED_IN_RECURSION, id) {
return;
}
if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind {
let def_id = id.owner.to_def_id();
let data = cx.tcx.def_path(def_id).data;
Expand Down Expand Up @@ -145,7 +152,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 +214,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 +252,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) => {
self.ret_vars = exprs
Expand All @@ -307,7 +299,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