Skip to content

Commit 3354876

Browse files
committed
Auto merge of #8182 - rust-lang:cache-test-items, r=giraffate
cache test item names This avoids quadratic behavior (collecting all test item names for each `eq_op` instance within the module). However, it invests a good deal of memory to buy this speedup. If that becomes a problem, I may need to change the cache to only store the chain of last visited modules. This hopefully fixes #8171. --- changelog: none
2 parents 5d194d1 + 8da9a82 commit 3354876

File tree

1 file changed

+42
-28
lines changed

1 file changed

+42
-28
lines changed

clippy_utils/src/lib.rs

+42-28
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
#![feature(box_patterns)]
2+
#![feature(control_flow_enum)]
23
#![feature(in_band_lifetimes)]
34
#![feature(let_else)]
5+
#![feature(once_cell)]
46
#![feature(rustc_private)]
5-
#![feature(control_flow_enum)]
67
#![recursion_limit = "512"]
78
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
89
#![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)]
@@ -60,9 +61,12 @@ pub use self::hir_utils::{both, count_eq, eq_expr_value, over, SpanlessEq, Spanl
6061

6162
use std::collections::hash_map::Entry;
6263
use std::hash::BuildHasherDefault;
64+
use std::lazy::SyncOnceCell;
65+
use std::sync::{Mutex, MutexGuard};
6366

6467
use if_chain::if_chain;
6568
use rustc_ast::ast::{self, Attribute, LitKind};
69+
use rustc_data_structures::fx::FxHashMap;
6670
use rustc_data_structures::unhash::UnhashMap;
6771
use rustc_hir as hir;
6872
use rustc_hir::def::{DefKind, Res};
@@ -87,6 +91,7 @@ use rustc_middle::ty::binding::BindingMode;
8791
use rustc_middle::ty::{layout::IntegerExt, BorrowKind, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable, UpvarCapture};
8892
use rustc_semver::RustcVersion;
8993
use rustc_session::Session;
94+
use rustc_span::def_id::LocalDefId;
9095
use rustc_span::hygiene::{ExpnKind, MacroKind};
9196
use rustc_span::source_map::original_sp;
9297
use rustc_span::sym;
@@ -2139,26 +2144,25 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
21392144
false
21402145
}
21412146

2142-
struct VisitConstTestStruct<'tcx> {
2147+
struct TestItemNamesVisitor<'tcx> {
21432148
tcx: TyCtxt<'tcx>,
21442149
names: Vec<Symbol>,
2145-
found: bool,
21462150
}
2147-
impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> {
2151+
2152+
impl<'hir> ItemLikeVisitor<'hir> for TestItemNamesVisitor<'hir> {
21482153
fn visit_item(&mut self, item: &Item<'_>) {
21492154
if let ItemKind::Const(ty, _body) = item.kind {
21502155
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
21512156
// We could also check for the type name `test::TestDescAndFn`
2152-
// and the `#[rustc_test_marker]` attribute?
21532157
if let Res::Def(DefKind::Struct, _) = path.res {
21542158
let has_test_marker = self
21552159
.tcx
21562160
.hir()
21572161
.attrs(item.hir_id())
21582162
.iter()
21592163
.any(|a| a.has_name(sym::rustc_test_marker));
2160-
if has_test_marker && self.names.contains(&item.ident.name) {
2161-
self.found = true;
2164+
if has_test_marker {
2165+
self.names.push(item.ident.name);
21622166
}
21632167
}
21642168
}
@@ -2169,32 +2173,42 @@ impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> {
21692173
fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {}
21702174
}
21712175

2176+
static TEST_ITEM_NAMES_CACHE: SyncOnceCell<Mutex<FxHashMap<LocalDefId, Vec<Symbol>>>> = SyncOnceCell::new();
2177+
2178+
fn with_test_item_names(tcx: TyCtxt<'tcx>, module: LocalDefId, f: impl Fn(&[Symbol]) -> bool) -> bool {
2179+
let cache = TEST_ITEM_NAMES_CACHE.get_or_init(|| Mutex::new(FxHashMap::default()));
2180+
let mut map: MutexGuard<'_, FxHashMap<LocalDefId, Vec<Symbol>>> = cache.lock().unwrap();
2181+
match map.entry(module) {
2182+
Entry::Occupied(entry) => f(entry.get()),
2183+
Entry::Vacant(entry) => {
2184+
let mut visitor = TestItemNamesVisitor { tcx, names: Vec::new() };
2185+
tcx.hir().visit_item_likes_in_module(module, &mut visitor);
2186+
visitor.names.sort_unstable();
2187+
f(&*entry.insert(visitor.names))
2188+
},
2189+
}
2190+
}
2191+
21722192
/// Checks if the function containing the given `HirId` is a `#[test]` function
21732193
///
21742194
/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`.
21752195
pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
2176-
let names: Vec<_> = tcx
2177-
.hir()
2178-
.parent_iter(id)
2179-
// Since you can nest functions we need to collect all until we leave
2180-
// function scope
2181-
.filter_map(|(_id, node)| {
2182-
if let Node::Item(item) = node {
2183-
if let ItemKind::Fn(_, _, _) = item.kind {
2184-
return Some(item.ident.name);
2196+
with_test_item_names(tcx, tcx.parent_module(id), |names| {
2197+
tcx.hir()
2198+
.parent_iter(id)
2199+
// Since you can nest functions we need to collect all until we leave
2200+
// function scope
2201+
.any(|(_id, node)| {
2202+
if let Node::Item(item) = node {
2203+
if let ItemKind::Fn(_, _, _) = item.kind {
2204+
// Note that we have sorted the item names in the visitor,
2205+
// so the binary_search gets the same as `contains`, but faster.
2206+
return names.binary_search(&item.ident.name).is_ok();
2207+
}
21852208
}
2186-
}
2187-
None
2188-
})
2189-
.collect();
2190-
let parent_mod = tcx.parent_module(id);
2191-
let mut vis = VisitConstTestStruct {
2192-
tcx,
2193-
names,
2194-
found: false,
2195-
};
2196-
tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis);
2197-
vis.found
2209+
false
2210+
})
2211+
})
21982212
}
21992213

22002214
/// Checks whether item either has `test` attribute applied, or

0 commit comments

Comments
 (0)