Skip to content

Commit 9bb2f00

Browse files
blyxyasblyxyas
committed
Follow review comments (optimize the filtering)
1 parent 53e0eba commit 9bb2f00

File tree

3 files changed

+147
-110
lines changed

3 files changed

+147
-110
lines changed

‎compiler/rustc_lint/src/late.rs

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use rustc_span::Span;
3030
use tracing::debug;
3131

3232
use crate::passes::LateLintPassObject;
33-
use crate::{LateContext, LateLintPass, LintStore};
33+
use crate::{LateContext, LateLintPass, LintStore, LintId};
3434

3535
/// Extract the [`LintStore`] from [`Session`].
3636
///
@@ -372,27 +372,16 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
372372
} else {
373373
let passes: Vec<_> =
374374
store.late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
375-
376375
// Filter unused lints
377-
let (lints_that_actually_run, lints_allowed) = &**tcx.lints_that_can_emit(());
378-
// let lints_that_actually_run = &lints_that_can_emit.0;
379-
// let lints_allowed = &lints_that_can_emit.1;
380-
381-
// Now, we'll filtered passes in a way that discards any lint that won't trigger.
382-
// If any lint is a given pass is detected to be emitted, we will keep that pass.
383-
// Otherwise, we don't
376+
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
384377
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
385378
.into_iter()
386379
.filter(|pass| {
387-
let pass = LintPass::get_lints(pass);
388-
pass.iter().any(|&lint| {
389-
let lint_name = name_without_tool(&lint.name.to_lowercase()).to_string();
390-
lints_that_actually_run.contains(&lint_name)
391-
|| (!lints_allowed.contains(&lint_name)
392-
&& lint.default_level != crate::Level::Allow)
393-
})
394-
})
395-
.collect();
380+
let lints = LintPass::get_lints(pass);
381+
lints.iter()
382+
.any(|lint|
383+
!lints_that_dont_need_to_run.contains(&LintId::of(lint)))
384+
}).collect();
396385

397386
filtered_passes.push(Box::new(builtin_lints));
398387

@@ -427,7 +416,7 @@ fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
427416

428417
fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
429418
// Note: `passes` is often empty.
430-
let mut passes: Vec<_> =
419+
let passes: Vec<_> =
431420
unerased_lint_store(tcx.sess).late_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
432421

433422
if passes.is_empty() {
@@ -445,7 +434,29 @@ fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
445434
only_module: false,
446435
};
447436

448-
let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
437+
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
438+
439+
// dbg!(&lints_that_dont_need_to_run);
440+
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
441+
.into_iter()
442+
.filter(|pass|{
443+
let lints = LintPass::get_lints(pass);
444+
// dbg!(&lints);
445+
lints.iter()
446+
.any(|lint|
447+
!lints_that_dont_need_to_run.contains(&LintId::of(lint)))
448+
}).collect();
449+
450+
// let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
451+
// .into_iter()
452+
// .filter(|pass| {
453+
// let lints = LintPass::get_lints(pass);
454+
// lints.iter()
455+
// .any(|lint|
456+
// !lints_that_dont_need_to_run.contains(&LintId::of(lint)))
457+
// }).collect();
458+
459+
let pass = RuntimeCombinedLateLintPass { passes: &mut filtered_passes[..] };
449460
late_lint_crate_inner(tcx, context, pass);
450461
}
451462

@@ -483,10 +494,3 @@ pub fn check_crate<'tcx>(tcx: TyCtxt<'tcx>) {
483494
},
484495
);
485496
}
486-
487-
/// Format name ignoring the name, useful for filtering non-used lints.
488-
/// For example, 'clippy::my_lint' will turn into 'my_lint'
489-
pub(crate) fn name_without_tool(name: &str) -> &str {
490-
// Doing some calculations here to account for those separators
491-
name.rsplit("::").next().unwrap_or(name)
492-
}

‎compiler/rustc_lint/src/levels.rs

Lines changed: 114 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use rustc_ast_pretty::pprust;
2-
use rustc_data_structures::{fx::FxIndexMap, fx::FxIndexSet, sync::Lrc};
2+
use rustc_data_structures::{fx::FxIndexMap, fx::FxIndexSet};
33
use rustc_errors::{Diag, LintDiagnostic, MultiSpan};
44
use rustc_feature::{Features, GateIssue};
55
use rustc_hir::intravisit::{self, Visitor};
@@ -115,34 +115,44 @@ impl LintLevelSets {
115115
}
116116
}
117117

118-
/// Walk the whole crate collecting nodes where lint levels change
119-
/// (e.g. `#[allow]` attributes), and joins that list with the warn-by-default
120-
/// (and not allowed in the crate) and CLI lints. The returned value is a tuple
121-
/// of 1. The lints that will emit (or at least, should run), and 2.
122-
/// The lints that are allowed at the crate level and will not emit.
123-
pub fn lints_that_can_emit(tcx: TyCtxt<'_>, (): ()) -> Lrc<(FxIndexSet<String>, FxIndexSet<String>)> {
124-
let mut visitor = LintLevelMinimum::new(tcx);
125-
visitor.process_opts();
126-
tcx.hir().walk_attributes(&mut visitor);
127-
118+
pub fn lints_that_dont_need_to_run(
119+
tcx: TyCtxt<'_>,
120+
(): (),
121+
) -> FxIndexSet<LintId> {
128122
let store = unerased_lint_store(&tcx.sess);
129123

130-
let lint_groups = store.get_lint_groups();
131-
for group in lint_groups {
132-
let binding = group.0.to_lowercase();
133-
let group_name = name_without_tool(&binding).to_string();
134-
if visitor.lints_that_actually_run.contains(&group_name) {
135-
for lint in group.1 {
136-
visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
137-
}
138-
} else if visitor.lints_allowed.contains(&group_name) {
139-
for lint in &group.1 {
140-
visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
124+
let dont_need_to_run: FxIndexSet<LintId> = store
125+
.get_lints()
126+
.into_iter()
127+
.filter_map(|lint| {
128+
if !lint.loadbearing && lint.default_level(tcx.sess.edition()) == Level::Allow {
129+
Some(LintId::of(lint))
130+
} else {
131+
None
141132
}
142-
}
143-
}
133+
})
134+
.collect();
144135

145-
Lrc::new((visitor.lints_that_actually_run, visitor.lints_allowed))
136+
let mut visitor = LintLevelMaximum { tcx, dont_need_to_run };
137+
visitor.process_opts();
138+
tcx.hir().walk_attributes(&mut visitor);
139+
140+
// let lint_groups = store.get_lint_groups();
141+
// for group in lint_groups {
142+
// let binding = group.0.to_lowercase();
143+
// let group_name = name_without_tool(&binding).to_string();
144+
// if visitor.lints_that_actually_run.contains(&group_name) {
145+
// for lint in group.1 {
146+
// visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
147+
// }
148+
// } else if visitor.lints_allowed.contains(&group_name) {
149+
// for lint in &group.1 {
150+
// visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
151+
// }
152+
// }
153+
// }
154+
155+
visitor.dont_need_to_run
146156
}
147157

148158
#[instrument(level = "trace", skip(tcx), ret)]
@@ -338,83 +348,105 @@ impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, LintLevelQueryMap<'tcx>> {
338348
///
339349
/// E.g., if a crate has a global #![allow(lint)] attribute, but a single item
340350
/// uses #[warn(lint)], this visitor will set that lint level as `Warn`
341-
struct LintLevelMinimum<'tcx> {
351+
struct LintLevelMaximum<'tcx> {
342352
tcx: TyCtxt<'tcx>,
343353
/// The actual list of detected lints.
344-
lints_that_actually_run: FxIndexSet<String>,
345-
lints_allowed: FxIndexSet<String>,
354+
dont_need_to_run: FxIndexSet<LintId>,
346355
}
347356

348-
impl<'tcx> LintLevelMinimum<'tcx> {
349-
pub fn new(tcx: TyCtxt<'tcx>) -> Self {
350-
let mut lints_that_actually_run = FxIndexSet::default();
351-
lints_that_actually_run.reserve(230);
352-
let mut lints_allowed = FxIndexSet::default();
353-
lints_allowed.reserve(100);
354-
Self {
355-
tcx,
356-
// That magic number is the current number of lints + some more for possible future lints
357-
lints_that_actually_run,
358-
lints_allowed,
359-
}
360-
}
361-
357+
impl<'tcx> LintLevelMaximum<'tcx> {
362358
fn process_opts(&mut self) {
363-
for (lint, level) in &self.tcx.sess.opts.lint_opts {
364-
if *level == Level::Allow {
365-
self.lints_allowed.insert(lint.clone());
366-
} else {
367-
self.lints_that_actually_run.insert(lint.to_string());
359+
let store = unerased_lint_store(self.tcx.sess);
360+
for (lint_group, level) in &self.tcx.sess.opts.lint_opts {
361+
if *level != Level::Allow {
362+
let Ok(lints) = store.find_lints(lint_group) else {
363+
return;
364+
};
365+
for lint in lints {
366+
self.dont_need_to_run.swap_remove(&lint);
367+
}
368368
}
369369
}
370370
}
371371
}
372372

373-
impl<'tcx> Visitor<'tcx> for LintLevelMinimum<'tcx> {
373+
impl<'tcx> Visitor<'tcx> for LintLevelMaximum<'tcx> {
374374
type NestedFilter = nested_filter::All;
375375

376376
fn nested_visit_map(&mut self) -> Self::Map {
377377
self.tcx.hir()
378378
}
379379

380380
fn visit_attribute(&mut self, attribute: &'tcx ast::Attribute) {
381-
if let Some(meta) = attribute.meta() {
382-
if [sym::warn, sym::deny, sym::forbid, sym::expect]
383-
.iter()
384-
.any(|kind| meta.has_name(*kind))
385-
{
381+
match Level::from_attr(attribute) {
382+
Some(
383+
Level::Warn
384+
| Level::Deny
385+
| Level::Forbid
386+
| Level::Expect(..)
387+
| Level::ForceWarn(..),
388+
) => {
389+
let store = unerased_lint_store(self.tcx.sess);
390+
let Some(meta) = attribute.meta() else { return };
386391
// SAFETY: Lint attributes are always a metalist inside a
387392
// metalist (even with just one lint).
388-
for meta_list in meta.meta_item_list().unwrap() {
389-
// If it's a tool lint (e.g. clippy::my_clippy_lint)
390-
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
391-
if meta_item.path.segments.len() == 1 {
392-
self.lints_that_actually_run.insert(
393-
// SAFETY: Lint attributes can only have literals
394-
meta_list.ident().unwrap().name.as_str().to_string(),
395-
);
396-
} else {
397-
self.lints_that_actually_run
398-
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
399-
}
393+
let Some(meta_item_list) = meta.meta_item_list() else { return };
394+
395+
for meta_list in meta_item_list {
396+
// Convert Path to String
397+
let Some(meta_item) = meta_list.meta_item() else {return};
398+
let ident: &str = &meta_item.path.segments.iter().map(|segment| segment.ident.as_str()).collect::<Vec<&str>>().join("::");
399+
let Ok(lints) = store.find_lints(
400+
// SAFETY: Lint attributes can only have literals
401+
ident,
402+
) else {
403+
return;
404+
};
405+
for lint in lints {
406+
self.dont_need_to_run.swap_remove(&lint);
400407
}
401-
}
402-
// We handle #![allow]s differently, as these remove checking rather than adding.
403-
} else if meta.has_name(sym::allow)
404-
&& let ast::AttrStyle::Inner = attribute.style
405-
{
406-
for meta_list in meta.meta_item_list().unwrap() {
407-
// If it's a tool lint (e.g. clippy::my_clippy_lint)
408-
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
409-
if meta_item.path.segments.len() == 1 {
410-
self.lints_allowed.insert(meta_list.name_or_empty().as_str().to_string());
411-
} else {
412-
self.lints_allowed
413-
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
414-
}
408+
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
409+
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
410+
// if meta_item.path.segments.len() == 1 {
411+
// let Ok(lints) = store.find_lints(
412+
// // SAFETY: Lint attributes can only have literals
413+
// meta_list.ident().unwrap().name.as_str(),
414+
// ) else {
415+
// return;
416+
// };
417+
// for lint in lints {
418+
// dbg!("LINT REMOVED", &lint);
419+
// self.dont_need_to_run.swap_remove(&lint);
420+
// }
421+
// } else {
422+
// let Ok(lints) = store.find_lints(
423+
// // SAFETY: Lint attributes can only have literals
424+
// meta_item.path.segments[1].ident.name.as_str(),
425+
// ) else {
426+
// return;
427+
// };
428+
// for lint in lints {
429+
// dbg!("LINT REMOVED", &lint);
430+
// self.dont_need_to_run.swap_remove(&lint);
431+
// }
432+
// }
415433
}
416-
}
417-
}
434+
// We handle #![allow]s differently, as these remove checking rather than adding.
435+
} // Some(Level::Allow) if ast::AttrStyle::Inner == attribute.style => {
436+
// for meta_list in meta.meta_item_list().unwrap() {
437+
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
438+
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
439+
// if meta_item.path.segments.len() == 1 {
440+
// self.lints_allowed
441+
// .insert(meta_list.name_or_empty().as_str().to_string());
442+
// } else {
443+
// self.lints_allowed
444+
// .insert(meta_item.path.segments[1].ident.name.as_str().to_string());
445+
// }
446+
// }
447+
// }
448+
// }
449+
_ => { return; }
418450
}
419451
}
420452
}
@@ -1050,7 +1082,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
10501082

10511083
pub(crate) fn provide(providers: &mut Providers) {
10521084
*providers =
1053-
Providers { shallow_lint_levels_on, lints_that_can_emit, ..*providers };
1085+
Providers { shallow_lint_levels_on, lints_that_dont_need_to_run, ..*providers };
10541086
}
10551087

10561088
pub(crate) fn parse_lint_and_tool_name(lint_name: &str) -> (Option<Symbol>, &str) {

‎compiler/rustc_middle/src/query/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use rustc_hir::def_id::{
2828
use rustc_hir::lang_items::{LangItem, LanguageItems};
2929
use rustc_hir::{Crate, ItemLocalId, ItemLocalMap, TraitCandidate};
3030
use rustc_index::IndexVec;
31+
use rustc_lint_defs::LintId;
3132
use rustc_macros::rustc_queries;
3233
use rustc_query_system::ich::StableHashingContext;
3334
use rustc_query_system::query::{try_get_cached, QueryCache, QueryMode, QueryState};
@@ -420,7 +421,7 @@ rustc_queries! {
420421
desc { "computing `#[expect]`ed lints in this crate" }
421422
}
422423

423-
query lints_that_can_emit(_: ()) -> &'tcx Lrc<(FxIndexSet<String>, FxIndexSet<String>)> {
424+
query lints_that_dont_need_to_run(_: ()) -> &'tcx FxIndexSet<LintId> {
424425
arena_cache
425426
desc { "Computing all lints that are explicitly enabled or with a default level greater than Allow" }
426427
}

0 commit comments

Comments
 (0)