Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ fn resolve_block<'tcx>(
if !terminating
&& !visitor
.tcx
.lints_that_dont_need_to_run(())
.skippable_lints(())
.contains(&lint::LintId::of(lint::builtin::TAIL_EXPR_DROP_ORDER))
{
// If this temporary scope will be changing once the codebase adopts Rust 2024,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ fn pre_expansion_lint<'a>(
lint_store,
registered_tools,
None,
rustc_lint::BuiltinCombinedPreExpansionLintPass::new(),
check_node,
);
},
Expand Down Expand Up @@ -479,7 +478,6 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) {
lint_store,
tcx.registered_tools(()),
Some(lint_buffer),
rustc_lint::BuiltinCombinedEarlyLintPass::new(),
EarlyCheckNode::CrateRoot(&*krate, &*krate.attrs),
)
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ use self::TargetLint::*;
use crate::levels::LintLevelsBuilder;
use crate::passes::{EarlyLintPassObject, LateLintPassObject};

type EarlyLintPassFactory = Box<dyn Fn() -> EarlyLintPassObject + sync::DynSend + sync::DynSync>;
pub(crate) type EarlyLintPassFactory =
Box<dyn Fn() -> EarlyLintPassObject + sync::DynSend + sync::DynSync>;
type LateLintPassFactory =
Box<dyn for<'tcx> Fn(TyCtxt<'tcx>) -> LateLintPassObject<'tcx> + sync::DynSend + sync::DynSync>;

Expand Down
65 changes: 39 additions & 26 deletions compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_span::{Ident, Span};
use tracing::debug;

use crate::DiagAndSess;
use crate::context::{EarlyContext, LintContext, LintStore};
use crate::context::{EarlyContext, EarlyLintPassFactory, LintContext, LintStore};
use crate::passes::{EarlyLintPass, EarlyLintPassObject};

pub(super) mod diagnostics;
Expand Down Expand Up @@ -316,7 +316,6 @@ pub fn check_ast_node<'a>(
lint_store: &LintStore,
registered_tools: &RegisteredTools,
lint_buffer: Option<LintBuffer>,
builtin_lints: impl EarlyLintPass + 'static,
check_node: EarlyCheckNode<'a>,
) {
let context = EarlyContext::new(
Expand All @@ -328,35 +327,20 @@ pub fn check_ast_node<'a>(
lint_buffer.unwrap_or_default(),
);

// Note: `passes` is often empty. In that case, it's faster to run
// `builtin_lints` directly rather than bundling it up into the
// `RuntimeCombinedEarlyLintPass`.
let passes =
if pre_expansion { &lint_store.pre_expansion_passes } else { &lint_store.early_passes };
if passes.is_empty() {
check_ast_node_inner(sess, check_node, context, builtin_lints);
let context = if pre_expansion {
let builtin_lints = crate::BuiltinCombinedPreExpansionLintPass::new();
let passes = &lint_store.pre_expansion_passes;
run_passes(check_node, context, builtin_lints, passes)
} else {
let mut passes: Vec<_> = passes.iter().map(|mk_pass| (mk_pass)()).collect();
passes.push(Box::new(builtin_lints));
let pass = RuntimeCombinedEarlyLintPass { passes };
check_ast_node_inner(sess, check_node, context, pass);
}
}

fn check_ast_node_inner<'a, T: EarlyLintPass>(
sess: &Session,
check_node: EarlyCheckNode<'a>,
context: EarlyContext<'_>,
pass: T,
) {
let mut cx = EarlyContextAndPass { context, pass };

cx.with_lint_attrs(check_node.id(), check_node.attrs(), |cx| check_node.check(cx));
let builtin_lints = crate::BuiltinCombinedEarlyLintPass::new();
let passes = &lint_store.early_passes;
run_passes(check_node, context, builtin_lints, passes)
};

// All of the buffered lints should have been emitted at this point.
// If not, that means that we somehow buffered a lint for a node id
// that was not lint-checked (perhaps it doesn't exist?). This is a bug.
for (id, lints) in cx.context.buffered.map {
for (id, lints) in context.buffered.map {
if !lints.is_empty() {
assert!(
sess.dcx().has_errors().is_some(),
Expand All @@ -367,3 +351,32 @@ fn check_ast_node_inner<'a, T: EarlyLintPass>(
}
}
}

fn run_passes<'a, 'ecx, T: EarlyLintPass + 'static>(
check_node: EarlyCheckNode<'a>,
context: EarlyContext<'ecx>,
builtin_lints: T,
passes: &[EarlyLintPassFactory],
) -> EarlyContext<'ecx> {
// Note: `passes` is often empty. In that case, it's faster to run
// `builtin_lints` directly rather than bundling it up into the
// `RuntimeCombinedEarlyLintPass`.
if passes.is_empty() {
run_pass(check_node, context, builtin_lints)
} else {
let mut passes: Vec<_> = passes.iter().map(|mk_pass| mk_pass()).collect();
passes.push(Box::new(builtin_lints));
let pass = RuntimeCombinedEarlyLintPass { passes };
run_pass(check_node, context, pass)
}
}

fn run_pass<'a, 'ecx, T: EarlyLintPass>(
check_node: EarlyCheckNode<'a>,
context: EarlyContext<'ecx>,
pass: T,
) -> EarlyContext<'ecx> {
let mut cx = EarlyContextAndPass { context, pass };
cx.with_lint_attrs(check_node.id(), check_node.attrs(), |cx| check_node.check(cx));
cx.context
}
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/if_let_rescope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl_lint_pass!(
impl<'tcx> LateLintPass<'tcx> for IfLetRescope {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if expr.span.edition().at_least_rust_2024()
|| cx.tcx.lints_that_dont_need_to_run(()).contains(&LintId::of(IF_LET_RESCOPE))
|| cx.tcx.skippable_lints(()).contains(&LintId::of(IF_LET_RESCOPE))
{
return;
}
Expand Down
52 changes: 22 additions & 30 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_span::Span;
use tracing::debug;

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

/// Extract the [`LintStore`] from [`Session`].
///
Expand Down Expand Up @@ -349,31 +349,26 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
only_module: true,
};

let skippable_lints = tcx.skippable_lints(());

// Note: `passes` is often empty. In that case, it's faster to run
// `builtin_lints` directly rather than bundling it up into the
// `RuntimeCombinedLateLintPass`.
let store = unerased_lint_store(tcx.sess);

if store.late_module_passes.is_empty() {
// If all builtin lints can be skipped, there is no point in running `late_lint_mod_inner`
// at all. This happens often for dependencies built with `--cap-lints=allow`.
let dont_need_to_run = tcx.lints_that_dont_need_to_run(());
let can_skip_lints = builtin_lints
.get_lints()
.iter()
.all(|lint| dont_need_to_run.contains(&LintId::of(lint)));
if !can_skip_lints {
let mut passes: Vec<_> = unerased_lint_store(tcx.sess)
.late_module_passes
.iter()
.map(|mk_pass| mk_pass(tcx))
.filter(|pass| pass_must_run(skippable_lints, &pass.get_lints()))
.collect();
let builtin_lints_must_run = pass_must_run(skippable_lints, &builtin_lints.get_lints());
if passes.is_empty() {
if builtin_lints_must_run {
late_lint_mod_inner(tcx, module_def_id, context, builtin_lints);
}
} else {
let builtin_lints = Box::new(builtin_lints) as Box<dyn LateLintPass<'tcx>>;
let passes = store
.late_module_passes
.iter()
.map(|mk_pass| (mk_pass)(tcx))
.chain(std::iter::once(builtin_lints))
.collect::<Vec<_>>();

if builtin_lints_must_run {
passes.push(Box::new(builtin_lints) as Box<dyn LateLintPass<'tcx>>);
}
let pass = RuntimeCombinedLateLintPass { passes };
late_lint_mod_inner(tcx, module_def_id, context, pass);
}
Expand Down Expand Up @@ -404,18 +399,15 @@ fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
}

fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
let lints_that_dont_need_to_run = tcx.lints_that_dont_need_to_run(());
let skippable_lints = tcx.skippable_lints(());

// Note: `passes` is often empty after filtering.
let mut passes: Vec<_> =
unerased_lint_store(tcx.sess).late_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
passes.retain(|pass| {
let lints = pass.get_lints();
// Lintless passes are always in
lints.is_empty() ||
// If the pass doesn't have a single needed lint, omit it
!lints.iter().all(|lint| lints_that_dont_need_to_run.contains(&LintId::of(lint)))
});
let passes: Vec<_> = unerased_lint_store(tcx.sess)
.late_passes
.iter()
.map(|mk_pass| mk_pass(tcx))
.filter(|pass| pass_must_run(skippable_lints, &pass.get_lints()))
.collect();
if passes.is_empty() {
return;
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ impl LintLevelSets {
}
}

fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> UnordSet<LintId> {
fn skippable_lints(tcx: TyCtxt<'_>, (): ()) -> UnordSet<LintId> {
let store = unerased_lint_store(&tcx.sess);
let root_map = tcx.shallow_lint_levels_on(hir::CRATE_OWNER_ID);

let mut dont_need_to_run: FxHashSet<LintId> = store
let mut skippable: FxHashSet<LintId> = store
.get_lints()
.into_iter()
.filter(|lint| {
Expand All @@ -145,13 +145,13 @@ fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> UnordSet<LintId> {
for (_, specs) in map.specs.iter() {
for (lint, level_spec) in specs.iter() {
if !level_spec.is_allow() {
dont_need_to_run.remove(lint);
skippable.remove(lint);
}
}
}
}

dont_need_to_run.into()
skippable.into()
}

#[instrument(level = "trace", skip(tcx), ret)]
Expand Down Expand Up @@ -1035,7 +1035,7 @@ where
}

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

pub(crate) fn parse_lint_and_tool_name(lint_name: &str) -> (Option<Symbol>, &str) {
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ use precedence::*;
use ptr_nulls::*;
use redundant_semicolon::*;
use reference_casting::*;
use rustc_data_structures::unord::UnordSet;
use rustc_hir::def_id::LocalModDefId;
use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -742,5 +743,21 @@ fn register_internals(store: &mut LintStore) {
);
}

/// Must a pass (which contains `lints`) run? Maybe not, e.g. for dependencies built with
/// `--cap-lints=allow`.
///
/// Note: this is a conservative estimate intended for optimization purposes. It might return
/// `true` for a pass that need not run, but it will never return `false` for a pass that must run.
pub fn pass_must_run(skippable: &UnordSet<LintId>, lints: &LintVec) -> bool {
Comment thread
Urgau marked this conversation as resolved.
Outdated
// A pass without any lints? Clippy sometimes does this, to collect things while traversing.
// Such a pass must always run.
if lints.is_empty() {
return true;
}

// Otherwise, the pass must run unless all lints within are skippable.
!lints.iter().all(|lint| skippable.contains(&LintId::of(lint)))
}

#[cfg(test)]
mod tests;
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ rustc_queries! {
desc { "computing `#[expect]`ed lints in this crate" }
}

query lints_that_dont_need_to_run(_: ()) -> &'tcx UnordSet<LintId> {
query skippable_lints(_: ()) -> &'tcx UnordSet<LintId> {
arena_cache
// This depends on the lint store, which includes internal lints when the
// untracked `-Zunstable-options` flag is set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
return;
}
if body.span.edition().at_least_rust_2024()
|| tcx.lints_that_dont_need_to_run(()).contains(&lint::LintId::of(TAIL_EXPR_DROP_ORDER))
|| tcx.skippable_lints(()).contains(&lint::LintId::of(TAIL_EXPR_DROP_ORDER))
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/combined_early_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! inlined calls. No vtable, no per-node dynamic dispatch.
//!
//! Unlike the late combine there is no `active` gate. rustc drops fully-disabled
//! late passes via `lints_that_dont_need_to_run`, but the early pass runner has
//! late passes via `skippable_lints`, but the early pass runner has
//! no such filtering, so a plain forward is equivalent and loses nothing.
//!
//! [`combined_late_pass`]: crate::combined_late_pass
Expand Down
11 changes: 3 additions & 8 deletions src/tools/clippy/clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ mod zombie_processes;
use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation};
use clippy_utils::macros::FormatArgsStorage;
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::Lint;
use rustc_lint::{Lint, pass_must_run};
use rustc_middle::ty::TyCtxt;
use utils::attr_collector::AttrStorage;

Expand Down Expand Up @@ -470,13 +470,8 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
}

store.late_passes.push(Box::new(move |tcx: TyCtxt<'_>| {
let dont_need = tcx.lints_that_dont_need_to_run(());
let is_active = |lints: &rustc_lint::LintVec| {
lints.is_empty()
|| !lints
.iter()
.all(|lint| dont_need.contains(&rustc_lint::LintId::of(lint)))
};
let skippable_lints = tcx.skippable_lints(());
let is_active = |lints: &rustc_lint::LintVec| pass_must_run(skippable_lints, lints);
Box::new(CombinedLateLintPass::new(
tcx,
conf,
Expand Down
Loading