Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 7267c62

Browse files
blyxyasblyxyas
committed
Follow review comments (optimize the filtering)
1 parent a6f9715 commit 7267c62

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};
@@ -143,34 +143,44 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp
143143
builder.provider.expectations
144144
}
145145

146-
/// Walk the whole crate collecting nodes where lint levels change
147-
/// (e.g. `#[allow]` attributes), and joins that list with the warn-by-default
148-
/// (and not allowed in the crate) and CLI lints. The returned value is a tuple
149-
/// of 1. The lints that will emit (or at least, should run), and 2.
150-
/// The lints that are allowed at the crate level and will not emit.
151-
pub fn lints_that_can_emit(tcx: TyCtxt<'_>, (): ()) -> Lrc<(FxIndexSet<String>, FxIndexSet<String>)> {
152-
let mut visitor = LintLevelMinimum::new(tcx);
153-
visitor.process_opts();
154-
tcx.hir().walk_attributes(&mut visitor);
155-
146+
pub fn lints_that_dont_need_to_run(
147+
tcx: TyCtxt<'_>,
148+
(): (),
149+
) -> FxIndexSet<LintId> {
156150
let store = unerased_lint_store(&tcx.sess);
157151

158-
let lint_groups = store.get_lint_groups();
159-
for group in lint_groups {
160-
let binding = group.0.to_lowercase();
161-
let group_name = name_without_tool(&binding).to_string();
162-
if visitor.lints_that_actually_run.contains(&group_name) {
163-
for lint in group.1 {
164-
visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
165-
}
166-
} else if visitor.lints_allowed.contains(&group_name) {
167-
for lint in &group.1 {
168-
visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
152+
let dont_need_to_run: FxIndexSet<LintId> = store
153+
.get_lints()
154+
.into_iter()
155+
.filter_map(|lint| {
156+
if !lint.loadbearing && lint.default_level(tcx.sess.edition()) == Level::Allow {
157+
Some(LintId::of(lint))
158+
} else {
159+
None
169160
}
170-
}
171-
}
161+
})
162+
.collect();
172163

173-
Lrc::new((visitor.lints_that_actually_run, visitor.lints_allowed))
164+
let mut visitor = LintLevelMaximum { tcx, dont_need_to_run };
165+
visitor.process_opts();
166+
tcx.hir().walk_attributes(&mut visitor);
167+
168+
// let lint_groups = store.get_lint_groups();
169+
// for group in lint_groups {
170+
// let binding = group.0.to_lowercase();
171+
// let group_name = name_without_tool(&binding).to_string();
172+
// if visitor.lints_that_actually_run.contains(&group_name) {
173+
// for lint in group.1 {
174+
// visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
175+
// }
176+
// } else if visitor.lints_allowed.contains(&group_name) {
177+
// for lint in &group.1 {
178+
// visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
179+
// }
180+
// }
181+
// }
182+
183+
visitor.dont_need_to_run
174184
}
175185

176186
#[instrument(level = "trace", skip(tcx), ret)]
@@ -477,83 +487,105 @@ impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'
477487
///
478488
/// E.g., if a crate has a global #![allow(lint)] attribute, but a single item
479489
/// uses #[warn(lint)], this visitor will set that lint level as `Warn`
480-
struct LintLevelMinimum<'tcx> {
490+
struct LintLevelMaximum<'tcx> {
481491
tcx: TyCtxt<'tcx>,
482492
/// The actual list of detected lints.
483-
lints_that_actually_run: FxIndexSet<String>,
484-
lints_allowed: FxIndexSet<String>,
493+
dont_need_to_run: FxIndexSet<LintId>,
485494
}
486495

487-
impl<'tcx> LintLevelMinimum<'tcx> {
488-
pub fn new(tcx: TyCtxt<'tcx>) -> Self {
489-
let mut lints_that_actually_run = FxIndexSet::default();
490-
lints_that_actually_run.reserve(230);
491-
let mut lints_allowed = FxIndexSet::default();
492-
lints_allowed.reserve(100);
493-
Self {
494-
tcx,
495-
// That magic number is the current number of lints + some more for possible future lints
496-
lints_that_actually_run,
497-
lints_allowed,
498-
}
499-
}
500-
496+
impl<'tcx> LintLevelMaximum<'tcx> {
501497
fn process_opts(&mut self) {
502-
for (lint, level) in &self.tcx.sess.opts.lint_opts {
503-
if *level == Level::Allow {
504-
self.lints_allowed.insert(lint.clone());
505-
} else {
506-
self.lints_that_actually_run.insert(lint.to_string());
498+
let store = unerased_lint_store(self.tcx.sess);
499+
for (lint_group, level) in &self.tcx.sess.opts.lint_opts {
500+
if *level != Level::Allow {
501+
let Ok(lints) = store.find_lints(lint_group) else {
502+
return;
503+
};
504+
for lint in lints {
505+
self.dont_need_to_run.swap_remove(&lint);
506+
}
507507
}
508508
}
509509
}
510510
}
511511

512-
impl<'tcx> Visitor<'tcx> for LintLevelMinimum<'tcx> {
512+
impl<'tcx> Visitor<'tcx> for LintLevelMaximum<'tcx> {
513513
type NestedFilter = nested_filter::All;
514514

515515
fn nested_visit_map(&mut self) -> Self::Map {
516516
self.tcx.hir()
517517
}
518518

519519
fn visit_attribute(&mut self, attribute: &'tcx ast::Attribute) {
520-
if let Some(meta) = attribute.meta() {
521-
if [sym::warn, sym::deny, sym::forbid, sym::expect]
522-
.iter()
523-
.any(|kind| meta.has_name(*kind))
524-
{
520+
match Level::from_attr(attribute) {
521+
Some(
522+
Level::Warn
523+
| Level::Deny
524+
| Level::Forbid
525+
| Level::Expect(..)
526+
| Level::ForceWarn(..),
527+
) => {
528+
let store = unerased_lint_store(self.tcx.sess);
529+
let Some(meta) = attribute.meta() else { return };
525530
// SAFETY: Lint attributes are always a metalist inside a
526531
// metalist (even with just one lint).
527-
for meta_list in meta.meta_item_list().unwrap() {
528-
// If it's a tool lint (e.g. clippy::my_clippy_lint)
529-
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
530-
if meta_item.path.segments.len() == 1 {
531-
self.lints_that_actually_run.insert(
532-
// SAFETY: Lint attributes can only have literals
533-
meta_list.ident().unwrap().name.as_str().to_string(),
534-
);
535-
} else {
536-
self.lints_that_actually_run
537-
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
538-
}
532+
let Some(meta_item_list) = meta.meta_item_list() else { return };
533+
534+
for meta_list in meta_item_list {
535+
// Convert Path to String
536+
let Some(meta_item) = meta_list.meta_item() else {return};
537+
let ident: &str = &meta_item.path.segments.iter().map(|segment| segment.ident.as_str()).collect::<Vec<&str>>().join("::");
538+
let Ok(lints) = store.find_lints(
539+
// SAFETY: Lint attributes can only have literals
540+
ident,
541+
) else {
542+
return;
543+
};
544+
for lint in lints {
545+
self.dont_need_to_run.swap_remove(&lint);
539546
}
540-
}
541-
// We handle #![allow]s differently, as these remove checking rather than adding.
542-
} else if meta.has_name(sym::allow)
543-
&& let ast::AttrStyle::Inner = attribute.style
544-
{
545-
for meta_list in meta.meta_item_list().unwrap() {
546-
// If it's a tool lint (e.g. clippy::my_clippy_lint)
547-
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
548-
if meta_item.path.segments.len() == 1 {
549-
self.lints_allowed.insert(meta_list.name_or_empty().as_str().to_string());
550-
} else {
551-
self.lints_allowed
552-
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
553-
}
547+
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
548+
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
549+
// if meta_item.path.segments.len() == 1 {
550+
// let Ok(lints) = store.find_lints(
551+
// // SAFETY: Lint attributes can only have literals
552+
// meta_list.ident().unwrap().name.as_str(),
553+
// ) else {
554+
// return;
555+
// };
556+
// for lint in lints {
557+
// dbg!("LINT REMOVED", &lint);
558+
// self.dont_need_to_run.swap_remove(&lint);
559+
// }
560+
// } else {
561+
// let Ok(lints) = store.find_lints(
562+
// // SAFETY: Lint attributes can only have literals
563+
// meta_item.path.segments[1].ident.name.as_str(),
564+
// ) else {
565+
// return;
566+
// };
567+
// for lint in lints {
568+
// dbg!("LINT REMOVED", &lint);
569+
// self.dont_need_to_run.swap_remove(&lint);
570+
// }
571+
// }
554572
}
555-
}
556-
}
573+
// We handle #![allow]s differently, as these remove checking rather than adding.
574+
} // Some(Level::Allow) if ast::AttrStyle::Inner == attribute.style => {
575+
// for meta_list in meta.meta_item_list().unwrap() {
576+
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
577+
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
578+
// if meta_item.path.segments.len() == 1 {
579+
// self.lints_allowed
580+
// .insert(meta_list.name_or_empty().as_str().to_string());
581+
// } else {
582+
// self.lints_allowed
583+
// .insert(meta_item.path.segments[1].ident.name.as_str().to_string());
584+
// }
585+
// }
586+
// }
587+
// }
588+
_ => { return; }
557589
}
558590
}
559591
}
@@ -1217,7 +1249,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
12171249

12181250
pub(crate) fn provide(providers: &mut Providers) {
12191251
*providers =
1220-
Providers { shallow_lint_levels_on, lint_expectations, lints_that_can_emit, ..*providers };
1252+
Providers { shallow_lint_levels_on, lint_expectations, lints_that_dont_need_to_run, ..*providers };
12211253
}
12221254

12231255
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)