Skip to content

Commit e48bbd3

Browse files
committed
Detect unused structs which implement private traits
1 parent c69fda7 commit e48bbd3

File tree

8 files changed

+123
-17
lines changed

8 files changed

+123
-17
lines changed

compiler/rustc_middle/src/query/mod.rs

+26
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,32 @@ rustc_queries! {
819819
desc { |tcx| "comparing impl items against trait for `{}`", tcx.def_path_str(impl_id) }
820820
}
821821

822+
/// Maps from associated items on the impl specified by `impl_id`
823+
/// to the corresponding associated item on the trait.
824+
///
825+
/// For example, with the following code
826+
///
827+
/// ```
828+
/// struct Type {}
829+
/// // DefId
830+
/// trait Trait { // trait_id
831+
/// fn f(); // trait_f
832+
/// fn g() {} // trait_g
833+
/// }
834+
///
835+
/// impl Trait for Type { // impl_id
836+
/// fn f() {} // impl_f
837+
/// fn g() {} // impl_g
838+
/// }
839+
/// ```
840+
///
841+
/// The map returned for `tcx.impl_item_trait_item_ids(impl_id)` would be
842+
///`{ impl_f: trait_f, impl_g: trait_g }`
843+
query impl_item_trait_item_ids(impl_id: DefId) -> &'tcx DefIdMap<DefId> {
844+
arena_cache
845+
desc { |tcx| "comparing impl items against trait for `{}`", tcx.def_path_str(impl_id) }
846+
}
847+
822848
/// Given `fn_def_id` of a trait or of an impl that implements a given trait:
823849
/// if `fn_def_id` is the def id of a function defined inside a trait, then it creates and returns
824850
/// the associated items that correspond to each impl trait in return position for that trait.

compiler/rustc_passes/src/dead.rs

+38-16
Original file line numberDiff line numberDiff line change
@@ -427,10 +427,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
427427
&& let ItemKind::Impl(impl_ref) =
428428
self.tcx.hir().expect_item(local_impl_id).kind
429429
{
430-
if self.tcx.visibility(trait_id).is_public()
431-
&& matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
430+
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
432431
&& !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty)
433432
{
433+
// skip methods of private ty,
434+
// they would be solved in `solve_rest_impl_items`
434435
continue;
435436
}
436437

@@ -487,32 +488,51 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
487488

488489
fn solve_rest_impl_items(&mut self, mut unsolved_impl_items: Vec<(hir::ItemId, LocalDefId)>) {
489490
let mut ready;
490-
(ready, unsolved_impl_items) = unsolved_impl_items
491-
.into_iter()
492-
.partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id));
491+
(ready, unsolved_impl_items) =
492+
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
493+
self.impl_item_with_used_self_and_trait_term(impl_id, impl_item_id)
494+
});
493495

494496
while !ready.is_empty() {
495497
self.worklist =
496498
ready.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect();
497499
self.mark_live_symbols();
498500

499-
(ready, unsolved_impl_items) = unsolved_impl_items
500-
.into_iter()
501-
.partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id));
501+
(ready, unsolved_impl_items) =
502+
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
503+
self.impl_item_with_used_self_and_trait_term(impl_id, impl_item_id)
504+
});
502505
}
503506
}
504507

505-
fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId) -> bool {
508+
fn impl_item_with_used_self_and_trait_term(
509+
&mut self,
510+
impl_id: hir::ItemId,
511+
impl_item_id: LocalDefId,
512+
) -> bool {
506513
if let TyKind::Path(hir::QPath::Resolved(_, path)) =
507514
self.tcx.hir().item(impl_id).expect_impl().self_ty.kind
508515
&& let Res::Def(def_kind, def_id) = path.res
509516
&& let Some(local_def_id) = def_id.as_local()
510517
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
511518
{
512-
self.live_symbols.contains(&local_def_id)
513-
} else {
514-
false
519+
if self.tcx.visibility(impl_item_id).is_public() {
520+
// for the public method, we don't know the trait item is used or not,
521+
// so we mark the method live if the self is used
522+
return self.live_symbols.contains(&local_def_id);
523+
} else if let Some(trait_item_id) = self
524+
.tcx
525+
.impl_item_trait_item_ids(impl_id.owner_id.to_def_id())
526+
.get(&impl_item_id.to_def_id())
527+
&& let Some(local_id) = trait_item_id.as_local()
528+
{
529+
// for the private method, we can know the trait item is ued or not,
530+
// so we mark the method live if the self is used and the trait item is used
531+
return self.live_symbols.contains(&local_id)
532+
&& self.live_symbols.contains(&local_def_id);
533+
}
515534
}
535+
false
516536
}
517537
}
518538

@@ -746,20 +766,22 @@ fn check_item<'tcx>(
746766
matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None);
747767
}
748768

749-
// for impl trait blocks, mark associate functions live if the trait is public
769+
// for trait impl blocks,
770+
// mark the method live if the self_ty is public,
771+
// or the method is public and may construct self
750772
if of_trait
751773
&& (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn)
752774
|| tcx.visibility(local_def_id).is_public()
753775
&& (ty_is_pub || may_construct_self))
754776
{
755777
worklist.push((local_def_id, ComesFromAllowExpect::No));
756-
} else if of_trait && tcx.visibility(local_def_id).is_public() {
757-
// pub method && private ty & methods not construct self
758-
unsolved_impl_items.push((id, local_def_id));
759778
} else if let Some(comes_from_allow) =
760779
has_allow_dead_code_or_lang_attr(tcx, local_def_id)
761780
{
762781
worklist.push((local_def_id, comes_from_allow));
782+
} else if of_trait {
783+
// private method || public method not constructs self
784+
unsolved_impl_items.push((id, local_def_id));
763785
}
764786
}
765787
}

compiler/rustc_ty_utils/src/assoc.rs

+8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub(crate) fn provide(providers: &mut Providers) {
1515
associated_types_for_impl_traits_in_associated_fn,
1616
associated_type_for_impl_trait_in_trait,
1717
impl_item_implementor_ids,
18+
impl_item_trait_item_ids,
1819
..*providers
1920
};
2021
}
@@ -92,6 +93,13 @@ fn impl_item_implementor_ids(tcx: TyCtxt<'_>, impl_id: DefId) -> DefIdMap<DefId>
9293
.collect()
9394
}
9495

96+
fn impl_item_trait_item_ids(tcx: TyCtxt<'_>, impl_id: DefId) -> DefIdMap<DefId> {
97+
tcx.associated_items(impl_id)
98+
.in_definition_order()
99+
.filter_map(|item| item.trait_item_def_id.map(|trait_item| (item.def_id, trait_item)))
100+
.collect()
101+
}
102+
95103
fn associated_item(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AssocItem {
96104
let id = tcx.local_def_id_to_hir_id(def_id);
97105
let parent_def_id = tcx.hir().get_parent_item(id);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#![deny(dead_code)]
2+
3+
struct Used;
4+
struct Unused; //~ ERROR struct `Unused` is never constructed
5+
6+
pub trait PubTrait {
7+
fn foo(&self) -> Self;
8+
}
9+
10+
impl PubTrait for Used {
11+
fn foo(&self) -> Self { Used }
12+
}
13+
14+
impl PubTrait for Unused {
15+
fn foo(&self) -> Self { Unused }
16+
}
17+
18+
trait PriTrait {
19+
fn foo(&self) -> Self;
20+
}
21+
22+
impl PriTrait for Used {
23+
fn foo(&self) -> Self { Used }
24+
}
25+
26+
impl PriTrait for Unused {
27+
fn foo(&self) -> Self { Unused }
28+
}
29+
30+
fn main() {
31+
let t = Used;
32+
let _t = <Used as PubTrait>::foo(&t);
33+
let _t = <Used as PriTrait>::foo(&t);
34+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: struct `Unused` is never constructed
2+
--> $DIR/unused-adt-impls-trait.rs:4:8
3+
|
4+
LL | struct Unused;
5+
| ^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused-adt-impls-trait.rs:1:9
9+
|
10+
LL | #![deny(dead_code)]
11+
| ^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

tests/ui/rust-2021/inherent-dyn-collision.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ mod inner {
2525
// having a struct of the same name as the trait in-scope, while *also*
2626
// implementing the trait for that struct but **without** importing the
2727
// trait itself into scope
28+
#[allow(dead_code)]
2829
struct TryIntoU32;
2930

3031
impl super::TryIntoU32 for TryIntoU32 {

tests/ui/rust-2021/inherent-dyn-collision.rs

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ mod inner {
2525
// having a struct of the same name as the trait in-scope, while *also*
2626
// implementing the trait for that struct but **without** importing the
2727
// trait itself into scope
28+
#[allow(dead_code)]
2829
struct TryIntoU32;
2930

3031
impl super::TryIntoU32 for TryIntoU32 {

tests/ui/rust-2021/inherent-dyn-collision.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: trait method `try_into` will become ambiguous in Rust 2021
2-
--> $DIR/inherent-dyn-collision.rs:41:9
2+
--> $DIR/inherent-dyn-collision.rs:42:9
33
|
44
LL | get_dyn_trait().try_into().unwrap()
55
| ^^^^^^^^^^^^^^^ help: disambiguate the method call: `(&*get_dyn_trait())`

0 commit comments

Comments
 (0)