Skip to content

Commit 95a90eb

Browse files
committed
Implement lint unconstructible_pub_struct
1 parent 377a931 commit 95a90eb

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
@@ -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
@@ -1205,6 +1205,7 @@ rustc_queries! {
12051205
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx Result<(
12061206
LocalDefIdSet,
12071207
LocalDefIdMap<FxIndexSet<DefId>>,
1208+
LocalDefIdSet,
12081209
), ErrorGuaranteed> {
12091210
arena_cache
12101211
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: 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)]
@@ -373,6 +406,34 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
373406
ControlFlow::Continue(())
374407
}
375408

409+
fn mark_live_symbols_until_unsolved_items_fixed(
410+
&mut self,
411+
unsolved_items: &mut Vec<LocalDefId>,
412+
) -> Result<(), ErrorGuaranteed> {
413+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
414+
return Err(guar);
415+
}
416+
417+
// We have marked the primary seeds as live. We now need to process unsolved items from traits
418+
// and trait impls: add them to the work list if the trait or the implemented type is live.
419+
let mut items_to_check: Vec<_> = unsolved_items
420+
.extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
421+
.collect();
422+
423+
while !items_to_check.is_empty() {
424+
self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
425+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
426+
return Err(guar);
427+
}
428+
429+
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
430+
self.check_impl_or_impl_item_live(local_def_id)
431+
}));
432+
}
433+
434+
Ok(())
435+
}
436+
376437
/// Automatically generated items marked with `rustc_trivial_field_reads`
377438
/// will be ignored for the purposes of dead code analysis (see PR #85200
378439
/// for discussion).
@@ -492,9 +553,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
492553
(self.tcx.local_parent(local_def_id), trait_item_id)
493554
}
494555
// impl items are live if the corresponding traits are live
495-
DefKind::Impl { of_trait: true } => {
496-
(local_def_id, self.tcx.impl_trait_id(local_def_id).as_local())
497-
}
556+
DefKind::Impl { of_trait } => (
557+
local_def_id,
558+
of_trait
559+
.then(|| self.tcx.impl_trait_id(local_def_id))
560+
.and_then(|did| did.as_local()),
561+
),
498562
_ => bug!(),
499563
};
500564

@@ -699,6 +763,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
699763
}
700764
}
701765

766+
fn has_allow_unconstructible_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
767+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
768+
let lint_level = tcx.lint_level_at_node(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id).level;
769+
matches!(lint_level, lint::Allow | lint::Expect)
770+
}
771+
702772
fn has_allow_dead_code_or_lang_attr(
703773
tcx: TyCtxt<'_>,
704774
def_id: LocalDefId,
@@ -758,7 +828,9 @@ fn maybe_record_as_seed<'tcx>(
758828
unsolved_items: &mut Vec<LocalDefId>,
759829
) {
760830
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id);
761-
if let Some(comes_from_allow) = allow_dead_code {
831+
if let Some(comes_from_allow) = allow_dead_code
832+
&& !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
833+
{
762834
worklist.push((owner_id.def_id, comes_from_allow));
763835
}
764836

@@ -822,6 +894,33 @@ fn create_and_seed_worklist(
822894
effective_vis
823895
.is_public_at_level(Level::Reachable)
824896
.then_some(id)
897+
.filter(|id| {
898+
let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
899+
DefKind::Impl { .. } => (false, true),
900+
DefKind::AssocFn => (
901+
!matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
902+
|| method_has_no_receiver(tcx, *id),
903+
true,
904+
),
905+
DefKind::Struct => (
906+
has_allow_unconstructible_pub_struct(tcx, *id)
907+
|| struct_can_be_constructed_directly(tcx, *id),
908+
false,
909+
),
910+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
911+
has_allow_unconstructible_pub_struct(tcx, tcx.local_parent(*id))
912+
|| struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
913+
false,
914+
),
915+
_ => (true, false),
916+
};
917+
918+
if !is_seed && is_impl_or_impl_item {
919+
unsolved_impl_item.push(*id);
920+
}
921+
922+
is_seed
923+
})
825924
.map(|id| (id, ComesFromAllowExpect::No))
826925
})
827926
// Seed entry point
@@ -842,7 +941,7 @@ fn create_and_seed_worklist(
842941
fn live_symbols_and_ignored_derived_traits(
843942
tcx: TyCtxt<'_>,
844943
(): (),
845-
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
944+
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet), ErrorGuaranteed> {
846945
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
847946
let mut symbol_visitor = MarkSymbolVisitor {
848947
worklist,
@@ -856,32 +955,33 @@ fn live_symbols_and_ignored_derived_traits(
856955
ignore_variant_stack: vec![],
857956
ignored_derived_traits: Default::default(),
858957
};
859-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
860-
return Err(guar);
861-
}
958+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
862959

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

871-
while !items_to_check.is_empty() {
872-
symbol_visitor
873-
.worklist
874-
.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
875-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
876-
return Err(guar);
965+
let mut unstructurable_pub_structs = LocalDefIdSet::default();
966+
for id in reachable_items {
967+
if symbol_visitor.live_symbols.contains(&id) {
968+
continue;
877969
}
878970

879-
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
880-
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
881-
}));
971+
if matches!(tcx.def_kind(id), DefKind::Struct) {
972+
unstructurable_pub_structs.insert(id);
973+
}
974+
975+
symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
882976
}
883977

884-
Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
978+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
979+
980+
Ok((
981+
symbol_visitor.live_symbols,
982+
symbol_visitor.ignored_derived_traits,
983+
unstructurable_pub_structs,
984+
))
885985
}
886986

887987
struct DeadItem {
@@ -1161,7 +1261,7 @@ impl<'tcx> DeadVisitor<'tcx> {
11611261
}
11621262

11631263
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1164-
let Ok((live_symbols, ignored_derived_traits)) =
1264+
let Ok((live_symbols, ignored_derived_traits, unstructurable_pub_structs)) =
11651265
tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
11661266
else {
11671267
return;
@@ -1253,6 +1353,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
12531353
for foreign_item in module_items.foreign_items() {
12541354
visitor.check_definition(foreign_item.owner_id.def_id);
12551355
}
1356+
1357+
for item in module_items.free_items() {
1358+
let def_id = item.owner_id.def_id;
1359+
1360+
if !unstructurable_pub_structs.contains(&def_id) {
1361+
continue;
1362+
}
1363+
1364+
let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
1365+
continue;
1366+
};
1367+
1368+
if name.as_str().starts_with('_') {
1369+
continue;
1370+
}
1371+
1372+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1373+
let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
1374+
let diag = UnconstructiblePubStruct { name, vis_span };
1375+
tcx.emit_node_span_lint(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
1376+
}
12561377
}
12571378

12581379
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)