Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-04-27 18:57:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #141407 - mu001999-contrib:dead-code/refactor, r=petrochenkov

Browse Source 
Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:
1. adding assoc fn and impl into `unsolved_items` directly during the initial construction of the worklist
2. converge the logic of checking whether assoc fn and impl are used to `item_should_be_checked`, and the item is considered used only when its corresponding trait and Self adt are used

This PR only refactors as much as possible to avoid affecting the original functions. However, due to the adjustment of the order of checks, the test results are slightly different, but overall, there is no regression problem

Fixes rust-lang/rust#127911
Fixes rust-lang/rust#128839

Extracted from https://github.com/rust-lang/rust/pull/128637.
r? petrochenkov

try-job: dist-aarch64-linux
This commit is contained in:
Matthias Krüger
2025-05-30 07:01:29 +02:00
committed by GitHub
parent 5fc3f26748 6eb6010368
commit 896be667b8
9 changed files with 181 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/rustc_middle/src/query/mod.rs
+1 -1
View File
@@ -1137,7 +1137,7 @@
/// their respective impl (i.e., part of the derive macro)
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
LocalDefIdSet,
LocalDefIdMap<Vec<(DefId, DefId)>>
LocalDefIdMap<FxIndexSet<(DefId, DefId)>>
) {
arena_cache
desc { "finding live symbols in crate" }
compiler/rustc_passes/src/dead.rs
+120 -130
View File
@@ -8,12 +8,13 @@
use hir::ItemKind;
use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
use rustc_abi::FieldIdx;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::MultiSpan;
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{self as hir, Node, PatKind, TyKind};
use rustc_hir::{self as hir, Node, PatKind, QPath, TyKind};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::privacy::Level;
use rustc_middle::query::Providers;
@@ -44,15 +45,20 @@ 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
/// Returns the local def id of the ADT if the given ty refers to a local one.
fn local_adt_def_of_ty<'tcx>(ty: &hir::Ty<'tcx>) -> Option<LocalDefId> {
match ty.kind {
TyKind::Path(QPath::Resolved(_, path)) => {
if 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)
{
Some(local_def_id)
} else {
None
}
}
_ => None,
}
}
@@ -78,7 +84,7 @@ struct MarkSymbolVisitor<'tcx> {
// maps from ADTs to ignored derived traits (e.g. Debug and Clone)
// and the span of their respective impl (i.e., part of the derive
// macro)
ignored_derived_traits: LocalDefIdMap<Vec<(DefId, DefId)>>,
ignored_derived_traits: LocalDefIdMap<FxIndexSet<(DefId, DefId)>>,
}
impl<'tcx> MarkSymbolVisitor<'tcx> {
@@ -360,7 +366,7 @@ fn should_ignore_item(&mut self, def_id: DefId) -> bool {
&& let Some(fn_sig) =
self.tcx.hir_fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id))
&& matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None)
&& let TyKind::Path(hir::QPath::Resolved(_, path)) =
&& let TyKind::Path(QPath::Resolved(_, path)) =
self.tcx.hir_expect_item(local_impl_of).expect_impl().self_ty.kind
&& let Res::Def(def_kind, did) = path.res
{
@@ -388,7 +394,7 @@ fn should_ignore_item(&mut self, def_id: DefId) -> bool {
self.ignored_derived_traits
.entry(adt_def_id)
.or_default()
.push((trait_of, impl_of));
.insert((trait_of, impl_of));
}
return true;
}
@@ -420,51 +426,22 @@ fn visit_node(&mut self, node: Node<'tcx>) {
intravisit::walk_item(self, item)
}
hir::ItemKind::ForeignMod { .. } => {}
hir::ItemKind::Trait(..) => {
for &impl_def_id in self.tcx.local_trait_impls(item.owner_id.def_id) {
if let ItemKind::Impl(impl_ref) = self.tcx.hir_expect_item(impl_def_id).kind
{
// skip items
// mark dependent traits live
intravisit::walk_generics(self, impl_ref.generics);
// mark dependent parameters live
intravisit::walk_path(self, impl_ref.of_trait.unwrap().path);
hir::ItemKind::Trait(.., trait_item_refs) => {
// mark assoc ty live if the trait is live
for trait_item in trait_item_refs {
if matches!(trait_item.kind, hir::AssocItemKind::Type) {
self.check_def_id(trait_item.id.owner_id.to_def_id());
}
}
intravisit::walk_item(self, item)
}
_ => intravisit::walk_item(self, item),
},
Node::TraitItem(trait_item) => {
// mark corresponding ImplTerm live
// mark the trait live
let trait_item_id = trait_item.owner_id.to_def_id();
if let Some(trait_id) = self.tcx.trait_of_item(trait_item_id) {
// mark the trait live
self.check_def_id(trait_id);
for impl_id in self.tcx.all_impls(trait_id) {
if let Some(local_impl_id) = impl_id.as_local()
&& let ItemKind::Impl(impl_ref) =
self.tcx.hir_expect_item(local_impl_id).kind
{
if !matches!(trait_item.kind, hir::TraitItemKind::Type(..))
&& !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;
}
// mark self_ty live
intravisit::walk_unambig_ty(self, impl_ref.self_ty);
if let Some(&impl_item_id) =
self.tcx.impl_item_implementor_ids(impl_id).get(&trait_item_id)
{
self.check_def_id(impl_item_id);
}
}
}
}
intravisit::walk_trait_item(self, trait_item);
}
@@ -508,48 +485,58 @@ 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, 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, impl_item_id)| {
self.impl_item_with_used_self(impl_id, impl_item_id)
});
/// Returns whether `local_def_id` is potentially alive or not.
/// `local_def_id` points to an impl or an impl item,
/// both impl and impl item that may be passed to this function are of a trait,
/// and added into the unsolved_items during `create_and_seed_worklist`
fn check_impl_or_impl_item_live(
&mut self,
impl_id: hir::ItemId,
local_def_id: LocalDefId,
) -> bool {
if self.should_ignore_item(local_def_id.to_def_id()) {
return false;
}
}
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)
{
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);
}
let trait_def_id = match self.tcx.def_kind(local_def_id) {
// assoc impl items of traits are live if the corresponding trait items are live
DefKind::AssocFn => self.tcx.associated_item(local_def_id).trait_item_def_id,
// impl items are live if the corresponding traits are live
DefKind::Impl { of_trait: true } => self
.tcx
.impl_trait_ref(impl_id.owner_id.def_id)
.and_then(|trait_ref| Some(trait_ref.skip_binder().def_id)),
_ => None,
};
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()
if let Some(trait_def_id) = trait_def_id {
if let Some(trait_def_id) = trait_def_id.as_local()
&& !self.live_symbols.contains(&trait_def_id)
{
// 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);
return false;
}
// FIXME: legacy logic to check whether the function may construct `Self`,
// this can be removed after supporting marking ADTs appearing in patterns
// as live, then we can check private impls of public traits directly
if let Some(fn_sig) =
self.tcx.hir_fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id))
&& matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None)
&& self.tcx.visibility(trait_def_id).is_public()
{
return true;
}
}
false
// The impl or impl item is used if the corresponding trait or trait item is used and the ty is used.
if let Some(local_def_id) =
local_adt_def_of_ty(self.tcx.hir_item(impl_id).expect_impl().self_ty)
&& !self.live_symbols.contains(&local_def_id)
{
return false;
}
true
}
}
@@ -584,7 +571,7 @@ fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'tcx>) {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
match expr.kind {
hir::ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(..)) => {
hir::ExprKind::Path(ref qpath @ QPath::TypeRelative(..)) => {
let res = self.typeck_results().qpath_res(qpath, expr.hir_id);
self.handle_res(res);
}
@@ -738,7 +725,7 @@ fn check_item<'tcx>(
tcx: TyCtxt<'tcx>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
struct_constructors: &mut LocalDefIdMap<LocalDefId>,
unsolved_impl_items: &mut Vec<(hir::ItemId, LocalDefId)>,
unsolved_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);
@@ -764,41 +751,33 @@ fn check_item<'tcx>(
}
}
DefKind::Impl { of_trait } => {
// 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());
if let Some(comes_from_allow) =
has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id)
{
worklist.push((id.owner_id.def_id, comes_from_allow));
} else if of_trait {
unsolved_items.push((id, id.owner_id.def_id));
}
let ty_is_pub = ty_ref_to_pub_struct(tcx, tcx.hir_item(id).expect_impl().self_ty);
for def_id in tcx.associated_item_def_ids(id.owner_id) {
let local_def_id = def_id.expect_local();
// And we access the Map here to get HirId from LocalDefId
for local_def_id in local_def_ids {
// check the function may construct Self
let mut may_construct_self = false;
if let Some(fn_sig) =
tcx.hir_fn_sig_by_hir_id(tcx.local_def_id_to_hir_id(local_def_id))
{
may_construct_self =
matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None);
}
// 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 let Some(comes_from_allow) =
has_allow_dead_code_or_lang_attr(tcx, local_def_id)
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));
// FIXME: This condition can be removed
// if we support dead check for assoc consts and tys.
if !matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) {
worklist.push((local_def_id, ComesFromAllowExpect::No));
} else {
// We only care about associated items of traits,
// because they cannot be visited directly,
// so we later mark them as live if their corresponding traits
// or trait items and self types are both live,
// but inherent associated items can be visited and marked directly.
unsolved_items.push((id, local_def_id));
}
}
}
}
@@ -892,8 +871,8 @@ fn create_and_seed_worklist(
fn live_symbols_and_ignored_derived_traits(
tcx: TyCtxt<'_>,
(): (),
) -> (LocalDefIdSet, LocalDefIdMap<Vec<(DefId, DefId)>>) {
let (worklist, struct_constructors, unsolved_impl_items) = create_and_seed_worklist(tcx);
) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<(DefId, DefId)>>) {
let (worklist, struct_constructors, mut unsolved_items) = create_and_seed_worklist(tcx);
let mut symbol_visitor = MarkSymbolVisitor {
worklist,
tcx,
@@ -907,7 +886,22 @@ 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);
let mut items_to_check;
(items_to_check, unsolved_items) =
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
});
while !items_to_check.is_empty() {
symbol_visitor.worklist =
items_to_check.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect();
symbol_visitor.mark_live_symbols();
(items_to_check, unsolved_items) =
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
});
}
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
}
@@ -921,7 +915,7 @@ struct DeadItem {
struct DeadVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
live_symbols: &'tcx LocalDefIdSet,
ignored_derived_traits: &'tcx LocalDefIdMap<Vec<(DefId, DefId)>>,
ignored_derived_traits: &'tcx LocalDefIdMap<FxIndexSet<(DefId, DefId)>>,
}
enum ShouldWarnAboutField {
@@ -1188,19 +1182,15 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
let def_kind = tcx.def_kind(item.owner_id);
let mut dead_codes = Vec::new();
// if we have diagnosed the trait, do not diagnose unused methods
if matches!(def_kind, DefKind::Impl { .. })
// Only diagnose unused assoc items in inherient impl and used trait,
// for unused assoc items in impls of trait,
// we have diagnosed them in the trait if they are unused,
// for unused assoc items in unused trait,
// we have diagnosed the unused trait.
if matches!(def_kind, DefKind::Impl { of_trait: false })
|| (def_kind == DefKind::Trait && live_symbols.contains(&item.owner_id.def_id))
{
for &def_id in tcx.associated_item_def_ids(item.owner_id.def_id) {
// We have diagnosed unused methods in traits
if matches!(def_kind, DefKind::Impl { of_trait: true })
&& tcx.def_kind(def_id) == DefKind::AssocFn
|| def_kind == DefKind::Trait && tcx.def_kind(def_id) != DefKind::AssocFn
{
continue;
}
if let Some(local_def_id) = def_id.as_local()
&& !visitor.is_live_code(local_def_id)
{
src/bootstrap/src/utils/exec.rs
+3
View File
@@ -332,16 +332,19 @@ fn default() -> Self {
/// Helper trait to format both Command and BootstrapCommand as a short execution line,
/// without all the other details (e.g. environment variables).
#[cfg(feature = "tracing")]
pub trait FormatShortCmd {
fn format_short_cmd(&self) -> String;
}
#[cfg(feature = "tracing")]
impl FormatShortCmd for BootstrapCommand {
fn format_short_cmd(&self) -> String {
self.command.format_short_cmd()
}
}
#[cfg(feature = "tracing")]
impl FormatShortCmd for Command {
fn format_short_cmd(&self) -> String {
let program = Path::new(self.get_program());
tests/ui/derives/clone-debug-dead-code.stderr
+1 -1
View File
@@ -40,7 +40,7 @@ LL | struct D { f: () }
| |
| field in this struct
|
= note: `D` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
= note: `D` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
error: field `f` is never read
--> $DIR/clone-debug-dead-code.rs:21:12
tests/ui/deriving/deriving-in-macro.rs
+2 -1
View File
@@ -1,5 +1,6 @@
//@ run-pass
//@ check-pass
#![allow(non_camel_case_types)]
#![allow(dead_code)]
macro_rules! define_vec {
() => (
tests/ui/lint/dead-code/alias-type-used-as-generic-arg-in-impl.rs
+19
View File
@@ -0,0 +1,19 @@
//@ check-pass
#![deny(dead_code)]
struct T<X>(X);
type A<X> = T<X>;
trait Tr {
fn foo();
}
impl<X> Tr for T<A<X>> {
fn foo() {}
}
fn main() {
T::<T<()>>::foo();
}
tests/ui/lint/dead-code/issue-41883.stderr
+2
View File
@@ -29,6 +29,8 @@ 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
tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr
+2
View File
@@ -56,6 +56,8 @@ 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
tests/ui/lint/dead-code/trait-only-used-as-type-bound.rs
+31
View File
@@ -0,0 +1,31 @@
//@ check-pass
#![deny(dead_code)]
trait UInt: Copy + From<u8> {}
impl UInt for u16 {}
trait Int: Copy {
type Unsigned: UInt;
fn as_unsigned(self) -> Self::Unsigned;
}
impl Int for i16 {
type Unsigned = u16;
fn as_unsigned(self) -> u16 {
self as _
}
}
fn priv_func<T: Int>(x: u8, y: T) -> (T::Unsigned, T::Unsigned) {
(T::Unsigned::from(x), y.as_unsigned())
}
pub fn pub_func(x: u8, y: i16) -> (u16, u16) {
priv_func(x, y)
}
fn main() {}