Skip to content

Commit

Permalink
Detect unused struct impls pub trait
Browse files Browse the repository at this point in the history
  • Loading branch information
mu001999 committed Feb 28, 2024
1 parent ef32456 commit 895dd41
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 10 deletions.
96 changes: 94 additions & 2 deletions compiler/rustc_passes/src/dead.rs
Expand Up @@ -15,7 +15,7 @@ use rustc_hir::{Node, PatKind, TyKind};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::privacy::Level;
use rustc_middle::query::Providers;
use rustc_middle::ty::{self, TyCtxt, Visibility};
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::lint;
use rustc_session::lint::builtin::DEAD_CODE;
use rustc_span::symbol::{sym, Symbol};
Expand Down Expand Up @@ -45,6 +45,18 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
)
}

fn ty_ref_to_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> bool {
if let TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
&& let Res::Def(def_kind, def_id) = path.res
&& def_id.is_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
tcx.visibility(def_id).is_public()
} else {
true
}
}

/// Determine if a work from the worklist is coming from the a `#[allow]`
/// or a `#[expect]` of `dead_code`
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
Expand Down Expand Up @@ -415,6 +427,13 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
&& let ItemKind::Impl(impl_ref) =
self.tcx.hir().expect_item(local_impl_id).kind
{
if self.tcx.visibility(trait_id).is_public()
&& matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
&& !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty)
{
continue;
}

// mark self_ty live
intravisit::walk_ty(self, impl_ref.self_ty);
if let Some(&impl_item_id) =
Expand Down Expand Up @@ -683,12 +702,23 @@ fn check_item<'tcx>(
.iter()
.filter_map(|def_id| def_id.as_local());

let ty_is_pub = ty_ref_to_pub_struct(tcx, tcx.hir().item(id).expect_impl().self_ty);

// And we access the Map here to get HirId from LocalDefId
for id in local_def_ids {
// check the function may construct Self
let mut may_construct_self = true;
if let Some(hir_id) = tcx.opt_local_def_id_to_hir_id(id)
&& let Some(fn_sig) = tcx.hir().fn_sig_by_hir_id(hir_id)
{
may_construct_self =
matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None);
}

// for impl trait blocks, mark associate functions live if the trait is public
if of_trait
&& (!matches!(tcx.def_kind(id), DefKind::AssocFn)
|| tcx.local_visibility(id) == Visibility::Public)
|| tcx.visibility(id).is_public() && (ty_is_pub || may_construct_self))
{
worklist.push((id, ComesFromAllowExpect::No));
} else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, id) {
Expand All @@ -712,6 +742,49 @@ fn check_item<'tcx>(
}
}

fn check_pub_impl_fns_of_constructed_self<'tcx>(
tcx: TyCtxt<'tcx>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
id: hir::ItemId,
live_symbols: &UnordSet<LocalDefId>,
) {
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id);
if let Some(comes_from_allow) = allow_dead_code {
worklist.push((id.owner_id.def_id, comes_from_allow));
}

match tcx.def_kind(id.owner_id) {
DefKind::Impl { of_trait: true } => {
let mut self_is_constructed = false;
if let TyKind::Path(hir::QPath::Resolved(_, path)) =
tcx.hir().item(id).expect_impl().self_ty.kind
&& let Res::Def(def_kind, def_id) = path.res
&& let Some(local_def_id) = def_id.as_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
self_is_constructed = live_symbols.contains(&local_def_id);
}

// get DefIds from another query
let local_def_ids = tcx
.associated_item_def_ids(id.owner_id)
.iter()
.filter_map(|def_id| def_id.as_local());

// And we access the Map here to get HirId from LocalDefId
for id in local_def_ids {
// for impl trait blocks, mark associate functions live if the trait is public
if !matches!(tcx.def_kind(id), DefKind::AssocFn)
|| (tcx.visibility(id).is_public() && self_is_constructed)
{
worklist.push((id, ComesFromAllowExpect::No));
}
}
}
_ => {}
}
}

fn check_trait_item(
tcx: TyCtxt<'_>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
Expand Down Expand Up @@ -778,6 +851,20 @@ fn create_and_seed_worklist(
(worklist, struct_constructors)
}

fn create_worklist_for_pub_impl_fns_of_constructed_self(
tcx: TyCtxt<'_>,
live_symbols: &UnordSet<LocalDefId>,
) -> Vec<(LocalDefId, ComesFromAllowExpect)> {
let mut worklist = Vec::new();

let crate_items = tcx.hir_crate_items(());
for id in crate_items.items() {
check_pub_impl_fns_of_constructed_self(tcx, &mut worklist, id, live_symbols);
}

worklist
}

fn live_symbols_and_ignored_derived_traits(
tcx: TyCtxt<'_>,
(): (),
Expand All @@ -796,6 +883,11 @@ fn live_symbols_and_ignored_derived_traits(
ignored_derived_traits: Default::default(),
};
symbol_visitor.mark_live_symbols();

symbol_visitor.worklist =
create_worklist_for_pub_impl_fns_of_constructed_self(tcx, &symbol_visitor.live_symbols);
symbol_visitor.mark_live_symbols();

(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
}

Expand Down
Expand Up @@ -13,7 +13,7 @@ LL | field3: (),
LL | field4: (),
| ^^^^^^
|
= note: `Whatever` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis
= note: `Whatever` has derived impls for the traits `Debug` and `Debug`, but these are intentionally ignored during dead code analysis
note: the lint level is defined here
--> $DIR/clone-debug-dead-code-in-the-same-struct.rs:1:11
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-33187.rs
@@ -1,4 +1,4 @@
//@ run-pass
//@ check-pass

struct Foo<A: Repr>(<A as Repr>::Data);

Expand Down
2 changes: 0 additions & 2 deletions tests/ui/lint/dead-code/issue-41883.stderr
Expand Up @@ -29,8 +29,6 @@ error: struct `UnusedStruct` is never constructed
|
LL | struct UnusedStruct;
| ^^^^^^^^^^^^
|
= note: `UnusedStruct` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis

error: aborting due to 4 previous errors

Expand Up @@ -56,8 +56,6 @@ warning: struct `Foo` is never constructed
|
LL | struct Foo(usize, #[allow(unused)] usize);
| ^^^
|
= note: `Foo` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis

error: aborting due to 2 previous errors; 2 warnings emitted

17 changes: 17 additions & 0 deletions tests/ui/lint/dead-code/unused-adt-impls-pub-trait.rs
@@ -0,0 +1,17 @@
#![deny(dead_code)]

enum Foo {} //~ ERROR enum `Foo` is never used

impl Clone for Foo {
fn clone(&self) -> Foo { loop {} }
}

pub trait PubTrait {
fn unused_method(&self) -> Self;
}

impl PubTrait for Foo {
fn unused_method(&self) -> Foo { loop {} }
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/lint/dead-code/unused-adt-impls-pub-trait.stderr
@@ -0,0 +1,14 @@
error: enum `Foo` is never used
--> $DIR/unused-adt-impls-pub-trait.rs:3:6
|
LL | enum Foo {}
| ^^^
|
note: the lint level is defined here
--> $DIR/unused-adt-impls-pub-trait.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^

error: aborting due to 1 previous error

2 changes: 1 addition & 1 deletion tests/ui/rust-2018/uniform-paths/issue-55779.rs
@@ -1,4 +1,4 @@
//@ run-pass
//@ check-pass
//@ edition:2018
//@ aux-crate:issue_55779_extern_trait=issue-55779-extern-trait.rs

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/augmented-assignments-trait.rs
@@ -1,4 +1,4 @@
//@ run-pass
//@ check-pass
use std::ops::AddAssign;

struct Int(#[allow(dead_code)] i32);
Expand Down

0 comments on commit 895dd41

Please sign in to comment.