Skip to content

Convert builtin "global" late lints to run per module #113734

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

Merged
merged 8 commits into from
Aug 5, 2023
Merged
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
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 @@ -268,8 +269,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 @@ -323,9 +325,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