Auto merge of rust-lang#113734 - cjgillot:no-crate-lint, r=petrochenkov

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 rust-lang#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`.

Author: bors

Diff for: compiler/rustc_interface/src/passes.rs

@@ -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(());
                     }
                 );
             },

Diff for: compiler/rustc_lint/src/builtin.rs

@@ -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;
@@ -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,
@@ -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,
@@ -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>>(
@@ -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);
             });
         },
         || {

Diff for: compiler/rustc_lint/src/foreign_modules.rs

@@ -1,4 +1,5 @@
 use crate::{
+    builtin::MISSING_DOCS,
     context::{CheckLintNameResult, LintStore},
     fluent_generated as fluent,
     late::unerased_lint_store,
@@ -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

Diff for: compiler/rustc_lint/src/late.rs

@@ -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;
@@ -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};
@@ -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!(
@@ -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,
     [
@@ -253,6 +236,8 @@ late_lint_methods!(
             OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
             MultipleSupertraitUpcastable: MultipleSupertraitUpcastable,
             MapUnitFn: MapUnitFn,
+            MissingDebugImplementations: MissingDebugImplementations,
+            MissingDoc: MissingDoc,
         ]
     ]
 );
@@ -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",
@@ -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

Diff for: compiler/rustc_lint/src/levels.rs

@@ -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,
@@ -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,
@@ -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),
@@ -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)
@@ -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,

Diff for: compiler/rustc_lint/src/lib.rs

@@ -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)> {

Diff for: compiler/rustc_lint/src/types.rs

@@ -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};
@@ -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);
@@ -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))
     });