From 96968350e17efdbb9958dbeaec4982d8cca0019d Mon Sep 17 00:00:00 2001 From: r0cky Date: Thu, 23 May 2024 09:06:43 +0800 Subject: [PATCH] Detect unused structs which implement private traits --- compiler/rustc_passes/src/dead.rs | 49 +++++++++++++------ .../lint/dead-code/unused-adt-impls-trait.rs | 34 +++++++++++++ .../dead-code/unused-adt-impls-trait.stderr | 14 ++++++ .../ui/rust-2021/inherent-dyn-collision.fixed | 1 + tests/ui/rust-2021/inherent-dyn-collision.rs | 1 + .../rust-2021/inherent-dyn-collision.stderr | 2 +- 6 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-trait.rs create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-trait.stderr diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 933412f677ce8..ddc50e2b811a2 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -425,10 +425,11 @@ 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(..)) + if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) && !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty) { + // skip methods of private ty, + // they would be solved in `solve_rest_impl_items` continue; } @@ -485,32 +486,46 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { fn solve_rest_impl_items(&mut self, mut unsolved_impl_items: Vec<(hir::ItemId, LocalDefId)>) { let mut ready; - (ready, unsolved_impl_items) = unsolved_impl_items - .into_iter() - .partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id)); + (ready, unsolved_impl_items) = + unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| { + self.impl_item_with_used_self(impl_id, impl_item_id) + }); while !ready.is_empty() { self.worklist = ready.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect(); self.mark_live_symbols(); - (ready, unsolved_impl_items) = unsolved_impl_items - .into_iter() - .partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id)); + (ready, unsolved_impl_items) = + unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| { + self.impl_item_with_used_self(impl_id, impl_item_id) + }); } } - fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId) -> bool { + fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId, impl_item_id: LocalDefId) -> bool { if let TyKind::Path(hir::QPath::Resolved(_, path)) = self.tcx.hir().item(impl_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.live_symbols.contains(&local_def_id) - } else { - false + if self.tcx.visibility(impl_item_id).is_public() { + // for the public method, we don't know the trait item is used or not, + // so we mark the method live if the self is used + return self.live_symbols.contains(&local_def_id); + } + + if let Some(trait_item_id) = self.tcx.associated_item(impl_item_id).trait_item_def_id + && let Some(local_id) = trait_item_id.as_local() + { + // for the private method, we can know the trait item is used or not, + // so we mark the method live if the self is used and the trait item is used + return self.live_symbols.contains(&local_id) + && self.live_symbols.contains(&local_def_id); + } } + false } } @@ -745,20 +760,22 @@ fn check_item<'tcx>( matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None); } - // for impl trait blocks, mark associate functions live if the trait is public + // for trait impl blocks, + // mark the method live if the self_ty is public, + // or the method is public and may construct self if of_trait && (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) || tcx.visibility(local_def_id).is_public() && (ty_is_pub || may_construct_self)) { worklist.push((local_def_id, ComesFromAllowExpect::No)); - } else if of_trait && tcx.visibility(local_def_id).is_public() { - // pub method && private ty & methods not construct self - unsolved_impl_items.push((id, local_def_id)); } else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, local_def_id) { worklist.push((local_def_id, comes_from_allow)); + } else if of_trait { + // private method || public method not constructs self + unsolved_impl_items.push((id, local_def_id)); } } } diff --git a/tests/ui/lint/dead-code/unused-adt-impls-trait.rs b/tests/ui/lint/dead-code/unused-adt-impls-trait.rs new file mode 100644 index 0000000000000..4714859afac4b --- /dev/null +++ b/tests/ui/lint/dead-code/unused-adt-impls-trait.rs @@ -0,0 +1,34 @@ +#![deny(dead_code)] + +struct Used; +struct Unused; //~ ERROR struct `Unused` is never constructed + +pub trait PubTrait { + fn foo(&self) -> Self; +} + +impl PubTrait for Used { + fn foo(&self) -> Self { Used } +} + +impl PubTrait for Unused { + fn foo(&self) -> Self { Unused } +} + +trait PriTrait { + fn foo(&self) -> Self; +} + +impl PriTrait for Used { + fn foo(&self) -> Self { Used } +} + +impl PriTrait for Unused { + fn foo(&self) -> Self { Unused } +} + +fn main() { + let t = Used; + let _t = ::foo(&t); + let _t = ::foo(&t); +} diff --git a/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr b/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr new file mode 100644 index 0000000000000..28bae5c2af09c --- /dev/null +++ b/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr @@ -0,0 +1,14 @@ +error: struct `Unused` is never constructed + --> $DIR/unused-adt-impls-trait.rs:4:8 + | +LL | struct Unused; + | ^^^^^^ + | +note: the lint level is defined here + --> $DIR/unused-adt-impls-trait.rs:1:9 + | +LL | #![deny(dead_code)] + | ^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/rust-2021/inherent-dyn-collision.fixed b/tests/ui/rust-2021/inherent-dyn-collision.fixed index f5702613af032..8fe1bf811dbf3 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.fixed +++ b/tests/ui/rust-2021/inherent-dyn-collision.fixed @@ -25,6 +25,7 @@ mod inner { // having a struct of the same name as the trait in-scope, while *also* // implementing the trait for that struct but **without** importing the // trait itself into scope + #[allow(dead_code)] struct TryIntoU32; impl super::TryIntoU32 for TryIntoU32 { diff --git a/tests/ui/rust-2021/inherent-dyn-collision.rs b/tests/ui/rust-2021/inherent-dyn-collision.rs index 0bcb34e5708bb..47c8446241988 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.rs +++ b/tests/ui/rust-2021/inherent-dyn-collision.rs @@ -25,6 +25,7 @@ mod inner { // having a struct of the same name as the trait in-scope, while *also* // implementing the trait for that struct but **without** importing the // trait itself into scope + #[allow(dead_code)] struct TryIntoU32; impl super::TryIntoU32 for TryIntoU32 { diff --git a/tests/ui/rust-2021/inherent-dyn-collision.stderr b/tests/ui/rust-2021/inherent-dyn-collision.stderr index f5905574ac39d..d9e720dd9af8f 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.stderr +++ b/tests/ui/rust-2021/inherent-dyn-collision.stderr @@ -1,5 +1,5 @@ warning: trait method `try_into` will become ambiguous in Rust 2021 - --> $DIR/inherent-dyn-collision.rs:41:9 + --> $DIR/inherent-dyn-collision.rs:42:9 | LL | get_dyn_trait().try_into().unwrap() | ^^^^^^^^^^^^^^^ help: disambiguate the method call: `(&*get_dyn_trait())`