Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[experiment] Unbundle late combined pass, filter late passes before linting #116271

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4095,6 +4095,7 @@ dependencies = [
"rustc_graphviz",
"rustc_hir",
"rustc_index",
"rustc_lint_defs",
"rustc_macros",
"rustc_query_system",
"rustc_serialize",
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
//! To add a new lint to rustc, declare it here using `declare_lint!()`.
//! Then add code to emit the new lint in the appropriate circumstances.
//! You can do that in an existing `LintPass` if it makes sense, or in a
//! new `LintPass`, or using `Session::add_lint` elsewhere in the
//! compiler. Only do the latter if the check can't be written cleanly as a
//! `LintPass` (also, note that such lints will need to be defined in
//! `rustc_session::lint::builtin`, not here).
//! new `LintPass`.
//!
//! If you define a new `EarlyLintPass`, you will also need to add it to the
//! `add_early_builtin!` or `add_early_builtin_with_new!` invocation in
Expand Down Expand Up @@ -39,13 +36,14 @@ use crate::{
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, SuggestChangingAssocTypes,
},
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext, LintId,
};
use rustc_ast::attr;
use rustc_ast::tokenstream::{TokenStream, TokenTree};
use rustc_ast::visit::{FnCtxt, FnKind};
use rustc_ast::{self as ast, *};
use rustc_ast_pretty::pprust::{self, expr_to_string};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, DecorateLint, MultiSpan};
use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, GateIssue, Stability};
use rustc_hir as hir;
Expand Down Expand Up @@ -1523,6 +1521,9 @@ declare_lint_pass!(
/// unused within this crate, even though downstream crates can't use it
/// without producing an error.
UnusedBrokenConst => []
fn is_enabled(&self, _: &FxHashSet<LintId>) -> bool {
true
}
);

impl<'tcx> LateLintPass<'tcx> for UnusedBrokenConst {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,9 @@ pub struct LateContext<'tcx> {

/// We are only looking at one module
pub only_module: bool,

#[cfg(debug_assertions)]
pub(super) permitted_lints: Cell<Option<&'static [&'static str]>>,
}

/// Context for lint checking of the AST, after expansion, before lowering to HIR.
Expand Down Expand Up @@ -1067,6 +1070,15 @@ impl<'tcx> LintContext for LateContext<'tcx> {
&'b mut DiagnosticBuilder<'a, ()>,
) -> &'b mut DiagnosticBuilder<'a, ()>,
) {
#[cfg(debug_assertions)]
if let Some(permitted) = self.permitted_lints.get() {
assert!(
permitted.contains(&lint.name),
"unexpected lint {} emitted from pass. permitted: {permitted:?}",
lint.name
);
}

let hir_id = self.last_node_with_lint_attrs;

match span {
Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rustc_hir::{def_id::DefId, Expr, ExprKind, GenericArg, PatKind, Path, PathSe
use rustc_hir::{HirId, Impl, Item, ItemKind, Node, Pat, Ty, TyKind};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::Span;

Expand Down Expand Up @@ -284,18 +283,11 @@ impl EarlyLintPass for LintPassImpl {
if let ast::ItemKind::Impl(box ast::Impl { of_trait: Some(lint_pass), .. }) = &item.kind {
if let Some(last) = lint_pass.path.segments.last() {
if last.ident.name == sym::LintPass {
let expn_data = lint_pass.path.span.ctxt().outer_expn_data();
let call_site = expn_data.call_site;
if expn_data.kind != ExpnKind::Macro(MacroKind::Bang, sym::impl_lint_pass)
&& call_site.ctxt().outer_expn_data().kind
!= ExpnKind::Macro(MacroKind::Bang, sym::declare_lint_pass)
{
cx.emit_spanned_lint(
LINT_PASS_IMPL_WITHOUT_MACRO,
lint_pass.path.span,
LintPassByHand,
);
}
cx.emit_spanned_lint(
LINT_PASS_IMPL_WITHOUT_MACRO,
lint_pass.path.span,
LintPassByHand,
);
}
}
}
Expand Down
98 changes: 76 additions & 22 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@
//! upon. As the ast is traversed, this keeps track of the current lint level
//! for all lint attributes.

use crate::{passes::LateLintPassObject, LateContext, LateLintPass, LintStore};
use crate::{passes::LateLintPassObject, LateContext, LateLintPass, Level, LintId, LintStore};
use rustc_ast as ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::join;
use rustc_hir as hir;
use rustc_hir::def_id::{LocalDefId, LocalModDefId};
use rustc_hir::intravisit as hir_visit;
use rustc_middle::hir::nested_filter;
use rustc_middle::lint::{reveal_actual_level, LintLevelSource};
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::lint::LintPass;
use rustc_span::Span;
Expand Down Expand Up @@ -315,8 +317,7 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas

// Combines multiple lint passes into a single pass, at runtime. Each
// `check_foo` method in `$methods` within this pass simply calls `check_foo`
// once per `$pass`. Compare with `declare_combined_late_lint_pass`, which is
// similar, but combines lint passes at compile time.
// once per `$pass`.
struct RuntimeCombinedLateLintPass<'a, 'tcx> {
passes: &'a mut [LateLintPassObject<'tcx>],
}
Expand All @@ -333,6 +334,8 @@ macro_rules! impl_late_lint_pass {
impl<'tcx> LateLintPass<'tcx> for RuntimeCombinedLateLintPass<'_, 'tcx> {
$(fn $f(&mut self, context: &LateContext<'tcx>, $($param: $arg),*) {
for pass in self.passes.iter_mut() {
#[cfg(debug_assertions)]
context.permitted_lints.set(pass.lint_names());
pass.$f(context, $($param),*);
}
})*
Expand All @@ -342,11 +345,7 @@ macro_rules! impl_late_lint_pass {

crate::late_lint_methods!(impl_late_lint_pass, []);

pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
tcx: TyCtxt<'tcx>,
module_def_id: LocalModDefId,
builtin_lints: T,
) {
pub fn late_lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
let context = LateContext {
tcx,
enclosing_body: None,
Expand All @@ -357,20 +356,20 @@ pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
last_node_with_lint_attrs: tcx.hir().local_def_id_to_hir_id(module_def_id),
generics: None,
only_module: true,
#[cfg(debug_assertions)]
permitted_lints: Cell::new(None),
};

// 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 mut passes: Vec<_> =
unerased_lint_store(tcx).late_module_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
if passes.is_empty() {
late_lint_mod_inner(tcx, module_def_id, context, builtin_lints);
} else {
passes.push(Box::new(builtin_lints));
let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
late_lint_mod_inner(tcx, module_def_id, context, pass);
}
let enabled_lints = tcx.enabled_lints(());

let mut passes: Vec<_> = unerased_lint_store(tcx)
.late_module_passes
.iter()
.map(|mk_pass| (mk_pass)(tcx))
.filter(|pass| pass.is_enabled(enabled_lints))
.collect();
let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
late_lint_mod_inner(tcx, module_def_id, context, pass);
}

fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
Expand Down Expand Up @@ -398,9 +397,18 @@ fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
}

fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
// Trigger check for duplicate diagnostic items
let _ = tcx.all_diagnostic_items(());

let enabled_lints = tcx.enabled_lints(());

// Note: `passes` is often empty.
let mut passes: Vec<_> =
unerased_lint_store(tcx).late_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();
let mut passes: Vec<_> = unerased_lint_store(tcx)
.late_passes
.iter()
.map(|mk_pass| (mk_pass)(tcx))
.filter(|pass| pass.is_enabled(enabled_lints))
.collect();

if passes.is_empty() {
return;
Expand All @@ -416,6 +424,8 @@ fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
last_node_with_lint_attrs: hir::CRATE_HIR_ID,
generics: None,
only_module: false,
#[cfg(debug_assertions)]
permitted_lints: Cell::new(None),
};

let pass = RuntimeCombinedLateLintPass { passes: &mut passes[..] };
Expand Down Expand Up @@ -456,3 +466,47 @@ pub fn check_crate<'tcx>(tcx: TyCtxt<'tcx>) {
},
);
}

pub(crate) fn enabled_lints(tcx: TyCtxt<'_>) -> FxHashSet<LintId> {
let get_level = |spec: Option<&FxHashMap<_, _>>, lint| match spec.and_then(|m| m.get(&lint)) {
Some(&(level, source)) => (Some(level), source),
None => (None, LintLevelSource::Default),
};
let may_lint_shallow = |spec: Option<&FxHashMap<_, _>>, level, mut source, lint| {
let actual =
reveal_actual_level(level, &mut source, tcx.sess, lint, |lint| get_level(spec, lint));

actual > Level::Allow
};

let root_lints =
tcx.shallow_lint_levels_on(hir::CRATE_OWNER_ID).specs.get(&hir::CRATE_HIR_ID.local_id);

let mut enabled: FxHashSet<_> = unerased_lint_store(tcx)
.get_lints()
.iter()
.map(|lint| LintId::of(lint))
.filter(|&lint| {
let (level, source) = get_level(root_lints, lint);
may_lint_shallow(root_lints, level, source, lint)
})
.collect();

for (def_id, maybe_owner) in tcx.hir().krate().owners.iter_enumerated() {
if let hir::MaybeOwner::Owner(_) = maybe_owner {
enabled.extend(
tcx.shallow_lint_levels_on(hir::OwnerId { def_id })
.specs
.values()
.flat_map(|spec| {
spec.iter().filter(|&(&lint, &(level, source))| {
may_lint_shallow(Some(spec), Some(level), source, lint)
})
})
.map(|(&lint, _)| lint),
);
}
}

enabled
}
Loading
Loading