Skip to content

Commit 71b4d10

Browse files
blyxyasblyxyas
committed
Follow review comments (optimize the filtering)
1 parent edc6577 commit 71b4d10

File tree

16 files changed

+276
-151
lines changed

16 files changed

+276
-151
lines changed

‎compiler/rustc_interface/src/passes.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
972972
tcx.ensure().check_mod_privacy(module);
973973
});
974974
});
975-
}
975+
} // { sess.time("mir_checking", || { tcx.hir().mir_for }) }
976976
);
977977

978978
// This check has to be run after all lints are done processing. We don't

‎compiler/rustc_lint/src/builtin.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
//! If you define a new `LateLintPass`, you will also need to add it to the
2121
//! `late_lint_methods!` invocation in `lib.rs`.
2222
23+
use std::default::Default;
2324
use std::fmt::Write;
2425

2526
use ast::token::TokenKind;
@@ -73,12 +74,10 @@ use crate::{
7374
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
7475
fluent_generated as fluent,
7576
};
76-
77-
use std::default::Default;
78-
use std::fmt::Write;
77+
// use std::fmt::Write;
7978

8079
// hardwired lints from rustc_lint_defs
81-
pub use rustc_session::lint::builtin::*;
80+
// pub use rustc_session::lint::builtin::*;
8281

8382
declare_lint! {
8483
/// The `while_true` lint detects `while true { }`.

‎compiler/rustc_lint/src/late.rs

+37-27
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,12 @@ use rustc_hir::def_id::{LocalDefId, LocalModDefId};
2424
use rustc_hir::{HirId, intravisit as hir_visit};
2525
use rustc_middle::hir::nested_filter;
2626
use rustc_middle::ty::{self, TyCtxt};
27-
use rustc_session::Session;
28-
use rustc_session::lint::LintPass;
27+
use rustc_session::{Session, lint::{LintPass, builtin::HardwiredLints}};
2928
use rustc_span::Span;
3029
use tracing::debug;
3130

3231
use crate::passes::LateLintPassObject;
33-
use crate::{LateContext, LateLintPass, LintStore};
32+
use crate::{LateContext, LateLintPass, LintId, LintStore};
3433

3534
/// Extract the [`LintStore`] from [`Session`].
3635
///
@@ -371,29 +370,24 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
371370
} else {
372371
let passes: Vec<_> =
373372
store.late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
374-
375373
// Filter unused lints
376-
let (lints_that_actually_run, lints_allowed) = &**tcx.lints_that_can_emit(());
377-
// let lints_that_actually_run = &lints_that_can_emit.0;
378-
// let lints_allowed = &lints_that_can_emit.1;
379-
380-
// Now, we'll filtered passes in a way that discards any lint that won't trigger.
381-
// If any lint is a given pass is detected to be emitted, we will keep that pass.
382-
// Otherwise, we don't
374+
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
383375
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
384376
.into_iter()
385377
.filter(|pass| {
386-
let pass = LintPass::get_lints(pass);
387-
pass.iter().any(|&lint| {
388-
let lint_name = name_without_tool(&lint.name.to_lowercase()).to_string();
389-
lints_that_actually_run.contains(&lint_name)
390-
|| (!lints_allowed.contains(&lint_name)
391-
&& lint.default_level != crate::Level::Allow)
392-
})
378+
let lints = LintPass::get_lints(pass);
379+
if lints.is_empty() {
380+
true
381+
} else {
382+
lints
383+
.iter()
384+
.any(|lint| !lints_that_dont_need_to_run.contains(&LintId::of(lint)))
385+
}
393386
})
394387
.collect();
395388

396389
filtered_passes.push(Box::new(builtin_lints));
390+
filtered_passes.push(Box::new(HardwiredLints));
397391

398392
let pass = RuntimeCombinedLateLintPass { passes: &mut filtered_passes[..] };
399393
late_lint_mod_inner(tcx, module_def_id, context, pass);
@@ -426,7 +420,7 @@ fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
426420

427421
fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
428422
// Note: `passes` is often empty.
429-
let mut passes: Vec<_> =
423+
let passes: Vec<_> =
430424
unerased_lint_store(tcx.sess).late_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
431425

432426
if passes.is_empty() {
@@ -444,7 +438,30 @@ fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
444438
only_module: false,
445439
};
446440

447-
let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
441+
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
442+
443+
// dbg!(&lints_that_dont_need_to_run);
444+
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
445+
.into_iter()
446+
.filter(|pass| {
447+
let lints = LintPass::get_lints(pass);
448+
!lints.iter().all(|lint| lints_that_dont_need_to_run.contains(&LintId::of(lint)))
449+
})
450+
.collect();
451+
452+
filtered_passes.push(Box::new(HardwiredLints));
453+
454+
// let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
455+
// .into_iter()
456+
// .filter(|pass| {
457+
// let lints = LintPass::get_lints(pass);
458+
// lints.iter()
459+
// .any(|lint|
460+
// !lints_that_dont_need_to_run.contains(&LintId::of(lint)))
461+
// }).collect();
462+
//
463+
464+
let pass = RuntimeCombinedLateLintPass { passes: &mut filtered_passes[..] };
448465
late_lint_crate_inner(tcx, context, pass);
449466
}
450467

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

‎compiler/rustc_lint/src/levels.rs

+119-83
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, FxIndexSet};
33
use rustc_errors::{Diag, LintDiagnostic, MultiSpan};
44
use rustc_feature::{Features, GateIssue};
55
use rustc_hir::HirId;
@@ -31,7 +31,7 @@ use crate::errors::{
3131
OverruledAttributeSub, RequestedLevel, UnknownToolInScopedLint, UnsupportedGroup,
3232
};
3333
use crate::fluent_generated as fluent;
34-
use crate::late::{unerased_lint_store, name_without_tool};
34+
use crate::late::{unerased_lint_store /*name_without_tool*/};
3535
use crate::lints::{
3636
DeprecatedLintName, DeprecatedLintNameFromCommandLine, IgnoredUnlessCrateSpecified,
3737
OverruledAttributeLint, RemovedLint, RemovedLintFromCommandLine, RenamedLint,
@@ -115,34 +115,41 @@ 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+
fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LintId> {
128119
let store = unerased_lint_store(&tcx.sess);
129120

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());
121+
let dont_need_to_run: FxIndexSet<LintId> = store
122+
.get_lints()
123+
.into_iter()
124+
.filter_map(|lint| {
125+
if !lint.loadbearing && lint.default_level(tcx.sess.edition()) == Level::Allow {
126+
Some(LintId::of(lint))
127+
} else {
128+
None
141129
}
142-
}
143-
}
130+
})
131+
.collect();
144132

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

148155
#[instrument(level = "trace", skip(tcx), ret)]
@@ -336,82 +343,112 @@ impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, LintLevelQueryMap<'tcx>> {
336343
///
337344
/// E.g., if a crate has a global #![allow(lint)] attribute, but a single item
338345
/// uses #[warn(lint)], this visitor will set that lint level as `Warn`
339-
struct LintLevelMinimum<'tcx> {
346+
struct LintLevelMaximum<'tcx> {
340347
tcx: TyCtxt<'tcx>,
341348
/// The actual list of detected lints.
342-
lints_that_actually_run: FxIndexSet<String>,
343-
lints_allowed: FxIndexSet<String>,
349+
dont_need_to_run: FxIndexSet<LintId>,
344350
}
345351

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

371-
impl<'tcx> Visitor<'tcx> for LintLevelMinimum<'tcx> {
368+
impl<'tcx> Visitor<'tcx> for LintLevelMaximum<'tcx> {
372369
type NestedFilter = nested_filter::All;
373370

374371
fn nested_visit_map(&mut self) -> Self::Map {
375372
self.tcx.hir()
376373
}
377374

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

10491086
pub(crate) fn provide(providers: &mut Providers) {
1050-
*providers =
1051-
Providers { shallow_lint_levels_on, lints_that_can_emit, ..*providers };
1087+
*providers = Providers { shallow_lint_levels_on, lints_that_dont_need_to_run, ..*providers };
10521088
}
10531089

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

0 commit comments

Comments
 (0)