Skip to content

Commit 2fe716e

Browse files
committed
Implement lint unconstructible_pub_struct
1 parent 9312cd6 commit 2fe716e

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
@@ -575,6 +575,10 @@ passes_trait_impl_const_stable =
575575
passes_transparent_incompatible =
576576
transparent {$target} cannot have other repr hints
577577
578+
passes_unconstructable_pub_struct =
579+
pub struct `{$name}` is unconstructable 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: 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)]
@@ -382,6 +420,34 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
382420
ControlFlow::Continue(())
383421
}
384422

423+
fn mark_live_symbols_until_unsolved_items_fixed(
424+
&mut self,
425+
unsolved_items: &mut Vec<LocalDefId>,
426+
) -> Result<(), ErrorGuaranteed> {
427+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
428+
return Err(guar);
429+
}
430+
431+
// We have marked the primary seeds as live. We now need to process unsolved items from traits
432+
// and trait impls: add them to the work list if the trait or the implemented type is live.
433+
let mut items_to_check: Vec<_> = unsolved_items
434+
.extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
435+
.collect();
436+
437+
while !items_to_check.is_empty() {
438+
self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
439+
if let ControlFlow::Break(guar) = self.mark_live_symbols() {
440+
return Err(guar);
441+
}
442+
443+
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
444+
self.check_impl_or_impl_item_live(local_def_id)
445+
}));
446+
}
447+
448+
Ok(())
449+
}
450+
385451
/// Automatically generated items marked with `rustc_trivial_field_reads`
386452
/// will be ignored for the purposes of dead code analysis (see PR #85200
387453
/// for discussion).
@@ -501,9 +567,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
501567
(self.tcx.local_parent(local_def_id), trait_item_id)
502568
}
503569
// 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-
}
570+
DefKind::Impl { of_trait } => (
571+
local_def_id,
572+
of_trait
573+
.then(|| self.tcx.impl_trait_id(local_def_id))
574+
.and_then(|did| did.as_local()),
575+
),
507576
_ => bug!(),
508577
};
509578

@@ -708,6 +777,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
708777
}
709778
}
710779

780+
fn has_allow_unconstructable_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
781+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
782+
let lint_level = tcx.lint_level_at_node(UNCONSTRUCTABLE_PUB_STRUCT, hir_id).level;
783+
matches!(lint_level, lint::Allow | lint::Expect)
784+
}
785+
711786
fn has_allow_dead_code_or_lang_attr(
712787
tcx: TyCtxt<'_>,
713788
def_id: LocalDefId,
@@ -767,7 +842,9 @@ fn maybe_record_as_seed<'tcx>(
767842
unsolved_items: &mut Vec<LocalDefId>,
768843
) {
769844
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 {
845+
if let Some(comes_from_allow) = allow_dead_code
846+
&& !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
847+
{
771848
worklist.push((owner_id.def_id, comes_from_allow));
772849
}
773850

@@ -831,6 +908,33 @@ fn create_and_seed_worklist(
831908
effective_vis
832909
.is_public_at_level(Level::Reachable)
833910
.then_some(id)
911+
.filter(|id| {
912+
let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
913+
DefKind::Impl { .. } => (false, true),
914+
DefKind::AssocFn => (
915+
!matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
916+
|| method_has_no_receiver(tcx, *id),
917+
true,
918+
),
919+
DefKind::Struct => (
920+
has_allow_unconstructable_pub_struct(tcx, *id)
921+
|| struct_can_be_constructed_directly(tcx, *id),
922+
false,
923+
),
924+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
925+
has_allow_unconstructable_pub_struct(tcx, tcx.local_parent(*id))
926+
|| struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
927+
false,
928+
),
929+
_ => (true, false),
930+
};
931+
932+
if !is_seed && is_impl_or_impl_item {
933+
unsolved_impl_item.push(*id);
934+
}
935+
936+
is_seed
937+
})
834938
.map(|id| (id, ComesFromAllowExpect::No))
835939
})
836940
// Seed entry point
@@ -851,7 +955,7 @@ fn create_and_seed_worklist(
851955
fn live_symbols_and_ignored_derived_traits(
852956
tcx: TyCtxt<'_>,
853957
(): (),
854-
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
958+
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet), ErrorGuaranteed> {
855959
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
856960
let mut symbol_visitor = MarkSymbolVisitor {
857961
worklist,
@@ -865,32 +969,33 @@ fn live_symbols_and_ignored_derived_traits(
865969
ignore_variant_stack: vec![],
866970
ignored_derived_traits: Default::default(),
867971
};
868-
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
869-
return Err(guar);
870-
}
972+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
871973

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();
974+
let reachable_items =
975+
tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| {
976+
effective_vis.is_public_at_level(Level::Reachable).then_some(id)
977+
});
879978

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);
979+
let mut unstructurable_pub_structs = LocalDefIdSet::default();
980+
for id in reachable_items {
981+
if symbol_visitor.live_symbols.contains(&id) {
982+
continue;
886983
}
887984

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-
}));
985+
if matches!(tcx.def_kind(id), DefKind::Struct) {
986+
unstructurable_pub_structs.insert(id);
987+
}
988+
989+
symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
891990
}
892991

893-
Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
992+
symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items)?;
993+
994+
Ok((
995+
symbol_visitor.live_symbols,
996+
symbol_visitor.ignored_derived_traits,
997+
unstructurable_pub_structs,
998+
))
894999
}
8951000

8961001
struct DeadItem {
@@ -1170,7 +1275,7 @@ impl<'tcx> DeadVisitor<'tcx> {
11701275
}
11711276

11721277
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1173-
let Ok((live_symbols, ignored_derived_traits)) =
1278+
let Ok((live_symbols, ignored_derived_traits, unstructurable_pub_structs)) =
11741279
tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
11751280
else {
11761281
return;
@@ -1262,6 +1367,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
12621367
for foreign_item in module_items.foreign_items() {
12631368
visitor.check_definition(foreign_item.owner_id.def_id);
12641369
}
1370+
1371+
for item in module_items.free_items() {
1372+
let def_id = item.owner_id.def_id;
1373+
1374+
if !unstructurable_pub_structs.contains(&def_id) {
1375+
continue;
1376+
}
1377+
1378+
let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
1379+
continue;
1380+
};
1381+
1382+
if name.as_str().starts_with('_') {
1383+
continue;
1384+
}
1385+
1386+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1387+
let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
1388+
let diag = UnconstructablePubStruct { name, vis_span };
1389+
tcx.emit_node_span_lint(UNCONSTRUCTABLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
1390+
}
12651391
}
12661392

12671393
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_unconstructable_pub_struct)]
1254+
pub(crate) struct UnconstructablePubStruct {
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)