Skip to content

Commit 095d1e9

Browse files
committed
Implement lint unconstructible_pub_struct
1 parent ab1d244 commit 095d1e9

File tree

7 files changed

+302
-26
lines changed

7 files changed

+302
-26
lines changed

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ declare_lint_pass! {
110110
TYVAR_BEHIND_RAW_POINTER,
111111
UNCONDITIONAL_PANIC,
112112
UNCONDITIONAL_RECURSION,
113+
UNCONSTRUCTIBLE_PUB_STRUCT,
113114
UNCOVERED_PARAM_IN_PROJECTION,
114115
UNEXPECTED_CFGS,
115116
UNFULFILLED_LINT_EXPECTATIONS,
@@ -755,6 +756,38 @@ declare_lint! {
755756
"detect unused, unexported items"
756757
}
757758

759+
declare_lint! {
760+
/// The `unconstructible_pub_struct` lint detects public structs that
761+
/// are unused locally and cannot be constructed externally.
762+
///
763+
/// ### Example
764+
///
765+
/// ```rust,compile_fail
766+
/// #![deny(unconstructible_pub_struct)]
767+
///
768+
/// pub struct Foo(i32);
769+
/// # fn main() {}
770+
/// ```
771+
///
772+
/// {{produces}}
773+
///
774+
/// ### Explanation
775+
///
776+
/// Unconstructible pub structs may signal a mistake or unfinished code.
777+
/// To silence the warning for individual items, prefix the name with an
778+
/// underscore such as `_Foo`.
779+
///
780+
/// To preserve this lint, add a field with unit or never types that
781+
/// indicate that the behavior is intentional, or use `PhantomData` as
782+
/// field types if the struct is only used at the type level to check
783+
/// things like well-formedness.
784+
///
785+
/// Otherwise, consider removing it if the struct is no longer in use.
786+
pub UNCONSTRUCTIBLE_PUB_STRUCT,
787+
Allow,
788+
"detects pub structs that are unused locally and cannot be constructed externally"
789+
}
790+
758791
declare_lint! {
759792
/// The `unused_attributes` lint detects attributes that were not used by
760793
/// the compiler.

compiler/rustc_middle/src/query/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,6 +1204,7 @@ rustc_queries! {
12041204
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
12051205
LocalDefIdSet,
12061206
LocalDefIdMap<FxIndexSet<DefId>>,
1207+
LocalDefIdSet,
12071208
) {
12081209
arena_cache
12091210
desc { "finding live symbols in crate" }

compiler/rustc_passes/messages.ftl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,10 @@ passes_trait_impl_const_stable =
576576
passes_transparent_incompatible =
577577
transparent {$target} cannot have other repr hints
578578
579+
passes_unconstructible_pub_struct =
580+
pub struct `{$name}` is unconstructible externally and never constructed locally
581+
.help = this struct may be unused locally and also externally, consider removing it
582+
579583
passes_unexportable_adt_with_private_fields = ADT types with private fields are not exportable
580584
.note = `{$field_name}` is private
581585

compiler/rustc_passes/src/dead.rs

Lines changed: 142 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
99
use rustc_abi::FieldIdx;
1010
use rustc_data_structures::fx::FxIndexSet;
1111
use rustc_errors::MultiSpan;
12-
use rustc_hir::def::{CtorOf, DefKind, Res};
12+
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
1313
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
1414
use rustc_hir::intravisit::{self, Visitor};
1515
use rustc_hir::{self as hir, Node, PatKind, QPath};
@@ -18,12 +18,13 @@ use rustc_middle::middle::privacy::Level;
1818
use rustc_middle::query::Providers;
1919
use rustc_middle::ty::{self, AssocTag, TyCtxt};
2020
use rustc_middle::{bug, span_bug};
21-
use rustc_session::lint::builtin::DEAD_CODE;
21+
use rustc_session::lint::builtin::{DEAD_CODE, UNCONSTRUCTIBLE_PUB_STRUCT};
2222
use rustc_session::lint::{self, LintExpectationId};
2323
use rustc_span::{Symbol, kw, sym};
2424

2525
use crate::errors::{
26-
ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment,
26+
ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UnconstructiblePubStruct,
27+
UselessAssignment,
2728
};
2829

2930
/// Any local definition that may call something in its body block should be explored. For example,
@@ -66,6 +67,38 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
6667
}
6768
}
6869

70+
fn struct_can_be_constructed_directly(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
71+
let adt_def = tcx.adt_def(id);
72+
73+
// Skip types contain fields of unit and never type,
74+
// it's usually intentional to make the type not constructible
75+
if adt_def.all_fields().any(|field| {
76+
let field_type = tcx.type_of(field.did).instantiate_identity();
77+
field_type.is_unit() || field_type.is_never()
78+
}) {
79+
return true;
80+
}
81+
82+
return adt_def.all_fields().all(|field| {
83+
let field_type = tcx.type_of(field.did).instantiate_identity();
84+
// Skip fields of PhantomData,
85+
// cause it's a common way to check things like well-formedness
86+
if field_type.is_phantom_data() {
87+
return true;
88+
}
89+
90+
field.vis.is_public()
91+
});
92+
}
93+
94+
fn method_has_no_receiver(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
95+
if let Some(fn_decl) = tcx.hir_fn_decl_by_hir_id(tcx.local_def_id_to_hir_id(id)) {
96+
!fn_decl.implicit_self.has_implicit_self()
97+
} else {
98+
true
99+
}
100+
}
101+
69102
/// Determine if a work from the worklist is coming from a `#[allow]`
70103
/// or a `#[expect]` of `dead_code`
71104
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
@@ -370,6 +403,28 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
370403
}
371404
}
372405

406+
fn mark_live_symbols_until_unsolved_items_fixed(
407+
&mut self,
408+
unsolved_items: &mut Vec<LocalDefId>,
409+
) {
410+
self.mark_live_symbols();
411+
412+
// We have marked the primary seeds as live. We now need to process unsolved items from traits
413+
// and trait impls: add them to the work list if the trait or the implemented type is live.
414+
let mut items_to_check: Vec<_> = unsolved_items
415+
.extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
416+
.collect();
417+
418+
while !items_to_check.is_empty() {
419+
self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
420+
self.mark_live_symbols();
421+
422+
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
423+
self.check_impl_or_impl_item_live(local_def_id)
424+
}));
425+
}
426+
}
427+
373428
/// Automatically generated items marked with `rustc_trivial_field_reads`
374429
/// will be ignored for the purposes of dead code analysis (see PR #85200
375430
/// for discussion).
@@ -486,9 +541,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
486541
(self.tcx.local_parent(local_def_id), trait_item_id)
487542
}
488543
// impl items are live if the corresponding traits are live
489-
DefKind::Impl { of_trait: true } => {
490-
(local_def_id, self.tcx.impl_trait_id(local_def_id).as_local())
491-
}
544+
DefKind::Impl { of_trait } => (
545+
local_def_id,
546+
of_trait
547+
.then(|| self.tcx.impl_trait_id(local_def_id))
548+
.and_then(|did| did.as_local()),
549+
),
492550
_ => bug!(),
493551
};
494552

@@ -675,6 +733,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
675733
}
676734
}
677735

736+
fn has_allow_unconstructible_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
737+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
738+
let lint_level = tcx.lint_level_at_node(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id).level;
739+
matches!(lint_level, lint::Allow | lint::Expect)
740+
}
741+
678742
fn has_allow_dead_code_or_lang_attr(
679743
tcx: TyCtxt<'_>,
680744
def_id: LocalDefId,
@@ -734,7 +798,9 @@ fn maybe_record_as_seed<'tcx>(
734798
unsolved_items: &mut Vec<LocalDefId>,
735799
) {
736800
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id);
737-
if let Some(comes_from_allow) = allow_dead_code {
801+
if let Some(comes_from_allow) = allow_dead_code
802+
&& !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
803+
{
738804
worklist.push((owner_id.def_id, comes_from_allow));
739805
}
740806

@@ -798,6 +864,33 @@ fn create_and_seed_worklist(
798864
effective_vis
799865
.is_public_at_level(Level::Reachable)
800866
.then_some(id)
867+
.filter(|id| {
868+
let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
869+
DefKind::Impl { .. } => (false, true),
870+
DefKind::AssocFn => (
871+
!matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
872+
|| method_has_no_receiver(tcx, *id),
873+
true,
874+
),
875+
DefKind::Struct => (
876+
has_allow_unconstructible_pub_struct(tcx, *id)
877+
|| struct_can_be_constructed_directly(tcx, *id),
878+
false,
879+
),
880+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
881+
has_allow_unconstructible_pub_struct(tcx, tcx.local_parent(*id))
882+
|| struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
883+
false,
884+
),
885+
_ => (true, false),
886+
};
887+
888+
if !is_seed && is_impl_or_impl_item {
889+
unsolved_impl_item.push(*id);
890+
}
891+
892+
is_seed
893+
})
801894
.map(|id| (id, ComesFromAllowExpect::No))
802895
})
803896
// Seed entry point
@@ -818,7 +911,7 @@ fn create_and_seed_worklist(
818911
fn live_symbols_and_ignored_derived_traits(
819912
tcx: TyCtxt<'_>,
820913
(): (),
821-
) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>) {
914+
) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet) {
822915
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
823916
let mut symbol_visitor = MarkSymbolVisitor {
824917
worklist,
@@ -832,28 +925,29 @@ fn live_symbols_and_ignored_derived_traits(
832925
ignore_variant_stack: vec![],
833926
ignored_derived_traits: Default::default(),
834927
};
835-
symbol_visitor.mark_live_symbols();
928+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items);
836929

837-
// We have marked the primary seeds as live. We now need to process unsolved items from traits
838-
// and trait impls: add them to the work list if the trait or the implemented type is live.
839-
let mut items_to_check: Vec<_> = unsolved_items
840-
.extract_if(.., |&mut local_def_id| {
841-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
842-
})
843-
.collect();
930+
let reachable_items =
931+
tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| {
932+
effective_vis.is_public_at_level(Level::Reachable).then_some(id)
933+
});
844934

845-
while !items_to_check.is_empty() {
846-
symbol_visitor
847-
.worklist
848-
.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
849-
symbol_visitor.mark_live_symbols();
935+
let mut unstructurable_pub_structs = LocalDefIdSet::default();
936+
for id in reachable_items {
937+
if symbol_visitor.live_symbols.contains(&id) {
938+
continue;
939+
}
940+
941+
if matches!(tcx.def_kind(id), DefKind::Struct) {
942+
unstructurable_pub_structs.insert(id);
943+
}
850944

851-
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
852-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
853-
}));
945+
symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
854946
}
855947

856-
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
948+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items);
949+
950+
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits, unstructurable_pub_structs)
857951
}
858952

859953
struct DeadItem {
@@ -1133,7 +1227,8 @@ impl<'tcx> DeadVisitor<'tcx> {
11331227
}
11341228

11351229
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1136-
let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(());
1230+
let (live_symbols, ignored_derived_traits, unstructurable_pub_structs) =
1231+
tcx.live_symbols_and_ignored_derived_traits(());
11371232
let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits };
11381233

11391234
let module_items = tcx.hir_module_items(module);
@@ -1220,6 +1315,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
12201315
for foreign_item in module_items.foreign_items() {
12211316
visitor.check_definition(foreign_item.owner_id.def_id);
12221317
}
1318+
1319+
for item in module_items.free_items() {
1320+
let def_id = item.owner_id.def_id;
1321+
1322+
if !unstructurable_pub_structs.contains(&def_id) {
1323+
continue;
1324+
}
1325+
1326+
let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
1327+
continue;
1328+
};
1329+
1330+
if name.as_str().starts_with('_') {
1331+
continue;
1332+
}
1333+
1334+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1335+
let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
1336+
let diag = UnconstructiblePubStruct { name, vis_span };
1337+
tcx.emit_node_span_lint(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
1338+
}
12231339
}
12241340

12251341
pub(crate) fn provide(providers: &mut Providers) {

compiler/rustc_passes/src/errors.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,14 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
12501250
},
12511251
}
12521252

1253+
#[derive(LintDiagnostic)]
1254+
#[diag(passes_unconstructible_pub_struct)]
1255+
pub(crate) struct UnconstructiblePubStruct {
1256+
pub name: Symbol,
1257+
#[help]
1258+
pub vis_span: Span,
1259+
}
1260+
12531261
#[derive(Subdiagnostic)]
12541262
#[note(passes_enum_variant_same_name)]
12551263
pub(crate) struct EnumVariantSameName<'tcx> {

0 commit comments

Comments
 (0)