Skip to content

Commit 28f82d0

Browse files
committed
Implement lint unconstructible_pub_struct
1 parent a80d39a commit 28f82d0

File tree

7 files changed

+315
-29
lines changed

7 files changed

+315
-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+
UNCONSTRUCTABLE_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 `unconstructable_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(unconstructable_pub_struct)]
770+
///
771+
/// pub struct Foo(i32);
772+
/// # fn main() {}
773+
/// ```
774+
///
775+
/// {{produces}}
776+
///
777+
/// ### Explanation
778+
///
779+
/// Unconstructable 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 UNCONSTRUCTABLE_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
@@ -586,6 +586,10 @@ passes_trait_impl_const_stable =
586586
passes_transparent_incompatible =
587587
transparent {$target} cannot have other repr hints
588588
589+
passes_unconstructable_pub_struct =
590+
pub struct `{$name}` is unconstructable externally and never constructed locally
591+
.help = this struct may be unused locally and also externally, consider removing it
592+
589593
passes_unexportable_adt_with_private_fields = ADT types with private fields are not exportable
590594
.note = `{$field_name}` is private
591595

compiler/rustc_passes/src/dead.rs

Lines changed: 155 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, UNCONSTRUCTABLE_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, UnconstructablePubStruct,
28+
UselessAssignment,
2829
};
2930

3031
/// Any local definition that may call something in its body block should be explored. For example,
@@ -67,6 +68,43 @@ 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+
// Such types often declared in Rust but constructed by FFI, so ignore
75+
if adt_def.repr().c() || adt_def.repr().transparent() {
76+
return true;
77+
}
78+
79+
// Skip types contain fields of unit and never type,
80+
// it's usually intentional to make the type not constructible
81+
if adt_def.all_fields().any(|field| {
82+
let field_type = tcx.type_of(field.did).instantiate_identity();
83+
field_type.is_unit() || field_type.is_never()
84+
}) {
85+
return true;
86+
}
87+
88+
adt_def.all_fields().all(|field| {
89+
let field_type = tcx.type_of(field.did).instantiate_identity();
90+
// Skip fields of PhantomData,
91+
// cause it's a common way to check things like well-formedness
92+
if field_type.is_phantom_data() {
93+
return true;
94+
}
95+
96+
field.vis.is_public()
97+
})
98+
}
99+
100+
fn method_has_no_receiver(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
101+
if let Some(fn_decl) = tcx.hir_fn_decl_by_hir_id(tcx.local_def_id_to_hir_id(id)) {
102+
!fn_decl.implicit_self.has_implicit_self()
103+
} else {
104+
true
105+
}
106+
}
107+
70108
/// Determine if a work from the worklist is coming from a `#[allow]`
71109
/// or a `#[expect]` of `dead_code`
72110
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
@@ -372,6 +410,34 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
372410
ControlFlow::Continue(())
373411
}
374412

413+
fn mark_live_symbols_until_unsolved_items_fixed(
414+
&mut self,
415+
unsolved_items: &mut Vec<LocalDefId>,
416+
) -> Result<(), ErrorGuaranteed> {
417+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
418+
return Err(guar);
419+
}
420+
421+
// We have marked the primary seeds as live. We now need to process unsolved items from traits
422+
// and trait impls: add them to the work list if the trait or the implemented type is live.
423+
let mut items_to_check: Vec<_> = unsolved_items
424+
.extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
425+
.collect();
426+
427+
while !items_to_check.is_empty() {
428+
self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
429+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
430+
return Err(guar);
431+
}
432+
433+
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
434+
self.check_impl_or_impl_item_live(local_def_id)
435+
}));
436+
}
437+
438+
Ok(())
439+
}
440+
375441
/// Automatically generated items marked with `rustc_trivial_field_reads`
376442
/// will be ignored for the purposes of dead code analysis (see PR #85200
377443
/// for discussion).
@@ -491,9 +557,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
491557
(self.tcx.local_parent(local_def_id), trait_item_id)
492558
}
493559
// impl items are live if the corresponding traits are live
494-
DefKind::Impl { of_trait: true } => {
495-
(local_def_id, self.tcx.impl_trait_id(local_def_id).as_local())
496-
}
560+
DefKind::Impl { of_trait } => (
561+
local_def_id,
562+
of_trait
563+
.then(|| self.tcx.impl_trait_id(local_def_id))
564+
.and_then(|did| did.as_local()),
565+
),
497566
_ => bug!(),
498567
};
499568

@@ -698,6 +767,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
698767
}
699768
}
700769

770+
fn has_allow_unconstructable_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
771+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
772+
let lint_level = tcx.lint_level_at_node(UNCONSTRUCTABLE_PUB_STRUCT, hir_id).level;
773+
matches!(lint_level, lint::Allow | lint::Expect)
774+
}
775+
701776
fn has_allow_dead_code_or_lang_attr(
702777
tcx: TyCtxt<'_>,
703778
def_id: LocalDefId,
@@ -757,7 +832,9 @@ fn maybe_record_as_seed<'tcx>(
757832
unsolved_items: &mut Vec<LocalDefId>,
758833
) {
759834
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id);
760-
if let Some(comes_from_allow) = allow_dead_code {
835+
if let Some(comes_from_allow) = allow_dead_code
836+
&& !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
837+
{
761838
worklist.push((owner_id.def_id, comes_from_allow));
762839
}
763840

@@ -821,6 +898,33 @@ fn create_and_seed_worklist(
821898
effective_vis
822899
.is_public_at_level(Level::Reachable)
823900
.then_some(id)
901+
.filter(|id| {
902+
let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
903+
DefKind::Impl { .. } => (false, true),
904+
DefKind::AssocFn => (
905+
!matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
906+
|| method_has_no_receiver(tcx, *id),
907+
true,
908+
),
909+
DefKind::Struct => (
910+
has_allow_unconstructable_pub_struct(tcx, *id)
911+
|| struct_can_be_constructed_directly(tcx, *id),
912+
false,
913+
),
914+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
915+
has_allow_unconstructable_pub_struct(tcx, tcx.local_parent(*id))
916+
|| struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
917+
false,
918+
),
919+
_ => (true, false),
920+
};
921+
922+
if !is_seed && is_impl_or_impl_item {
923+
unsolved_impl_item.push(*id);
924+
}
925+
926+
is_seed
927+
})
824928
.map(|id| (id, ComesFromAllowExpect::No))
825929
})
826930
// Seed entry point
@@ -841,7 +945,7 @@ fn create_and_seed_worklist(
841945
fn live_symbols_and_ignored_derived_traits(
842946
tcx: TyCtxt<'_>,
843947
(): (),
844-
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
948+
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet), ErrorGuaranteed> {
845949
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
846950
let mut symbol_visitor = MarkSymbolVisitor {
847951
worklist,
@@ -855,32 +959,33 @@ fn live_symbols_and_ignored_derived_traits(
855959
ignore_variant_stack: vec![],
856960
ignored_derived_traits: Default::default(),
857961
};
858-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
859-
return Err(guar);
860-
}
962+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
861963

862-
// We have marked the primary seeds as live. We now need to process unsolved items from traits
863-
// and trait impls: add them to the work list if the trait or the implemented type is live.
864-
let mut items_to_check: Vec<_> = unsolved_items
865-
.extract_if(.., |&mut local_def_id| {
866-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
867-
})
868-
.collect();
964+
let reachable_items =
965+
tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| {
966+
effective_vis.is_public_at_level(Level::Reachable).then_some(id)
967+
});
869968

870-
while !items_to_check.is_empty() {
871-
symbol_visitor
872-
.worklist
873-
.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
874-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
875-
return Err(guar);
969+
let mut unstructurable_pub_structs = LocalDefIdSet::default();
970+
for id in reachable_items {
971+
if symbol_visitor.live_symbols.contains(&id) {
972+
continue;
876973
}
877974

878-
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
879-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
880-
}));
975+
if matches!(tcx.def_kind(id), DefKind::Struct) {
976+
unstructurable_pub_structs.insert(id);
977+
}
978+
979+
symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
881980
}
882981

883-
Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
982+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
983+
984+
Ok((
985+
symbol_visitor.live_symbols,
986+
symbol_visitor.ignored_derived_traits,
987+
unstructurable_pub_structs,
988+
))
884989
}
885990

886991
struct DeadItem {
@@ -1160,7 +1265,7 @@ impl<'tcx> DeadVisitor<'tcx> {
11601265
}
11611266

11621267
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1163-
let Ok((live_symbols, ignored_derived_traits)) =
1268+
let Ok((live_symbols, ignored_derived_traits, unstructurable_pub_structs)) =
11641269
tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
11651270
else {
11661271
return;
@@ -1252,6 +1357,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
12521357
for foreign_item in module_items.foreign_items() {
12531358
visitor.check_definition(foreign_item.owner_id.def_id);
12541359
}
1360+
1361+
for item in module_items.free_items() {
1362+
let def_id = item.owner_id.def_id;
1363+
1364+
if !unstructurable_pub_structs.contains(&def_id) {
1365+
continue;
1366+
}
1367+
1368+
let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
1369+
continue;
1370+
};
1371+
1372+
if name.as_str().starts_with('_') {
1373+
continue;
1374+
}
1375+
1376+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1377+
let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
1378+
let diag = UnconstructablePubStruct { name, vis_span };
1379+
tcx.emit_node_span_lint(UNCONSTRUCTABLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
1380+
}
12551381
}
12561382

12571383
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
@@ -1261,6 +1261,14 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
12611261
},
12621262
}
12631263

1264+
#[derive(LintDiagnostic)]
1265+
#[diag(passes_unconstructable_pub_struct)]
1266+
pub(crate) struct UnconstructablePubStruct {
1267+
pub name: Symbol,
1268+
#[help]
1269+
pub vis_span: Span,
1270+
}
1271+
12641272
#[derive(Subdiagnostic)]
12651273
#[note(passes_enum_variant_same_name)]
12661274
pub(crate) struct EnumVariantSameName<'tcx> {

0 commit comments

Comments
 (0)