Skip to content

Commit f4930ea

Browse files
committed
Implement lint unconstructible_pub_struct
1 parent 9312cd6 commit f4930ea

File tree

7 files changed

+310
-29
lines changed

7 files changed

+310
-29
lines changed

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ declare_lint_pass! {
112112
TYVAR_BEHIND_RAW_POINTER,
113113
UNCONDITIONAL_PANIC,
114114
UNCONDITIONAL_RECURSION,
115+
UNCONSTRUCTIBLE_PUB_STRUCT,
115116
UNCOVERED_PARAM_IN_PROJECTION,
116117
UNEXPECTED_CFGS,
117118
UNFULFILLED_LINT_EXPECTATIONS,
@@ -758,6 +759,38 @@ declare_lint! {
758759
"detect unused, unexported items"
759760
}
760761

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

compiler/rustc_middle/src/query/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,7 @@ rustc_queries! {
12211221
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx Result<(
12221222
LocalDefIdSet,
12231223
LocalDefIdMap<FxIndexSet<DefId>>,
1224+
LocalDefIdSet,
12241225
), ErrorGuaranteed> {
12251226
arena_cache
12261227
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
@@ -575,6 +575,10 @@ passes_trait_impl_const_stable =
575575
passes_transparent_incompatible =
576576
transparent {$target} cannot have other repr hints
577577
578+
passes_unconstructible_pub_struct =
579+
pub struct `{$name}` is unconstructible externally and never constructed locally
580+
.help = this struct may be unused locally and also externally, consider removing it
581+
578582
passes_unexportable_adt_with_private_fields = ADT types with private fields are not exportable
579583
.note = `{$field_name}` is private
580584

compiler/rustc_passes/src/dead.rs

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

2626
use crate::errors::{
27-
ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment,
27+
ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UnconstructiblePubStruct,
28+
UselessAssignment,
2829
};
2930

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

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

418+
fn mark_live_symbols_until_unsolved_items_fixed(
419+
&mut self,
420+
unsolved_items: &mut Vec<LocalDefId>,
421+
) -> Result<(), ErrorGuaranteed> {
422+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
423+
return Err(guar);
424+
}
425+
426+
// We have marked the primary seeds as live. We now need to process unsolved items from traits
427+
// and trait impls: add them to the work list if the trait or the implemented type is live.
428+
let mut items_to_check: Vec<_> = unsolved_items
429+
.extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
430+
.collect();
431+
432+
while !items_to_check.is_empty() {
433+
self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
434+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
435+
return Err(guar);
436+
}
437+
438+
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
439+
self.check_impl_or_impl_item_live(local_def_id)
440+
}));
441+
}
442+
443+
Ok(())
444+
}
445+
385446
/// Automatically generated items marked with `rustc_trivial_field_reads`
386447
/// will be ignored for the purposes of dead code analysis (see PR #85200
387448
/// for discussion).
@@ -501,9 +562,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
501562
(self.tcx.local_parent(local_def_id), trait_item_id)
502563
}
503564
// impl items are live if the corresponding traits are live
504-
DefKind::Impl { of_trait: true } => {
505-
(local_def_id, self.tcx.impl_trait_id(local_def_id).as_local())
506-
}
565+
DefKind::Impl { of_trait } => (
566+
local_def_id,
567+
of_trait
568+
.then(|| self.tcx.impl_trait_id(local_def_id))
569+
.and_then(|did| did.as_local()),
570+
),
507571
_ => bug!(),
508572
};
509573

@@ -708,6 +772,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
708772
}
709773
}
710774

775+
fn has_allow_unconstructible_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
776+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
777+
let lint_level = tcx.lint_level_at_node(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id).level;
778+
matches!(lint_level, lint::Allow | lint::Expect)
779+
}
780+
711781
fn has_allow_dead_code_or_lang_attr(
712782
tcx: TyCtxt<'_>,
713783
def_id: LocalDefId,
@@ -767,7 +837,9 @@ fn maybe_record_as_seed<'tcx>(
767837
unsolved_items: &mut Vec<LocalDefId>,
768838
) {
769839
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id);
770-
if let Some(comes_from_allow) = allow_dead_code {
840+
if let Some(comes_from_allow) = allow_dead_code
841+
&& !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
842+
{
771843
worklist.push((owner_id.def_id, comes_from_allow));
772844
}
773845

@@ -831,6 +903,33 @@ fn create_and_seed_worklist(
831903
effective_vis
832904
.is_public_at_level(Level::Reachable)
833905
.then_some(id)
906+
.filter(|id| {
907+
let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
908+
DefKind::Impl { .. } => (false, true),
909+
DefKind::AssocFn => (
910+
!matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
911+
|| method_has_no_receiver(tcx, *id),
912+
true,
913+
),
914+
DefKind::Struct => (
915+
has_allow_unconstructible_pub_struct(tcx, *id)
916+
|| struct_can_be_constructed_directly(tcx, *id),
917+
false,
918+
),
919+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
920+
has_allow_unconstructible_pub_struct(tcx, tcx.local_parent(*id))
921+
|| struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
922+
false,
923+
),
924+
_ => (true, false),
925+
};
926+
927+
if !is_seed && is_impl_or_impl_item {
928+
unsolved_impl_item.push(*id);
929+
}
930+
931+
is_seed
932+
})
834933
.map(|id| (id, ComesFromAllowExpect::No))
835934
})
836935
// Seed entry point
@@ -851,7 +950,7 @@ fn create_and_seed_worklist(
851950
fn live_symbols_and_ignored_derived_traits(
852951
tcx: TyCtxt<'_>,
853952
(): (),
854-
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
953+
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet), ErrorGuaranteed> {
855954
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
856955
let mut symbol_visitor = MarkSymbolVisitor {
857956
worklist,
@@ -865,32 +964,33 @@ fn live_symbols_and_ignored_derived_traits(
865964
ignore_variant_stack: vec![],
866965
ignored_derived_traits: Default::default(),
867966
};
868-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
869-
return Err(guar);
870-
}
967+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
871968

872-
// We have marked the primary seeds as live. We now need to process unsolved items from traits
873-
// and trait impls: add them to the work list if the trait or the implemented type is live.
874-
let mut items_to_check: Vec<_> = unsolved_items
875-
.extract_if(.., |&mut local_def_id| {
876-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
877-
})
878-
.collect();
969+
let reachable_items =
970+
tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| {
971+
effective_vis.is_public_at_level(Level::Reachable).then_some(id)
972+
});
879973

880-
while !items_to_check.is_empty() {
881-
symbol_visitor
882-
.worklist
883-
.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
884-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
885-
return Err(guar);
974+
let mut unstructurable_pub_structs = LocalDefIdSet::default();
975+
for id in reachable_items {
976+
if symbol_visitor.live_symbols.contains(&id) {
977+
continue;
886978
}
887979

888-
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
889-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
890-
}));
980+
if matches!(tcx.def_kind(id), DefKind::Struct) {
981+
unstructurable_pub_structs.insert(id);
982+
}
983+
984+
symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
891985
}
892986

893-
Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
987+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
988+
989+
Ok((
990+
symbol_visitor.live_symbols,
991+
symbol_visitor.ignored_derived_traits,
992+
unstructurable_pub_structs,
993+
))
894994
}
895995

896996
struct DeadItem {
@@ -1170,7 +1270,7 @@ impl<'tcx> DeadVisitor<'tcx> {
11701270
}
11711271

11721272
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1173-
let Ok((live_symbols, ignored_derived_traits)) =
1273+
let Ok((live_symbols, ignored_derived_traits, unstructurable_pub_structs)) =
11741274
tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
11751275
else {
11761276
return;
@@ -1262,6 +1362,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
12621362
for foreign_item in module_items.foreign_items() {
12631363
visitor.check_definition(foreign_item.owner_id.def_id);
12641364
}
1365+
1366+
for item in module_items.free_items() {
1367+
let def_id = item.owner_id.def_id;
1368+
1369+
if !unstructurable_pub_structs.contains(&def_id) {
1370+
continue;
1371+
}
1372+
1373+
let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
1374+
continue;
1375+
};
1376+
1377+
if name.as_str().starts_with('_') {
1378+
continue;
1379+
}
1380+
1381+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1382+
let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
1383+
let diag = UnconstructiblePubStruct { name, vis_span };
1384+
tcx.emit_node_span_lint(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
1385+
}
12651386
}
12661387

12671388
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
@@ -1249,6 +1249,14 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
12491249
},
12501250
}
12511251

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

0 commit comments

Comments
 (0)