about summary refs log tree commit diff
path: root/compiler/rustc_lint
diff options
context:
space:
mode:
authorblyxyas <blyxyas@gmail.com>2024-06-18 22:44:28 +0200
committerblyxyas <blyxyas@gmail.com>2024-10-19 16:20:33 +0200
commit71b4d108c7e71c15c0f61d737ebdc0e1cf559d3c (patch)
tree2554ae8686bc2a11446fe6c34f5ed99311a4f8ac /compiler/rustc_lint
parentedc65776274d14fc7f7f93de66ac3b2d15bbbc37 (diff)
downloadrust-71b4d108c7e71c15c0f61d737ebdc0e1cf559d3c.tar.gz
rust-71b4d108c7e71c15c0f61d737ebdc0e1cf559d3c.zip
Follow review comments (optimize the filtering)
Diffstat (limited to 'compiler/rustc_lint')
-rw-r--r--compiler/rustc_lint/src/builtin.rs7
-rw-r--r--compiler/rustc_lint/src/late.rs64
-rw-r--r--compiler/rustc_lint/src/levels.rs202
-rw-r--r--compiler/rustc_lint/src/lib.rs2
-rw-r--r--compiler/rustc_lint/src/passes.rs13
-rw-r--r--compiler/rustc_lint/src/unused.rs2
6 files changed, 173 insertions, 117 deletions
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
index acd12f7f20d..33c87bbfb70 100644
--- a/compiler/rustc_lint/src/builtin.rs
+++ b/compiler/rustc_lint/src/builtin.rs
@@ -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 { }`.
diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs
index c7187ee1b30..f8bd873cdf5 100644
--- a/compiler/rustc_lint/src/late.rs
+++ b/compiler/rustc_lint/src/late.rs
@@ -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)
-}
diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs
index 527fc117351..f5323c295d0 100644
--- a/compiler/rustc_lint/src/levels.rs
+++ b/compiler/rustc_lint/src/levels.rs
@@ -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);
-    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)]
@@ -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) {
diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
index 5984810961f..11dd17bcd4a 100644
--- a/compiler/rustc_lint/src/lib.rs
+++ b/compiler/rustc_lint/src/lib.rs
@@ -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",
diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs
index 3750f90a044..6dbcdefe08d 100644
--- a/compiler/rustc_lint/src/passes.rs
+++ b/compiler/rustc_lint/src/passes.rs
@@ -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 {
diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs
index a05616bf486..a7ac847c018 100644
--- a/compiler/rustc_lint/src/unused.rs
+++ b/compiler/rustc_lint/src/unused.rs
@@ -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() }
     }
 }