Skip to content

Commit

Permalink
Auto merge of #113734 - cjgillot:no-crate-lint, r=petrochenkov
Browse files Browse the repository at this point in the history
Convert builtin "global" late lints to run per module

The compiler currently has 4 non-incremental lints:
1. `clashing_extern_declarations`;
2. `missing_debug_implementations`;
3. ~`unnameable_test_items`;~ changed by #114414
4. `missing_docs`.

Non-incremental lints get reexecuted for each compilation, which is slow. Moreover, those lints are allow-by-default, so run for nothing most of the time. This PR attempts to make them more incremental-friendly.

`clashing_extern_declarations` is moved to a standalone query.

`missing_debug_implementation` can use `non_blanket_impls_for_ty` instead of recomputing it.

`missing_docs` is harder as it needs to track if there is a `doc(hidden)` module surrounding. I hack around this using the lint level engine. That's easy to implement and allows to re-enable the lint for a re-exported module, while a more proper solution would reuse the same device as `unnameable_test_items`.
  • Loading branch information
bors committed Aug 5, 2023
2 parents fca59ab + 905b63d commit 67626b8
Show file tree
Hide file tree
Showing 16 changed files with 653 additions and 671 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,10 +846,11 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
},
{
sess.time("lint_checking", || {
rustc_lint::check_crate(tcx, || {
rustc_lint::BuiltinCombinedLateLintPass::new()
});
rustc_lint::check_crate(tcx);
});
},
{
tcx.ensure().clashing_extern_declarations(());
}
);
},
Expand Down
463 changes: 23 additions & 440 deletions compiler/rustc_lint/src/builtin.rs

Large diffs are not rendered by default.

402 changes: 402 additions & 0 deletions compiler/rustc_lint/src/foreign_modules.rs

Large diffs are not rendered by default.

42 changes: 22 additions & 20 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use crate::{passes::LateLintPassObject, LateContext, LateLintPass, LintStore};
use rustc_ast as ast;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::{join, DynSend};
use rustc_data_structures::sync::join;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit as hir_visit;
Expand Down Expand Up @@ -336,7 +336,7 @@ macro_rules! impl_late_lint_pass {

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

pub(super) fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
pub fn late_lint_mod<'tcx, T: LateLintPass<'tcx> + 'tcx>(
tcx: TyCtxt<'tcx>,
module_def_id: LocalDefId,
builtin_lints: T,
Expand Down Expand Up @@ -376,17 +376,32 @@ fn late_lint_mod_inner<'tcx, T: LateLintPass<'tcx>>(
let mut cx = LateContextAndPass { context, pass };

let (module, _span, hir_id) = tcx.hir().get_module(module_def_id);

// There is no module lint that will have the crate itself as an item, so check it here.
if hir_id == hir::CRATE_HIR_ID {
lint_callback!(cx, check_crate,);
}

cx.process_mod(module, hir_id);

// Visit the crate attributes
if hir_id == hir::CRATE_HIR_ID {
for attr in tcx.hir().attrs(hir::CRATE_HIR_ID).iter() {
cx.visit_attribute(attr)
}
lint_callback!(cx, check_crate_post,);
}
}

fn late_lint_crate<'tcx, T: LateLintPass<'tcx> + 'tcx>(tcx: TyCtxt<'tcx>, builtin_lints: T) {
fn late_lint_crate<'tcx>(tcx: TyCtxt<'tcx>) {
// Note: `passes` is often empty.
let mut passes: Vec<_> =
unerased_lint_store(tcx).late_passes.iter().map(|mk_pass| (mk_pass)(tcx)).collect();

if passes.is_empty() {
return;
}

let context = LateContext {
tcx,
enclosing_body: None,
Expand All @@ -399,18 +414,8 @@ fn late_lint_crate<'tcx, T: LateLintPass<'tcx> + 'tcx>(tcx: TyCtxt<'tcx>, builti
only_module: false,
};

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

fn late_lint_crate_inner<'tcx, T: LateLintPass<'tcx>>(
Expand All @@ -432,15 +437,12 @@ fn late_lint_crate_inner<'tcx, T: LateLintPass<'tcx>>(
}

/// Performs lint checking on a crate.
pub fn check_crate<'tcx, T: LateLintPass<'tcx> + 'tcx>(
tcx: TyCtxt<'tcx>,
builtin_lints: impl FnOnce() -> T + Send + DynSend,
) {
pub fn check_crate<'tcx>(tcx: TyCtxt<'tcx>) {
join(
|| {
tcx.sess.time("crate_lints", || {
// Run whole crate non-incremental lints
late_lint_crate(tcx, builtin_lints());
late_lint_crate(tcx);
});
},
|| {
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
builtin::MISSING_DOCS,
context::{CheckLintNameResult, LintStore},
fluent_generated as fluent,
late::unerased_lint_store,
Expand Down Expand Up @@ -667,6 +668,16 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
continue;
}

// `#[doc(hidden)]` disables missing_docs check.
if attr.has_name(sym::doc)
&& attr
.meta_item_list()
.map_or(false, |l| ast::attr::list_contains_name(&l, sym::hidden))
{
self.insert(LintId::of(MISSING_DOCS), (Level::Allow, LintLevelSource::Default));
continue;
}

let level = match Level::from_attr(attr) {
None => continue,
// This is the only lint level with a `LintExpectationId` that can be created from an attribute
Expand Down
45 changes: 15 additions & 30 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ mod enum_intrinsics_non_enums;
mod errors;
mod expect;
mod for_loops_over_fallibles;
mod foreign_modules;
pub mod hidden_unicode_codepoints;
mod internal;
mod invalid_from_utf8;
Expand Down Expand Up @@ -125,11 +126,11 @@ use types::*;
use unused::*;

/// Useful for other parts of the compiler / Clippy.
pub use builtin::SoftLints;
pub use builtin::{MissingDoc, SoftLints};
pub use context::{CheckLintNameResult, FindLintError, LintStore};
pub use context::{EarlyContext, LateContext, LintContext};
pub use early::{check_ast_node, EarlyCheckNode};
pub use late::{check_crate, unerased_lint_store};
pub use late::{check_crate, late_lint_mod, unerased_lint_store};
pub use passes::{EarlyLintPass, LateLintPass};
pub use rustc_session::lint::Level::{self, *};
pub use rustc_session::lint::{BufferedEarlyLint, FutureIncompatibleInfo, Lint, LintId};
Expand All @@ -140,11 +141,12 @@ fluent_messages! { "../messages.ftl" }
pub fn provide(providers: &mut Providers) {
levels::provide(providers);
expect::provide(providers);
foreign_modules::provide(providers);
*providers = Providers { lint_mod, ..*providers };
}

fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
late::late_lint_mod(tcx, module_def_id, BuiltinCombinedModuleLateLintPass::new());
late_lint_mod(tcx, module_def_id, BuiltinCombinedModuleLateLintPass::new());
}

early_lint_methods!(
Expand Down Expand Up @@ -182,25 +184,6 @@ early_lint_methods!(
]
);

// FIXME: Make a separate lint type which does not require typeck tables.

late_lint_methods!(
declare_combined_late_lint_pass,
[
pub BuiltinCombinedLateLintPass,
[
// Tracks attributes of parents
MissingDoc: MissingDoc::new(),
// Builds a global list of all impls of `Debug`.
// FIXME: Turn the computation of types which implement Debug into a query
// and change this to a module lint pass
MissingDebugImplementations: MissingDebugImplementations::default(),
// Keeps a global list of foreign declarations.
ClashingExternDeclarations: ClashingExternDeclarations::new(),
]
]
);

late_lint_methods!(
declare_combined_late_lint_pass,
[
Expand Down Expand Up @@ -253,6 +236,8 @@ late_lint_methods!(
OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
MultipleSupertraitUpcastable: MultipleSupertraitUpcastable,
MapUnitFn: MapUnitFn,
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
]
]
);
Expand Down Expand Up @@ -281,7 +266,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&BuiltinCombinedPreExpansionLintPass::get_lints());
store.register_lints(&BuiltinCombinedEarlyLintPass::get_lints());
store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints());
store.register_lints(&BuiltinCombinedLateLintPass::get_lints());
store.register_lints(&foreign_modules::get_lints());

add_lint_group!(
"nonstandard_style",
Expand Down Expand Up @@ -521,20 +506,20 @@ fn register_internals(store: &mut LintStore) {
store.register_lints(&LintPassImpl::get_lints());
store.register_early_pass(|| Box::new(LintPassImpl));
store.register_lints(&DefaultHashTypes::get_lints());
store.register_late_pass(|_| Box::new(DefaultHashTypes));
store.register_late_mod_pass(|_| Box::new(DefaultHashTypes));
store.register_lints(&QueryStability::get_lints());
store.register_late_pass(|_| Box::new(QueryStability));
store.register_late_mod_pass(|_| Box::new(QueryStability));
store.register_lints(&ExistingDocKeyword::get_lints());
store.register_late_pass(|_| Box::new(ExistingDocKeyword));
store.register_late_mod_pass(|_| Box::new(ExistingDocKeyword));
store.register_lints(&TyTyKind::get_lints());
store.register_late_pass(|_| Box::new(TyTyKind));
store.register_late_mod_pass(|_| Box::new(TyTyKind));
store.register_lints(&Diagnostics::get_lints());
store.register_early_pass(|| Box::new(Diagnostics));
store.register_late_pass(|_| Box::new(Diagnostics));
store.register_late_mod_pass(|_| Box::new(Diagnostics));
store.register_lints(&BadOptAccess::get_lints());
store.register_late_pass(|_| Box::new(BadOptAccess));
store.register_late_mod_pass(|_| Box::new(BadOptAccess));
store.register_lints(&PassByValue::get_lints());
store.register_late_pass(|_| Box::new(PassByValue));
store.register_late_mod_pass(|_| Box::new(PassByValue));
// FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and
// `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and
// these lints will trigger all of the time - change this once migration to diagnostic structs
Expand Down
43 changes: 21 additions & 22 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,7 @@ pub fn transparent_newtype_field<'a, 'tcx>(
}

/// Is type known to be non-null?
fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool {
let tcx = cx.tcx;
fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool {
match ty.kind() {
ty::FnPtr(_) => true,
ty::Ref(..) => true,
Expand All @@ -835,24 +834,21 @@ fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKi

def.variants()
.iter()
.filter_map(|variant| transparent_newtype_field(cx.tcx, variant))
.any(|field| ty_is_known_nonnull(cx, field.ty(tcx, args), mode))
.filter_map(|variant| transparent_newtype_field(tcx, variant))
.any(|field| ty_is_known_nonnull(tcx, field.ty(tcx, args), mode))
}
_ => false,
}
}

/// Given a non-null scalar (or transparent) type `ty`, return the nullable version of that type.
/// If the type passed in was not scalar, returns None.
fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
let tcx = cx.tcx;
fn get_nullable_type<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
Some(match *ty.kind() {
ty::Adt(field_def, field_args) => {
let inner_field_ty = {
let mut first_non_zst_ty = field_def
.variants()
.iter()
.filter_map(|v| transparent_newtype_field(cx.tcx, v));
let mut first_non_zst_ty =
field_def.variants().iter().filter_map(|v| transparent_newtype_field(tcx, v));
debug_assert_eq!(
first_non_zst_ty.clone().count(),
1,
Expand All @@ -863,7 +859,7 @@ fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t
.expect("No non-zst fields in transparent type.")
.ty(tcx, field_args)
};
return get_nullable_type(cx, inner_field_ty);
return get_nullable_type(tcx, inner_field_ty);
}
ty::Int(ty) => Ty::new_int(tcx, ty),
ty::Uint(ty) => Ty::new_uint(tcx, ty),
Expand Down Expand Up @@ -895,43 +891,44 @@ fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t
/// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes.
/// FIXME: This duplicates code in codegen.
pub(crate) fn repr_nullable_ptr<'tcx>(
cx: &LateContext<'tcx>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
ckind: CItemKind,
) -> Option<Ty<'tcx>> {
debug!("is_repr_nullable_ptr(cx, ty = {:?})", ty);
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
if let ty::Adt(ty_def, args) = ty.kind() {
let field_ty = match &ty_def.variants().raw[..] {
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
([], [field]) | ([field], []) => field.ty(cx.tcx, args),
([], [field]) | ([field], []) => field.ty(tcx, args),
_ => return None,
},
_ => return None,
};

if !ty_is_known_nonnull(cx, field_ty, ckind) {
if !ty_is_known_nonnull(tcx, field_ty, ckind) {
return None;
}

// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
// If the computed size for the field and the enum are different, the nonnull optimization isn't
// being applied (and we've got a problem somewhere).
let compute_size_skeleton = |t| SizeSkeleton::compute(t, cx.tcx, cx.param_env).unwrap();
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, param_env).unwrap();
if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) {
bug!("improper_ctypes: Option nonnull optimization not applied?");
}

// Return the nullable type this Option-like enum can be safely represented with.
let field_ty_abi = &cx.layout_of(field_ty).unwrap().abi;
let field_ty_abi = &tcx.layout_of(param_env.and(field_ty)).unwrap().abi;
if let Abi::Scalar(field_ty_scalar) = field_ty_abi {
match field_ty_scalar.valid_range(cx) {
match field_ty_scalar.valid_range(&tcx) {
WrappingRange { start: 0, end }
if end == field_ty_scalar.size(&cx.tcx).unsigned_int_max() - 1 =>
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
{
return Some(get_nullable_type(cx, field_ty).unwrap());
return Some(get_nullable_type(tcx, field_ty).unwrap());
}
WrappingRange { start: 1, .. } => {
return Some(get_nullable_type(cx, field_ty).unwrap());
return Some(get_nullable_type(tcx, field_ty).unwrap());
}
WrappingRange { start, end } => {
unreachable!("Unhandled start and end range: ({}, {})", start, end)
Expand Down Expand Up @@ -1116,7 +1113,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none()
{
// Special-case types like `Option<extern fn()>`.
if repr_nullable_ptr(self.cx, ty, self.mode).is_none() {
if repr_nullable_ptr(self.cx.tcx, self.cx.param_env, ty, self.mode)
.is_none()
{
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_enum_repr_reason,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,11 @@ rustc_queries! {
separate_provide_extern
}

/// Lint against `extern fn` declarations having incompatible types.
query clashing_extern_declarations(_: ()) {
desc { "checking `extern fn` declarations are compatible" }
}

/// Identifies the entry-point (e.g., the `main` function) for a given
/// crate, returning `None` if there is no entry point (such as for library crates).
query entry_fn(_: ()) -> Option<(DefId, EntryFnType)> {
Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{HirId, Path};
use rustc_interface::interface;
use rustc_lint::{late_lint_mod, MissingDoc};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
use rustc_session::config::{self, CrateType, ErrorOutputType, ResolveDocLinks};
Expand Down Expand Up @@ -262,8 +263,9 @@ pub(crate) fn create_config(
parse_sess_created: None,
register_lints: Some(Box::new(crate::lint::register_lints)),
override_queries: Some(|_sess, providers, _external_providers| {
// We do not register late module lints, so this only runs `MissingDoc`.
// Most lints will require typechecking, so just don't run them.
providers.lint_mod = |_, _| {};
providers.lint_mod = |tcx, module_def_id| late_lint_mod(tcx, module_def_id, MissingDoc);
// hack so that `used_trait_imports` won't try to call typeck
providers.used_trait_imports = |_, _| {
static EMPTY_SET: LazyLock<UnordSet<LocalDefId>> = LazyLock::new(UnordSet::default);
Expand Down Expand Up @@ -317,9 +319,7 @@ pub(crate) fn run_global_ctxt(
tcx.hir().for_each_module(|module| tcx.ensure().check_mod_item_types(module))
});
tcx.sess.abort_if_errors();
tcx.sess.time("missing_docs", || {
rustc_lint::check_crate(tcx, rustc_lint::builtin::MissingDoc::new);
});
tcx.sess.time("missing_docs", || rustc_lint::check_crate(tcx));
tcx.sess.time("check_mod_attrs", || {
tcx.hir().for_each_module(|module| tcx.ensure().check_mod_attrs(module))
});
Expand Down
Loading

0 comments on commit 67626b8

Please sign in to comment.