Skip to content

Commit 5a08af0

Browse files
committed
Detect pub structs never constructed
1 parent 21e6de7 commit 5a08af0

File tree

4 files changed

+123
-22
lines changed

4 files changed

+123
-22
lines changed

compiler/rustc_passes/src/dead.rs

+70-22
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_hir::{Node, PatKind, TyKind};
1515
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1616
use rustc_middle::middle::privacy::Level;
1717
use rustc_middle::query::Providers;
18-
use rustc_middle::ty::{self, TyCtxt};
18+
use rustc_middle::ty::{self, AssocItemContainer, TyCtxt};
1919
use rustc_middle::{bug, span_bug};
2020
use rustc_session::lint;
2121
use rustc_session::lint::builtin::DEAD_CODE;
@@ -44,16 +44,38 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
4444
)
4545
}
4646

47+
fn struct_all_fields_are_public(tcx: TyCtxt<'_>, id: DefId) -> bool {
48+
tcx.adt_def(id).all_fields().all(|field| field.vis.is_public())
49+
}
50+
51+
/// pub struct whose fields are all public
52+
fn ty_ref_to_pure_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> bool {
53+
if let TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
54+
&& let Res::Def(def_kind, def_id) = path.res
55+
&& def_id.is_local()
56+
{
57+
return match def_kind {
58+
DefKind::Enum | DefKind::Union => tcx.visibility(def_id).is_public(),
59+
DefKind::Struct => {
60+
tcx.visibility(def_id).is_public() && struct_all_fields_are_public(tcx, def_id)
61+
}
62+
_ => true,
63+
};
64+
}
65+
66+
true
67+
}
68+
69+
/// pub struct but may have private fields
4770
fn ty_ref_to_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> bool {
4871
if let TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
4972
&& let Res::Def(def_kind, def_id) = path.res
5073
&& def_id.is_local()
51-
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
74+
&& matches!(def_kind, DefKind::Enum | DefKind::Union | DefKind::Struct)
5275
{
53-
tcx.visibility(def_id).is_public()
54-
} else {
55-
true
76+
return tcx.visibility(def_id).is_public();
5677
}
78+
true
5779
}
5880

5981
/// Determine if a work from the worklist is coming from the a `#[allow]`
@@ -426,10 +448,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
426448
self.tcx.hir().expect_item(local_impl_id).kind
427449
{
428450
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
429-
&& !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty)
451+
&& !ty_ref_to_pure_pub_struct(self.tcx, impl_ref.self_ty)
430452
{
431-
// skip methods of private ty,
432-
// they would be solved in `solve_rest_impl_items`
453+
// skip impl-items of non pure pub ty,
454+
// cause we don't know the ty is constructed or not,
455+
// check these later in `solve_rest_impl_items`
433456
continue;
434457
}
435458

@@ -510,22 +533,21 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
510533
&& let Some(local_def_id) = def_id.as_local()
511534
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
512535
{
513-
if self.tcx.visibility(impl_item_id).is_public() {
514-
// for the public method, we don't know the trait item is used or not,
515-
// so we mark the method live if the self is used
516-
return self.live_symbols.contains(&local_def_id);
517-
}
518-
519536
if let Some(trait_item_id) = self.tcx.associated_item(impl_item_id).trait_item_def_id
520537
&& let Some(local_id) = trait_item_id.as_local()
521538
{
522-
// for the private method, we can know the trait item is used or not,
539+
// for the local impl item, we can know the trait item is used or not,
523540
// so we mark the method live if the self is used and the trait item is used
524-
return self.live_symbols.contains(&local_id)
525-
&& self.live_symbols.contains(&local_def_id);
541+
self.live_symbols.contains(&local_id) && self.live_symbols.contains(&local_def_id)
542+
} else {
543+
// for the foreign method and inherent pub method,
544+
// we don't know the trait item or the method is used or not,
545+
// so we mark the method live if the self is used
546+
self.live_symbols.contains(&local_def_id)
526547
}
548+
} else {
549+
false
527550
}
528-
false
529551
}
530552
}
531553

@@ -747,7 +769,8 @@ fn check_item<'tcx>(
747769
.iter()
748770
.filter_map(|def_id| def_id.as_local());
749771

750-
let ty_is_pub = ty_ref_to_pub_struct(tcx, tcx.hir().item(id).expect_impl().self_ty);
772+
let self_ty = tcx.hir().item(id).expect_impl().self_ty;
773+
let ty_is_pure_pub = ty_ref_to_pure_pub_struct(tcx, self_ty);
751774

752775
// And we access the Map here to get HirId from LocalDefId
753776
for local_def_id in local_def_ids {
@@ -766,15 +789,29 @@ fn check_item<'tcx>(
766789
if of_trait
767790
&& (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn)
768791
|| tcx.visibility(local_def_id).is_public()
769-
&& (ty_is_pub || may_construct_self))
792+
&& (ty_is_pure_pub || may_construct_self))
793+
{
794+
// if the impl item is public,
795+
// and the ty may be constructed or can be constructed in foreign crates,
796+
// mark the impl item live
797+
worklist.push((local_def_id, ComesFromAllowExpect::No));
798+
} else if !of_trait
799+
&& tcx.visibility(local_def_id).is_public()
800+
&& (ty_is_pure_pub || may_construct_self)
770801
{
802+
// if the impl item is public,
803+
// and the ty may be constructed or can be constructed in foreign crates,
804+
// mark the impl item live
771805
worklist.push((local_def_id, ComesFromAllowExpect::No));
772806
} else if let Some(comes_from_allow) =
773807
has_allow_dead_code_or_lang_attr(tcx, local_def_id)
774808
{
775809
worklist.push((local_def_id, comes_from_allow));
776-
} else if of_trait {
777-
// private method || public method not constructs self
810+
} else if of_trait
811+
|| tcx.visibility(local_def_id).is_public()
812+
&& ty_ref_to_pub_struct(tcx, self_ty)
813+
{
814+
// private impl items of traits || public impl items not constructs self
778815
unsolved_impl_items.push((id, local_def_id));
779816
}
780817
}
@@ -841,6 +878,14 @@ fn create_and_seed_worklist(
841878
effective_vis
842879
.is_public_at_level(Level::Reachable)
843880
.then_some(id)
881+
.filter(|&id|
882+
// checks impls, impl-items and pub structs with all public fields later
883+
match tcx.def_kind(id) {
884+
DefKind::Impl { .. } => false,
885+
DefKind::AssocFn => !matches!(tcx.associated_item(id).container, AssocItemContainer::ImplContainer),
886+
DefKind::Struct => struct_all_fields_are_public(tcx, id.to_def_id()),
887+
_ => true
888+
})
844889
.map(|id| (id, ComesFromAllowExpect::No))
845890
})
846891
// Seed entry point
@@ -1116,6 +1161,9 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
11161161
// We have diagnosed unused methods in traits
11171162
if matches!(def_kind, DefKind::Impl { of_trait: true })
11181163
&& tcx.def_kind(def_id) == DefKind::AssocFn
1164+
// skip unused public inherent methods,
1165+
// cause we have diagnosed unconstructed struct
1166+
|| matches!(def_kind, DefKind::Impl { of_trait: false }) && tcx.visibility(def_id).is_public() && ty_ref_to_pub_struct(tcx, tcx.hir().item(item).expect_impl().self_ty)
11191167
|| def_kind == DefKind::Trait && tcx.def_kind(def_id) != DefKind::AssocFn
11201168
{
11211169
continue;

library/core/src/clone.rs

+2
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ pub macro Clone($item:item) {
184184
//
185185
// These structs should never appear in user code.
186186
#[doc(hidden)]
187+
#[allow(dead_code)]
187188
#[allow(missing_debug_implementations)]
188189
#[unstable(
189190
feature = "derive_clone_copy",
@@ -194,6 +195,7 @@ pub struct AssertParamIsClone<T: Clone + ?Sized> {
194195
_field: crate::marker::PhantomData<T>,
195196
}
196197
#[doc(hidden)]
198+
#[allow(dead_code)]
197199
#[allow(missing_debug_implementations)]
198200
#[unstable(
199201
feature = "derive_clone_copy",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#![deny(dead_code)]
2+
3+
pub struct NeverConstructed(()); //~ ERROR struct `NeverConstructed` is never constructed
4+
5+
impl NeverConstructed {
6+
pub fn not_construct_self(&self) {}
7+
}
8+
9+
impl Clone for NeverConstructed {
10+
fn clone(&self) -> NeverConstructed {
11+
NeverConstructed(())
12+
}
13+
}
14+
15+
pub trait Trait {
16+
fn not_construct_self(&self) {}
17+
}
18+
19+
impl Trait for NeverConstructed {}
20+
21+
pub struct Constructed(());
22+
23+
impl Constructed {
24+
pub fn construct_self() -> Self {
25+
Constructed(())
26+
}
27+
}
28+
29+
impl Clone for Constructed {
30+
fn clone(&self) -> Constructed {
31+
Constructed(())
32+
}
33+
}
34+
35+
impl Trait for Constructed {}
36+
37+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: struct `NeverConstructed` is never constructed
2+
--> $DIR/unused_pub_struct.rs:3:12
3+
|
4+
LL | pub struct NeverConstructed(());
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused_pub_struct.rs:1:9
9+
|
10+
LL | #![deny(dead_code)]
11+
| ^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

0 commit comments

Comments
 (0)