1
Fork 0

Follow review comments (optimize the filtering)

This commit is contained in:
blyxyas 2024-06-18 22:44:28 +02:00
parent edc6577627
commit 71b4d108c7
16 changed files with 276 additions and 151 deletions

View file

@ -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;
@ -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 { }`.

View file

@ -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`].
///
@ -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);
@ -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() {
@ -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);
}
@ -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)
}

View file

@ -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;
@ -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,
@ -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);
fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LintId> {
let store = unerased_lint_store(&tcx.sess);
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();
let mut visitor = LintLevelMaximum { tcx, dont_need_to_run };
visitor.process_opts();
tcx.hir().walk_attributes(&mut visitor);
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 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());
}
}
}
Lrc::new((visitor.lints_that_actually_run, visitor.lints_allowed))
visitor.dont_need_to_run
}
#[instrument(level = "trace", skip(tcx), ret)]
@ -336,39 +343,29 @@ 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 {
@ -376,42 +373,82 @@ impl<'tcx> Visitor<'tcx> for LintLevelMinimum<'tcx> {
}
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;
}
}
}
@ -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) {

View file

@ -199,7 +199,6 @@ late_lint_methods!(
ForLoopsOverFallibles: ForLoopsOverFallibles,
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
DropForgetUseless: DropForgetUseless,
HardwiredLints: HardwiredLints,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
InvalidFromUtf8: InvalidFromUtf8,
@ -280,6 +279,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&BuiltinCombinedEarlyLintPass::get_lints());
store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints());
store.register_lints(&foreign_modules::get_lints());
store.register_lints(&HardwiredLints::default().get_lints());
add_lint_group!(
"nonstandard_style",

View file

@ -70,7 +70,18 @@ macro_rules! declare_late_lint_pass {
// for all the `check_*` methods.
late_lint_methods!(declare_late_lint_pass, []);
impl LateLintPass<'_> for HardwiredLints {}
impl LateLintPass<'_> for HardwiredLints {
fn check_fn(
&mut self,
_: &LateContext<'_>,
_: rustc_hir::intravisit::FnKind<'_>,
_: &'_ rustc_hir::FnDecl<'_>,
_: &'_ rustc_hir::Body<'_>,
_: rustc_span::Span,
_: rustc_span::def_id::LocalDefId,
) {
}
}
#[macro_export]
macro_rules! expand_combined_late_lint_pass_method {

View file

@ -1026,7 +1026,7 @@ pub(crate) struct UnusedParens {
}
impl Default for UnusedParens {
fpub(crate) fn default() -> Self {
fn default() -> Self {
Self { with_self_ty_parens: false, parens_in_cast_in_lt: Vec::new() }
}
}