Skip to content

Commit

Permalink
Follow review comments (optimize the filtering)
Browse files Browse the repository at this point in the history
  • Loading branch information
@blyxyas
blyxyas committed Oct 19, 2024
1 parent edc6577 commit 71b4d10
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 151 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
tcx.ensure().check_mod_privacy(module);
});
});
}
} // { sess.time("mir_checking", || { tcx.hir().mir_for }) }
);

// This check has to be run after all lints are done processing. We don't
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_lint/src/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
//! If you define a new `LateLintPass`, you will also need to add it to the
//! `late_lint_methods!` invocation in `lib.rs`.
use std::default::Default;
use std::fmt::Write;

use ast::token::TokenKind;
Expand Down Expand Up @@ -73,12 +74,10 @@ use crate::{
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
fluent_generated as fluent,
};

use std::default::Default;
use std::fmt::Write;
// use std::fmt::Write;

// hardwired lints from rustc_lint_defs
pub use rustc_session::lint::builtin::*;
// pub use rustc_session::lint::builtin::*;

declare_lint! {
/// The `while_true` lint detects `while true { }`.
Expand Down
64 changes: 37 additions & 27 deletions compiler/rustc_lint/src/late.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@ use rustc_hir::def_id::{LocalDefId, LocalModDefId};
use rustc_hir::{HirId, intravisit as hir_visit};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::Session;
use rustc_session::lint::LintPass;
use rustc_session::{Session, lint::{LintPass, builtin::HardwiredLints}};
use rustc_span::Span;
use tracing::debug;

use crate::passes::LateLintPassObject;
use crate::{LateContext, LateLintPass, LintStore};
use crate::{LateContext, LateLintPass, LintId, LintStore};

/// Extract the [`LintStore`] from [`Session`].
///
Expand Down Expand Up @@ -371,29 +370,24 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
} else {
let passes: Vec<_> =
store.late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();

// Filter unused lints
let (lints_that_actually_run, lints_allowed) = &**tcx.lints_that_can_emit(());
// let lints_that_actually_run = &lints_that_can_emit.0;
// let lints_allowed = &lints_that_can_emit.1;

// Now, we'll filtered passes in a way that discards any lint that won't trigger.
// If any lint is a given pass is detected to be emitted, we will keep that pass.
// Otherwise, we don't
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
.into_iter()
.filter(|pass| {
let pass = LintPass::get_lints(pass);
pass.iter().any(|&lint| {
let lint_name = name_without_tool(&lint.name.to_lowercase()).to_string();
lints_that_actually_run.contains(&lint_name)
|| (!lints_allowed.contains(&lint_name)
&& lint.default_level != crate::Level::Allow)
})
let lints = LintPass::get_lints(pass);
if lints.is_empty() {
true
} else {
lints
.iter()
.any(|lint| !lints_that_dont_need_to_run.contains(&LintId::of(lint)))
}
})
.collect();

filtered_passes.push(Box::new(builtin_lints));
filtered_passes.push(Box::new(HardwiredLints));

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

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

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

let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());

// dbg!(&lints_that_dont_need_to_run);
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
.into_iter()
.filter(|pass| {
let lints = LintPass::get_lints(pass);
!lints.iter().all(|lint| lints_that_dont_need_to_run.contains(&LintId::of(lint)))
})
.collect();

filtered_passes.push(Box::new(HardwiredLints));

// let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
// .into_iter()
// .filter(|pass| {
// let lints = LintPass::get_lints(pass);
// lints.iter()
// .any(|lint|
// !lints_that_dont_need_to_run.contains(&LintId::of(lint)))
// }).collect();
//

let pass = RuntimeCombinedLateLintPass { passes: &mut filtered_passes[..] };
late_lint_crate_inner(tcx, context, pass);
}

Expand Down Expand Up @@ -482,10 +499,3 @@ pub fn check_crate<'tcx>(tcx: TyCtxt<'tcx>) {
},
);
}

/// Format name ignoring the name, useful for filtering non-used lints.
/// For example, 'clippy::my_lint' will turn into 'my_lint'
pub(crate) fn name_without_tool(name: &str) -> &str {
// Doing some calculations here to account for those separators
name.rsplit("::").next().unwrap_or(name)
}
202 changes: 119 additions & 83 deletions compiler/rustc_lint/src/levels.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_ast_pretty::pprust;
use rustc_data_structures::{fx::FxIndexMap, fx::FxIndexSet, sync::Lrc};
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_errors::{Diag, LintDiagnostic, MultiSpan};
use rustc_feature::{Features, GateIssue};
use rustc_hir::HirId;
Expand Down Expand Up @@ -31,7 +31,7 @@ use crate::errors::{
OverruledAttributeSub, RequestedLevel, UnknownToolInScopedLint, UnsupportedGroup,
};
use crate::fluent_generated as fluent;
use crate::late::{unerased_lint_store, name_without_tool};
use crate::late::{unerased_lint_store /*name_without_tool*/};
use crate::lints::{
DeprecatedLintName, DeprecatedLintNameFromCommandLine, IgnoredUnlessCrateSpecified,
OverruledAttributeLint, RemovedLint, RemovedLintFromCommandLine, RenamedLint,
Expand Down Expand Up @@ -115,34 +115,41 @@ impl LintLevelSets {
}
}

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

fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LintId> {
let store = unerased_lint_store(&tcx.sess);

let lint_groups = store.get_lint_groups();
for group in lint_groups {
let binding = group.0.to_lowercase();
let group_name = name_without_tool(&binding).to_string();
if visitor.lints_that_actually_run.contains(&group_name) {
for lint in group.1 {
visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
}
} else if visitor.lints_allowed.contains(&group_name) {
for lint in &group.1 {
visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
let dont_need_to_run: FxIndexSet<LintId> = store
.get_lints()
.into_iter()
.filter_map(|lint| {
if !lint.loadbearing && lint.default_level(tcx.sess.edition()) == Level::Allow {
Some(LintId::of(lint))
} else {
None
}
}
}
})
.collect();

Lrc::new((visitor.lints_that_actually_run, visitor.lints_allowed))
let mut visitor = LintLevelMaximum { tcx, dont_need_to_run };
visitor.process_opts();
tcx.hir().walk_attributes(&mut visitor);

// let lint_groups = store.get_lint_groups();
// for group in lint_groups {
// let binding = group.0.to_lowercase();
// let group_name = name_without_tool(&binding).to_string();
// if visitor.lints_that_actually_run.contains(&group_name) {
// for lint in group.1 {
// visitor.lints_that_actually_run.insert(name_without_tool(&lint.to_string()).to_string());
// }
// } else if visitor.lints_allowed.contains(&group_name) {
// for lint in &group.1 {
// visitor.lints_allowed.insert(name_without_tool(&lint.to_string()).to_string());
// }
// }
// }

visitor.dont_need_to_run
}

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

impl<'tcx> LintLevelMinimum<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>) -> Self {
let mut lints_that_actually_run = FxIndexSet::default();
lints_that_actually_run.reserve(230);
let mut lints_allowed = FxIndexSet::default();
lints_allowed.reserve(100);
Self {
tcx,
// That magic number is the current number of lints + some more for possible future lints
lints_that_actually_run,
lints_allowed,
}
}

impl<'tcx> LintLevelMaximum<'tcx> {
fn process_opts(&mut self) {
for (lint, level) in &self.tcx.sess.opts.lint_opts {
if *level == Level::Allow {
self.lints_allowed.insert(lint.clone());
} else {
self.lints_that_actually_run.insert(lint.to_string());
let store = unerased_lint_store(self.tcx.sess);
for (lint_group, level) in &self.tcx.sess.opts.lint_opts {
if *level != Level::Allow {
let Ok(lints) = store.find_lints(lint_group) else {
return;
};
for lint in lints {
self.dont_need_to_run.swap_remove(&lint);
}
}
}
}
}

impl<'tcx> Visitor<'tcx> for LintLevelMinimum<'tcx> {
impl<'tcx> Visitor<'tcx> for LintLevelMaximum<'tcx> {
type NestedFilter = nested_filter::All;

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

fn visit_attribute(&mut self, attribute: &'tcx ast::Attribute) {
if let Some(meta) = attribute.meta() {
if [sym::warn, sym::deny, sym::forbid, sym::expect]
.iter()
.any(|kind| meta.has_name(*kind))
{
match Level::from_attr(attribute) {
Some(
Level::Warn
| Level::Deny
| Level::Forbid
| Level::Expect(..)
| Level::ForceWarn(..),
) => {
let store = unerased_lint_store(self.tcx.sess);
let Some(meta) = attribute.meta() else { return };
// SAFETY: Lint attributes are always a metalist inside a
// metalist (even with just one lint).
for meta_list in meta.meta_item_list().unwrap() {
// If it's a tool lint (e.g. clippy::my_clippy_lint)
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
if meta_item.path.segments.len() == 1 {
self.lints_that_actually_run.insert(
// SAFETY: Lint attributes can only have literals
meta_list.ident().unwrap().name.as_str().to_string(),
);
} else {
self.lints_that_actually_run
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
}
}
}
// We handle #![allow]s differently, as these remove checking rather than adding.
} else if meta.has_name(sym::allow)
&& let ast::AttrStyle::Inner = attribute.style
{
for meta_list in meta.meta_item_list().unwrap() {
// If it's a tool lint (e.g. clippy::my_clippy_lint)
if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
if meta_item.path.segments.len() == 1 {
self.lints_allowed.insert(meta_list.name_or_empty().as_str().to_string());
} else {
self.lints_allowed
.insert(meta_item.path.segments[1].ident.name.as_str().to_string());
}
let Some(meta_item_list) = meta.meta_item_list() else { return };

for meta_list in meta_item_list {
// Convert Path to String
let Some(meta_item) = meta_list.meta_item() else { return };
let ident: &str = &meta_item
.path
.segments
.iter()
.map(|segment| segment.ident.as_str())
.collect::<Vec<&str>>()
.join("::");
let Ok(lints) = store.find_lints(
// SAFETY: Lint attributes can only have literals
ident,
) else {
return;
};
for lint in lints {
self.dont_need_to_run.swap_remove(&lint);
}
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
// if meta_item.path.segments.len() == 1 {
// let Ok(lints) = store.find_lints(
// // SAFETY: Lint attributes can only have literals
// meta_list.ident().unwrap().name.as_str(),
// ) else {
// return;
// };
// for lint in lints {
// dbg!("LINT REMOVED", &lint);
// self.dont_need_to_run.swap_remove(&lint);
// }
// } else {
// let Ok(lints) = store.find_lints(
// // SAFETY: Lint attributes can only have literals
// meta_item.path.segments[1].ident.name.as_str(),
// ) else {
// return;
// };
// for lint in lints {
// dbg!("LINT REMOVED", &lint);
// self.dont_need_to_run.swap_remove(&lint);
// }
// }
}
// We handle #![allow]s differently, as these remove checking rather than adding.
} // Some(Level::Allow) if ast::AttrStyle::Inner == attribute.style => {
// for meta_list in meta.meta_item_list().unwrap() {
// // If it's a tool lint (e.g. clippy::my_clippy_lint)
// if let ast::NestedMetaItem::MetaItem(meta_item) = meta_list {
// if meta_item.path.segments.len() == 1 {
// self.lints_allowed
// .insert(meta_list.name_or_empty().as_str().to_string());
// } else {
// self.lints_allowed
// .insert(meta_item.path.segments[1].ident.name.as_str().to_string());
// }
// }
// }
// }
_ => {
return;
}
}
}
Expand Down Expand Up @@ -1047,8 +1084,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}

pub(crate) fn provide(providers: &mut Providers) {
*providers =
Providers { shallow_lint_levels_on, lints_that_can_emit, ..*providers };
*providers = Providers { shallow_lint_levels_on, lints_that_dont_need_to_run, ..*providers };
}

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

0 comments on commit 71b4d10

Please sign in to comment.