From 1299aa7c18377dfbe43095b5be4ff2c0161eae8d Mon Sep 17 00:00:00 2001 From: r0cky Date: Thu, 29 Feb 2024 14:08:01 +0800 Subject: [PATCH] Detect unused struct impls pub trait --- compiler/rustc_passes/src/dead.rs | 96 ++++++++++++++++--- tests/ui/lint/dead-code/issue-41883.stderr | 2 - ...tiple-dead-codes-in-the-same-struct.stderr | 2 - .../dead-code/unused-adt-impls-pub-trait.rs | 17 ++++ .../unused-adt-impls-pub-trait.stderr | 14 +++ .../ui/rust-2018/uniform-paths/issue-55779.rs | 2 +- .../ui/traits/augmented-assignments-trait.rs | 2 +- tests/ui/traits/issue-33187.rs | 2 +- 8 files changed, 119 insertions(+), 18 deletions(-) create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-pub-trait.rs create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-pub-trait.stderr diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index a3106856a67..cdfde2b9405 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -15,7 +15,7 @@ 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}; @@ -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)] @@ -415,6 +427,13 @@ fn visit_node(&mut self, node: Node<'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) = @@ -465,6 +484,36 @@ fn mark_as_used_if_union(&mut self, adt: ty::AdtDef<'tcx>, fields: &[hir::ExprFi } } } + + 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)); + + 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)); + } + } + + fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId) -> 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 + } + } } impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> { @@ -652,6 +701,7 @@ fn check_item<'tcx>( tcx: TyCtxt<'tcx>, worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, struct_constructors: &mut LocalDefIdMap, + unsolved_impl_items: &mut Vec<(hir::ItemId, LocalDefId)>, id: hir::ItemId, ) { let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id); @@ -683,16 +733,33 @@ 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 { + for local_def_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(local_def_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) + && (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) + || tcx.visibility(local_def_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) { - worklist.push((id, comes_from_allow)); + 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)); } } } @@ -743,9 +810,14 @@ fn check_foreign_item( fn create_and_seed_worklist( tcx: TyCtxt<'_>, -) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, LocalDefIdMap) { +) -> ( + Vec<(LocalDefId, ComesFromAllowExpect)>, + LocalDefIdMap, + Vec<(hir::ItemId, LocalDefId)>, +) { let effective_visibilities = &tcx.effective_visibilities(()); // see `MarkSymbolVisitor::struct_constructors` + let mut unsolved_impl_item = Vec::new(); let mut struct_constructors = Default::default(); let mut worklist = effective_visibilities .iter() @@ -764,7 +836,7 @@ fn create_and_seed_worklist( let crate_items = tcx.hir_crate_items(()); for id in crate_items.items() { - check_item(tcx, &mut worklist, &mut struct_constructors, id); + check_item(tcx, &mut worklist, &mut struct_constructors, &mut unsolved_impl_item, id); } for id in crate_items.trait_items() { @@ -775,14 +847,14 @@ fn create_and_seed_worklist( check_foreign_item(tcx, &mut worklist, id); } - (worklist, struct_constructors) + (worklist, struct_constructors, unsolved_impl_item) } fn live_symbols_and_ignored_derived_traits( tcx: TyCtxt<'_>, (): (), ) -> (LocalDefIdSet, LocalDefIdMap>) { - let (worklist, struct_constructors) = create_and_seed_worklist(tcx); + let (worklist, struct_constructors, unsolved_impl_items) = create_and_seed_worklist(tcx); let mut symbol_visitor = MarkSymbolVisitor { worklist, tcx, @@ -796,6 +868,8 @@ fn live_symbols_and_ignored_derived_traits( ignored_derived_traits: Default::default(), }; symbol_visitor.mark_live_symbols(); + symbol_visitor.solve_rest_impl_items(unsolved_impl_items); + (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits) } diff --git a/tests/ui/lint/dead-code/issue-41883.stderr b/tests/ui/lint/dead-code/issue-41883.stderr index cf079e4dda3..47ccef9a530 100644 --- a/tests/ui/lint/dead-code/issue-41883.stderr +++ b/tests/ui/lint/dead-code/issue-41883.stderr @@ -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 diff --git a/tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr b/tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr index b992005318f..25a7d96cb89 100644 --- a/tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr +++ b/tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr @@ -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 diff --git a/tests/ui/lint/dead-code/unused-adt-impls-pub-trait.rs b/tests/ui/lint/dead-code/unused-adt-impls-pub-trait.rs new file mode 100644 index 00000000000..10d6c728c2d --- /dev/null +++ b/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() {} diff --git a/tests/ui/lint/dead-code/unused-adt-impls-pub-trait.stderr b/tests/ui/lint/dead-code/unused-adt-impls-pub-trait.stderr new file mode 100644 index 00000000000..f0d0cc50f7d --- /dev/null +++ b/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 + diff --git a/tests/ui/rust-2018/uniform-paths/issue-55779.rs b/tests/ui/rust-2018/uniform-paths/issue-55779.rs index 246b8dd82c5..f90054e6b1d 100644 --- a/tests/ui/rust-2018/uniform-paths/issue-55779.rs +++ b/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 diff --git a/tests/ui/traits/augmented-assignments-trait.rs b/tests/ui/traits/augmented-assignments-trait.rs index 37fe42ce1c6..a0f8b39eb7a 100644 --- a/tests/ui/traits/augmented-assignments-trait.rs +++ b/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); diff --git a/tests/ui/traits/issue-33187.rs b/tests/ui/traits/issue-33187.rs index 6a039527b3b..6ae7e150c51 100644 --- a/tests/ui/traits/issue-33187.rs +++ b/tests/ui/traits/issue-33187.rs @@ -1,4 +1,4 @@ -//@ run-pass +//@ check-pass struct Foo(::Data);