Skip to content

Commit 252f598

Browse files
smoeliussmoelius
committed
Address review comments
1 parent 52318c0 commit 252f598

File tree

3 files changed

+94
-96
lines changed

3 files changed

+94
-96
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@ use rustc_hir::{
2020
};
2121
use rustc_index::bit_set::BitSet;
2222
use rustc_infer::infer::TyCtxtInferExt;
23-
use rustc_lint::{LateContext, LateLintPass, LintArray, LintPass};
24-
use rustc_lint_defs::lint_array;
23+
use rustc_lint::{LateContext, LateLintPass};
2524
use rustc_middle::mir::{Rvalue, StatementKind};
2625
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
2726
use rustc_middle::ty::{
2827
self, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
2928
ProjectionPredicate, Ty, TyCtxt, TypeVisitable, TypeckResults,
3029
};
3130
use rustc_semver::RustcVersion;
32-
use rustc_session::declare_tool_lint;
31+
use rustc_session::{declare_tool_lint, impl_lint_pass};
3332
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
3433
use rustc_trait_selection::infer::InferCtxtExt as _;
3534
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
@@ -146,24 +145,12 @@ declare_clippy_lint! {
146145
"dereferencing when the compiler would automatically dereference"
147146
}
148147

149-
#[expect(rustc::internal)]
150-
impl<'tcx> LintPass for Dereferencing<'tcx> {
151-
fn name(&self) -> &'static str {
152-
stringify!($ty)
153-
}
154-
}
155-
156-
impl<'tcx> Dereferencing<'tcx> {
157-
#[expect(dead_code)]
158-
pub fn get_lints() -> LintArray {
159-
lint_array!(
160-
EXPLICIT_DEREF_METHODS,
161-
NEEDLESS_BORROW,
162-
REF_BINDING_TO_REFERENCE,
163-
EXPLICIT_AUTO_DEREF,
164-
)
165-
}
166-
}
148+
impl_lint_pass!(Dereferencing<'_> => [
149+
EXPLICIT_DEREF_METHODS,
150+
NEEDLESS_BORROW,
151+
REF_BINDING_TO_REFERENCE,
152+
EXPLICIT_AUTO_DEREF,
153+
]);
167154

168155
#[derive(Default)]
169156
pub struct Dereferencing<'tcx> {
@@ -187,9 +174,10 @@ pub struct Dereferencing<'tcx> {
187174
/// e.g. `m!(x) | Foo::Bar(ref x)`
188175
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
189176

190-
/// Map from body owners to `PossibleBorrowerMap`s. Used by `needless_borrow_impl_arg_position`
191-
/// to determine when a borrowed expression can instead be moved.
192-
possible_borrowers: FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
177+
/// Stack of (body owner, `PossibleBorrowerMap`) pairs. Used by
178+
/// `needless_borrow_impl_arg_position` to determine when a borrowed expression can instead
179+
/// be moved.
180+
possible_borrowers: Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
193181

194182
// `IntoIterator` for arrays requires Rust 1.53.
195183
msrv: Option<RustcVersion>,
@@ -571,6 +559,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
571559
}
572560

573561
fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
562+
if self.possible_borrowers.last().map_or(false, |&(local_def_id, _)| {
563+
local_def_id == cx.tcx.hir().body_owner_def_id(body.id())
564+
}) {
565+
self.possible_borrowers.pop();
566+
}
567+
574568
if Some(body.id()) == self.current_body {
575569
for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) {
576570
let replacements = pat.replacements;
@@ -703,7 +697,7 @@ impl Position {
703697
#[expect(clippy::too_many_lines)]
704698
fn walk_parents<'tcx>(
705699
cx: &LateContext<'tcx>,
706-
possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
700+
possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
707701
e: &'tcx Expr<'_>,
708702
msrv: Option<RustcVersion>,
709703
) -> (Position, &'tcx [Adjustment<'tcx>]) {
@@ -1061,7 +1055,7 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
10611055
#[expect(clippy::too_many_arguments)]
10621056
fn needless_borrow_impl_arg_position<'tcx>(
10631057
cx: &LateContext<'tcx>,
1064-
possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
1058+
possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
10651059
parent: &Expr<'tcx>,
10661060
arg_index: usize,
10671061
param_ty: ParamTy,
@@ -1202,7 +1196,7 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
12021196

12031197
fn referent_used_exactly_once<'a, 'tcx>(
12041198
cx: &'a LateContext<'tcx>,
1205-
possible_borrower: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
1199+
possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
12061200
reference: &Expr<'tcx>,
12071201
) -> bool {
12081202
let mir = enclosing_mir(cx.tcx, reference.hir_id);
@@ -1213,9 +1207,13 @@ fn referent_used_exactly_once<'a, 'tcx>(
12131207
&& !place.has_deref()
12141208
{
12151209
let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
1216-
let possible_borrower = possible_borrower
1217-
.entry(body_owner_local_def_id)
1218-
.or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
1210+
if possible_borrowers
1211+
.last()
1212+
.map_or(true, |&(local_def_id, _)| local_def_id != body_owner_local_def_id)
1213+
{
1214+
possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
1215+
}
1216+
let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
12191217
// If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
12201218
// that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
12211219
// itself. See the comment in that method for an explanation as to why.

‎clippy_lints/src/lib.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ extern crate rustc_index;
3737
extern crate rustc_infer;
3838
extern crate rustc_lexer;
3939
extern crate rustc_lint;
40-
extern crate rustc_lint_defs;
4140
extern crate rustc_middle;
4241
extern crate rustc_parse;
4342
extern crate rustc_session;

‎clippy_lints/src/utils/internal_lints.rs‎

Lines changed: 66 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -429,86 +429,87 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass {
429429
return;
430430
}
431431

432-
match &item.kind {
433-
hir::ItemKind::Static(ty, Mutability::Not, body_id) => {
434-
let is_lint_ref_ty = is_lint_ref_type(cx, ty);
435-
if is_deprecated_lint(cx, ty) || is_lint_ref_ty {
436-
check_invalid_clippy_version_attribute(cx, item);
437-
438-
let expr = &cx.tcx.hir().body(*body_id).value;
439-
let fields;
440-
if is_lint_ref_ty {
441-
if let ExprKind::AddrOf(_, _, inner_exp) = expr.kind
442-
&& let ExprKind::Struct(_, struct_fields, _) = inner_exp.kind {
443-
fields = struct_fields;
444-
} else {
445-
return;
446-
}
447-
} else if let ExprKind::Struct(_, struct_fields, _) = expr.kind {
448-
fields = struct_fields;
432+
if let hir::ItemKind::Static(ty, Mutability::Not, body_id) = item.kind {
433+
let is_lint_ref_ty = is_lint_ref_type(cx, ty);
434+
if is_deprecated_lint(cx, ty) || is_lint_ref_ty {
435+
check_invalid_clippy_version_attribute(cx, item);
436+
437+
let expr = &cx.tcx.hir().body(body_id).value;
438+
let fields;
439+
if is_lint_ref_ty {
440+
if let ExprKind::AddrOf(_, _, inner_exp) = expr.kind
441+
&& let ExprKind::Struct(_, struct_fields, _) = inner_exp.kind {
442+
fields = struct_fields;
449443
} else {
450444
return;
451445
}
446+
} else if let ExprKind::Struct(_, struct_fields, _) = expr.kind {
447+
fields = struct_fields;
448+
} else {
449+
return;
450+
}
452451

453-
let field = fields
454-
.iter()
455-
.find(|f| f.ident.as_str() == "desc")
456-
.expect("lints must have a description field");
457-
458-
if let ExprKind::Lit(Spanned {
459-
node: LitKind::Str(ref sym, _),
460-
..
461-
}) = field.expr.kind
462-
{
463-
let sym_str = sym.as_str();
464-
if is_lint_ref_ty {
465-
if sym_str == "default lint description" {
466-
span_lint(
467-
cx,
468-
DEFAULT_LINT,
469-
item.span,
470-
&format!("the lint `{}` has the default lint description", item.ident.name),
471-
);
472-
}
452+
let field = fields
453+
.iter()
454+
.find(|f| f.ident.as_str() == "desc")
455+
.expect("lints must have a description field");
473456

474-
self.declared_lints.insert(item.ident.name, item.span);
475-
} else if sym_str == "default deprecation note" {
457+
if let ExprKind::Lit(Spanned {
458+
node: LitKind::Str(ref sym, _),
459+
..
460+
}) = field.expr.kind
461+
{
462+
let sym_str = sym.as_str();
463+
if is_lint_ref_ty {
464+
if sym_str == "default lint description" {
476465
span_lint(
477466
cx,
478-
DEFAULT_DEPRECATION_REASON,
467+
DEFAULT_LINT,
479468
item.span,
480-
&format!("the lint `{}` has the default deprecation reason", item.ident.name),
469+
&format!("the lint `{}` has the default lint description", item.ident.name),
481470
);
482471
}
472+
473+
self.declared_lints.insert(item.ident.name, item.span);
474+
} else if sym_str == "default deprecation note" {
475+
span_lint(
476+
cx,
477+
DEFAULT_DEPRECATION_REASON,
478+
item.span,
479+
&format!("the lint `{}` has the default deprecation reason", item.ident.name),
480+
);
483481
}
484482
}
485-
},
486-
hir::ItemKind::Impl(hir::Impl {
483+
}
484+
} else if let Some(macro_call) = root_macro_call_first_node(cx, item) {
485+
if !matches!(
486+
cx.tcx.item_name(macro_call.def_id).as_str(),
487+
"impl_lint_pass" | "declare_lint_pass"
488+
) {
489+
return;
490+
}
491+
if let hir::ItemKind::Impl(hir::Impl {
487492
of_trait: None,
488493
items: impl_item_refs,
489494
..
490-
}) => {
491-
let macro_call = root_macro_call_first_node(cx, item);
492-
if let Some(macro_call) = &macro_call
493-
&& !matches!(
494-
cx.tcx.item_name(macro_call.def_id).as_str(),
495-
"impl_lint_pass" | "declare_lint_pass"
496-
)
497-
{
498-
return;
499-
}
500-
if let Some(iiref) = impl_item_refs.iter().find(|iiref| iiref.ident.as_str() == "get_lints") {
501-
let mut collector = LintCollector {
502-
output: &mut self.registered_lints,
503-
cx,
504-
};
505-
let body_id = cx.tcx.hir().body_owned_by(iiref.id.def_id.def_id);
506-
collector.visit_expr(cx.tcx.hir().body(body_id).value);
507-
} else {
508-
assert!(macro_call.is_none(), "LintPass needs to implement get_lints");
509-
}
510-
},
511-
_ => {},
495+
}) = item.kind
496+
{
497+
let mut collector = LintCollector {
498+
output: &mut self.registered_lints,
499+
cx,
500+
};
501+
let body_id = cx.tcx.hir().body_owned_by(
502+
cx.tcx.hir().local_def_id(
503+
impl_item_refs
504+
.iter()
505+
.find(|iiref| iiref.ident.as_str() == "get_lints")
506+
.expect("LintPass needs to implement get_lints")
507+
.id
508+
.hir_id(),
509+
),
510+
);
511+
collector.visit_expr(cx.tcx.hir().body(body_id).value);
512+
}
512513
}
513514
}
514515

0 commit comments

Comments
 (0)