Skip to content

Commit cc1d40f

Browse files
committed
Auto merge of #125116 - blyxyas:ignore-allowed-lints-final, r=<try>
(Big performance change) Do not run lints that cannot emit Before this lint, adding a lint was a difficult matter because it always had some overhead involved. This was because all lints would run, no matter their default level, or if the user had `#![allow]`ed them. This PR changes that. This change would improve both the Rust lint infrastructure and Clippy, but Clippy will see the most benefit, as it has about 900 registered lints (and growing!) So yeah, with this little patch we filter all lints pre-linting, and remove any lint that is either: - Manually `#![allow]`ed in the whole crate, - Allowed in the command line, or - Not manually enabled with `#[warn]` or similar, and its default level is `Allow` I open this PR to receive some feedback, mainly related to performance. We have lots of `Lock`s, `with_lock` and similar functions (also lots of cloning), so the filtering performance is not the best. In an older iteration, instead of doing this in the parsing phase, we developed a visitor with the same function but without so many locks, would reverting to that change help? I'm not sure tbh.
2 parents c45e831 + 7606f89 commit cc1d40f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+247
-49
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -4342,6 +4342,7 @@ dependencies = [
43424342
"rustc_hir",
43434343
"rustc_hir_pretty",
43444344
"rustc_index",
4345+
"rustc_lint_defs",
43454346
"rustc_macros",
43464347
"rustc_query_system",
43474348
"rustc_serialize",

compiler/rustc_ast/src/attr/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ impl AttrItem {
210210
self.args.span().map_or(self.path.span, |args_span| self.path.span.to(args_span))
211211
}
212212

213-
fn meta_item_list(&self) -> Option<ThinVec<NestedMetaItem>> {
213+
pub fn meta_item_list(&self) -> Option<ThinVec<NestedMetaItem>> {
214214
match &self.args {
215215
AttrArgs::Delimited(args) if args.delim == Delimiter::Parenthesis => {
216216
MetaItemKind::list_from_tokens(args.tokens.clone())

compiler/rustc_lint/src/builtin.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy};
7373

7474
use crate::nonstandard_style::{method_context, MethodLateContext};
7575

76+
use std::default::Default;
7677
use std::fmt::Write;
7778

7879
// hardwired lints from rustc_lint_defs
@@ -462,6 +463,7 @@ declare_lint! {
462463
report_in_external_macro
463464
}
464465

466+
#[derive(Default)]
465467
pub struct MissingDoc;
466468

467469
impl_lint_pass!(MissingDoc => [MISSING_DOCS]);
@@ -903,8 +905,8 @@ pub struct DeprecatedAttr {
903905

904906
impl_lint_pass!(DeprecatedAttr => []);
905907

906-
impl DeprecatedAttr {
907-
pub fn new() -> DeprecatedAttr {
908+
impl Default for DeprecatedAttr {
909+
fn default() -> Self {
908910
DeprecatedAttr { depr_attrs: deprecated_attributes() }
909911
}
910912
}

compiler/rustc_lint/src/early.rs

+3
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,9 @@ impl LintPass for RuntimeCombinedEarlyLintPass<'_> {
316316
fn name(&self) -> &'static str {
317317
panic!()
318318
}
319+
fn get_lints(&self) -> crate::LintVec {
320+
panic!()
321+
}
319322
}
320323

321324
macro_rules! impl_early_lint_pass {

compiler/rustc_lint/src/late.rs

+36-5
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,9 @@ impl LintPass for RuntimeCombinedLateLintPass<'_, '_> {
325325
fn name(&self) -> &'static str {
326326
panic!()
327327
}
328+
fn get_lints(&self) -> crate::LintVec {
329+
panic!()
330+
}
328331
}
329332

330333
macro_rules! impl_late_lint_pass {
@@ -360,13 +363,41 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
360363
// Note: `passes` is often empty. In that case, it's faster to run
361364
// `builtin_lints` directly rather than bundling it up into the
362365
// `RuntimeCombinedLateLintPass`.
363-
let late_module_passes = &unerased_lint_store(tcx.sess).late_module_passes;
364-
if late_module_passes.is_empty() {
366+
let store = unerased_lint_store(tcx.sess);
367+
368+
if store.late_module_passes.is_empty() {
365369
late_lint_mod_inner(tcx, module_def_id, context, builtin_lints);
366370
} else {
367-
let mut passes: Vec<_> = late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
368-
passes.push(Box::new(builtin_lints));
369-
let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
371+
let passes: Vec<_> =
372+
store.late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
373+
374+
// Filter unused lints
375+
let (lints_to_emit, lints_allowed) = &**tcx.lints_that_can_emit(());
376+
// let lints_to_emit = &lints_that_can_emit.0;
377+
// let lints_allowed = &lints_that_can_emit.1;
378+
379+
// Now, we'll filtered passes in a way that discards any lint that won't trigger.
380+
// If any lint is a given pass is detected to be emitted, we will keep that pass.
381+
// Otherwise, we don't
382+
let mut filtered_passes: Vec<Box<dyn LateLintPass<'tcx>>> = passes
383+
.into_iter()
384+
.filter(|pass| {
385+
let pass = LintPass::get_lints(pass);
386+
pass.iter().any(|&lint| {
387+
let lint_name = &lint.name.to_lowercase()
388+
// Doing some calculations here to account for those separators
389+
[lint.name.find("::").unwrap_or(lint.name.len() - 2) + 2..]
390+
.to_string();
391+
lints_to_emit.contains(&lint_name)
392+
|| (!lints_allowed.contains(&lint_name)
393+
&& lint.default_level != crate::Level::Allow)
394+
})
395+
})
396+
.collect();
397+
398+
filtered_passes.push(Box::new(builtin_lints));
399+
400+
let pass = RuntimeCombinedLateLintPass { passes: &mut filtered_passes[..] };
370401
late_lint_mod_inner(tcx, module_def_id, context, pass);
371402
}
372403
}

compiler/rustc_lint/src/levels.rs

+74-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ use crate::{
1515
};
1616
use rustc_ast as ast;
1717
use rustc_ast_pretty::pprust;
18-
use rustc_data_structures::fx::FxIndexMap;
18+
use rustc_data_structures::{
19+
fx::FxIndexMap,
20+
sync::{join, Lock, Lrc},
21+
};
1922
use rustc_errors::{Diag, DiagMessage, LintDiagnostic, MultiSpan};
2023
use rustc_feature::{Features, GateIssue};
2124
use rustc_hir as hir;
@@ -151,6 +154,19 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp
151154
builder.provider.expectations
152155
}
153156

157+
/// Walk the whole crate collecting nodes where lint levels change
158+
/// (e.g. `#[allow]` attributes), and joins that list with the warn-by-default
159+
/// (and not allowed in the crate) and CLI lints. The returned value is a tuple
160+
/// of 1. The lints that will emit (or at least, should run), and 2.
161+
/// The lints that are allowed at the crate level and will not emit.
162+
pub fn lints_that_can_emit(tcx: TyCtxt<'_>, (): ()) -> Lrc<(Vec<String>, Vec<String>)> {
163+
let mut visitor = LintLevelMinimum::new(tcx);
164+
visitor.process_opts();
165+
visitor.lint_level_minimums();
166+
167+
Lrc::new((visitor.lints_to_emit.into_inner(), visitor.lints_allowed.into_inner()))
168+
}
169+
154170
#[instrument(level = "trace", skip(tcx), ret)]
155171
fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLevelMap {
156172
let store = unerased_lint_store(tcx.sess);
@@ -253,7 +269,7 @@ impl LintLevelsProvider for LintLevelQueryMap<'_> {
253269
}
254270
}
255271

256-
struct QueryMapExpectationsWrapper<'tcx> {
272+
pub(crate) struct QueryMapExpectationsWrapper<'tcx> {
257273
tcx: TyCtxt<'tcx>,
258274
/// HirId of the currently investigated element.
259275
cur: HirId,
@@ -450,6 +466,60 @@ impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'
450466
}
451467
}
452468

469+
/// Visitor with the only function of visiting every item-like in a crate and
470+
/// computing the highest level that every lint gets put to.
471+
///
472+
/// E.g., if a crate has a global #![allow(lint)] attribute, but a single item
473+
/// uses #[warn(lint)], this visitor will set that lint level as `Warn`
474+
struct LintLevelMinimum<'tcx> {
475+
tcx: TyCtxt<'tcx>,
476+
/// The actual list of detected lints.
477+
lints_to_emit: Lock<Vec<String>>,
478+
lints_allowed: Lock<Vec<String>>,
479+
}
480+
481+
impl<'tcx> LintLevelMinimum<'tcx> {
482+
pub fn new(tcx: TyCtxt<'tcx>) -> Self {
483+
Self {
484+
tcx,
485+
// That magic number is the current number of lints + some more for possible future lints
486+
lints_to_emit: Lock::new(Vec::with_capacity(230)),
487+
lints_allowed: Lock::new(Vec::with_capacity(100)),
488+
}
489+
}
490+
491+
fn process_opts(&mut self) {
492+
for (lint, level) in &self.tcx.sess.opts.lint_opts {
493+
if *level == Level::Allow {
494+
self.lints_allowed.with_lock(|lints_allowed| lints_allowed.push(lint.to_string()));
495+
} else {
496+
self.lints_to_emit.with_lock(|lints_to_emit| lints_to_emit.push(lint.to_string()));
497+
}
498+
}
499+
}
500+
501+
fn lint_level_minimums(&mut self) {
502+
join(
503+
|| {
504+
self.tcx.sess.psess.lints_that_can_emit.with_lock(|vec| {
505+
for lint_symbol in vec {
506+
self.lints_to_emit
507+
.with_lock(|lints_to_emit| lints_to_emit.push(lint_symbol.to_string()));
508+
}
509+
});
510+
},
511+
|| {
512+
self.tcx.sess.psess.lints_allowed.with_lock(|vec| {
513+
for lint_symbol in vec {
514+
self.lints_allowed
515+
.with_lock(|lints_allowed| lints_allowed.push(lint_symbol.to_string()));
516+
}
517+
});
518+
},
519+
);
520+
}
521+
}
522+
453523
pub struct LintLevelsBuilder<'s, P> {
454524
sess: &'s Session,
455525
features: &'s Features,
@@ -1133,7 +1203,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
11331203
}
11341204

11351205
pub(crate) fn provide(providers: &mut Providers) {
1136-
*providers = Providers { shallow_lint_levels_on, lint_expectations, ..*providers };
1206+
*providers =
1207+
Providers { shallow_lint_levels_on, lint_expectations, lints_that_can_emit, ..*providers };
11371208
}
11381209

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

compiler/rustc_lint/src/lib.rs

+12-11
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#![feature(rustdoc_internals)]
3131
#![feature(array_windows)]
3232
#![feature(box_patterns)]
33+
#![feature(box_into_inner)]
3334
#![feature(control_flow_enum)]
3435
#![feature(extract_if)]
3536
#![feature(if_let_guard)]
@@ -156,15 +157,15 @@ early_lint_methods!(
156157
[
157158
pub BuiltinCombinedEarlyLintPass,
158159
[
159-
UnusedParens: UnusedParens::new(),
160+
UnusedParens: UnusedParens::default(),
160161
UnusedBraces: UnusedBraces,
161162
UnusedImportBraces: UnusedImportBraces,
162163
UnsafeCode: UnsafeCode,
163164
SpecialModuleName: SpecialModuleName,
164165
AnonymousParameters: AnonymousParameters,
165166
EllipsisInclusiveRangePatterns: EllipsisInclusiveRangePatterns::default(),
166167
NonCamelCaseTypes: NonCamelCaseTypes,
167-
DeprecatedAttr: DeprecatedAttr::new(),
168+
DeprecatedAttr: DeprecatedAttr::default(),
168169
WhileTrue: WhileTrue,
169170
NonAsciiIdents: NonAsciiIdents,
170171
HiddenUnicodeCodepoints: HiddenUnicodeCodepoints,
@@ -545,23 +546,23 @@ fn register_builtins(store: &mut LintStore) {
545546
}
546547

547548
fn register_internals(store: &mut LintStore) {
548-
store.register_lints(&LintPassImpl::get_lints());
549+
store.register_lints(&LintPassImpl::default().get_lints());
549550
store.register_early_pass(|| Box::new(LintPassImpl));
550-
store.register_lints(&DefaultHashTypes::get_lints());
551+
store.register_lints(&DefaultHashTypes::default().get_lints());
551552
store.register_late_mod_pass(|_| Box::new(DefaultHashTypes));
552-
store.register_lints(&QueryStability::get_lints());
553+
store.register_lints(&QueryStability::default().get_lints());
553554
store.register_late_mod_pass(|_| Box::new(QueryStability));
554-
store.register_lints(&ExistingDocKeyword::get_lints());
555+
store.register_lints(&ExistingDocKeyword::default().get_lints());
555556
store.register_late_mod_pass(|_| Box::new(ExistingDocKeyword));
556-
store.register_lints(&TyTyKind::get_lints());
557+
store.register_lints(&TyTyKind::default().get_lints());
557558
store.register_late_mod_pass(|_| Box::new(TyTyKind));
558-
store.register_lints(&Diagnostics::get_lints());
559+
store.register_lints(&Diagnostics::default().get_lints());
559560
store.register_late_mod_pass(|_| Box::new(Diagnostics));
560-
store.register_lints(&BadOptAccess::get_lints());
561+
store.register_lints(&BadOptAccess::default().get_lints());
561562
store.register_late_mod_pass(|_| Box::new(BadOptAccess));
562-
store.register_lints(&PassByValue::get_lints());
563+
store.register_lints(&PassByValue::default().get_lints());
563564
store.register_late_mod_pass(|_| Box::new(PassByValue));
564-
store.register_lints(&SpanUseEqCtxt::get_lints());
565+
store.register_lints(&SpanUseEqCtxt::default().get_lints());
565566
store.register_late_mod_pass(|_| Box::new(SpanUseEqCtxt));
566567
// FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and
567568
// `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and

compiler/rustc_lint/src/passes.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ macro_rules! declare_combined_late_lint_pass {
108108

109109
$v fn get_lints() -> $crate::LintVec {
110110
let mut lints = Vec::new();
111-
$(lints.extend_from_slice(&$pass::get_lints());)*
111+
$(lints.extend_from_slice(&$pass::default().get_lints());)*
112112
lints
113113
}
114114
}
@@ -122,6 +122,9 @@ macro_rules! declare_combined_late_lint_pass {
122122
fn name(&self) -> &'static str {
123123
panic!()
124124
}
125+
fn get_lints(&self) -> LintVec {
126+
panic!()
127+
}
125128
}
126129
)
127130
}
@@ -218,7 +221,7 @@ macro_rules! declare_combined_early_lint_pass {
218221

219222
$v fn get_lints() -> $crate::LintVec {
220223
let mut lints = Vec::new();
221-
$(lints.extend_from_slice(&$pass::get_lints());)*
224+
$(lints.extend_from_slice(&$pass::default().get_lints());)*
222225
lints
223226
}
224227
}
@@ -232,6 +235,9 @@ macro_rules! declare_combined_early_lint_pass {
232235
fn name(&self) -> &'static str {
233236
panic!()
234237
}
238+
fn get_lints(&self) -> LintVec {
239+
panic!()
240+
}
235241
}
236242
)
237243
}

compiler/rustc_lint/src/types.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ declare_lint! {
170170
"detects ambiguous wide pointer comparisons"
171171
}
172172

173-
#[derive(Copy, Clone)]
173+
#[derive(Copy, Clone, Default)]
174174
pub struct TypeLimits {
175175
/// Id of the last visited negated expression
176176
negated_expr_id: Option<hir::HirId>,

compiler/rustc_lint/src/unused.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1018,8 +1018,8 @@ pub struct UnusedParens {
10181018
parens_in_cast_in_lt: Vec<ast::NodeId>,
10191019
}
10201020

1021-
impl UnusedParens {
1022-
pub fn new() -> Self {
1021+
impl Default for UnusedParens {
1022+
fn default() -> Self {
10231023
Self { with_self_ty_parens: false, parens_in_cast_in_lt: Vec::new() }
10241024
}
10251025
}

compiler/rustc_lint_defs/src/lib.rs

+14-4
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,7 @@ pub type LintVec = Vec<&'static Lint>;
870870

871871
pub trait LintPass {
872872
fn name(&self) -> &'static str;
873+
fn get_lints(&self) -> LintVec;
873874
}
874875

875876
/// Implements `LintPass for $ty` with the given list of `Lint` statics.
@@ -878,9 +879,7 @@ macro_rules! impl_lint_pass {
878879
($ty:ty => [$($lint:expr),* $(,)?]) => {
879880
impl $crate::LintPass for $ty {
880881
fn name(&self) -> &'static str { stringify!($ty) }
881-
}
882-
impl $ty {
883-
pub fn get_lints() -> $crate::LintVec { vec![$($lint),*] }
882+
fn get_lints(&self) -> $crate::LintVec { vec![$($lint),*] }
884883
}
885884
};
886885
}
@@ -890,7 +889,18 @@ macro_rules! impl_lint_pass {
890889
#[macro_export]
891890
macro_rules! declare_lint_pass {
892891
($(#[$m:meta])* $name:ident => [$($lint:expr),* $(,)?]) => {
893-
$(#[$m])* #[derive(Copy, Clone)] pub struct $name;
892+
$(#[$m])* #[derive(Copy, Clone, Default)] pub struct $name;
894893
$crate::impl_lint_pass!($name => [$($lint),*]);
895894
};
896895
}
896+
897+
#[allow(rustc::lint_pass_impl_without_macro)]
898+
impl<P: LintPass + ?Sized> LintPass for Box<P> {
899+
fn name(&self) -> &'static str {
900+
(**self).name()
901+
}
902+
903+
fn get_lints(&self) -> LintVec {
904+
(**self).get_lints()
905+
}
906+
}

compiler/rustc_middle/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ rustc_graphviz = { path = "../rustc_graphviz" }
2727
rustc_hir = { path = "../rustc_hir" }
2828
rustc_hir_pretty = { path = "../rustc_hir_pretty" }
2929
rustc_index = { path = "../rustc_index" }
30+
rustc_lint_defs = { path = "../rustc_lint_defs" }
3031
rustc_macros = { path = "../rustc_macros" }
3132
rustc_query_system = { path = "../rustc_query_system" }
3233
rustc_serialize = { path = "../rustc_serialize" }

compiler/rustc_middle/src/lint.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl ShallowLintLevelMap {
117117
/// This lint level is not usable for diagnostics, it needs to be corrected by
118118
/// `reveal_actual_level` beforehand.
119119
#[instrument(level = "trace", skip(self, tcx), ret)]
120-
fn probe_for_lint_level(
120+
pub fn probe_for_lint_level(
121121
&self,
122122
tcx: TyCtxt<'_>,
123123
id: LintId,

0 commit comments

Comments
 (0)