From bc177055f748d87f358fc360ba19932ca79b7c68 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 2 Aug 2025 19:19:17 +0000 Subject: [PATCH 1/9] Do not record derived impl def-id for dead code. --- compiler/rustc_middle/src/query/mod.rs | 5 ++--- compiler/rustc_passes/src/dead.rs | 23 +++++++++-------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 2941808e806f8..a45895765945f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1178,11 +1178,10 @@ rustc_queries! { /// Return the live symbols in the crate for dead code check. /// - /// The second return value maps from ADTs to ignored derived traits (e.g. Debug and Clone) and - /// their respective impl (i.e., part of the derive macro) + /// The second return value maps from ADTs to ignored derived traits (e.g. Debug and Clone). query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx ( LocalDefIdSet, - LocalDefIdMap> + LocalDefIdMap>, ) { arena_cache desc { "finding live symbols in crate" } diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index a90d1af87ca19..f8d1ef2988202 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -84,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>, + ignored_derived_traits: LocalDefIdMap>, } impl<'tcx> MarkSymbolVisitor<'tcx> { @@ -380,10 +380,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { if let ty::Adt(adt_def, _) = trait_ref.self_ty().kind() && let Some(adt_def_id) = adt_def.did().as_local() { - self.ignored_derived_traits - .entry(adt_def_id) - .or_default() - .insert((trait_of, impl_of)); + self.ignored_derived_traits.entry(adt_def_id).or_default().insert(trait_of); } return true; } @@ -881,7 +878,7 @@ fn create_and_seed_worklist( fn live_symbols_and_ignored_derived_traits( tcx: TyCtxt<'_>, (): (), -) -> (LocalDefIdSet, LocalDefIdMap>) { +) -> (LocalDefIdSet, LocalDefIdMap>) { let (worklist, struct_constructors, mut unsolved_items) = create_and_seed_worklist(tcx); let mut symbol_visitor = MarkSymbolVisitor { worklist, @@ -925,7 +922,7 @@ struct DeadItem { struct DeadVisitor<'tcx> { tcx: TyCtxt<'tcx>, live_symbols: &'tcx LocalDefIdSet, - ignored_derived_traits: &'tcx LocalDefIdMap>, + ignored_derived_traits: &'tcx LocalDefIdMap>, } enum ShouldWarnAboutField { @@ -1047,20 +1044,18 @@ impl<'tcx> DeadVisitor<'tcx> { let encl_def_id = get_parent_if_enum_variant(tcx, encl_def_id); let ignored_derived_impls = - if let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id) { + self.ignored_derived_traits.get(&encl_def_id).map(|ign_traits| { let trait_list = ign_traits .iter() - .map(|(trait_id, _)| self.tcx.item_name(*trait_id)) + .map(|trait_id| self.tcx.item_name(*trait_id)) .collect::>(); let trait_list_len = trait_list.len(); - Some(IgnoredDerivedImpls { + IgnoredDerivedImpls { name: self.tcx.item_name(encl_def_id.to_def_id()), trait_list: trait_list.into(), trait_list_len, - }) - } else { - None - }; + } + }); let enum_variants_with_same_name = dead_codes .iter() From aba0b65707f9980babb6dcc2855f32a3b21f05e7 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 2 Aug 2025 22:42:54 +0000 Subject: [PATCH 2/9] Use DefKind in should_explore. --- compiler/rustc_passes/src/dead.rs | 44 ++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index f8d1ef2988202..bf5a31f7dde9e 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -33,16 +33,40 @@ use crate::errors::{ // function, then we should explore its block to check for codes that // may need to be marked as live. fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { - matches!( - tcx.hir_node_by_def_id(def_id), - Node::Item(..) - | Node::ImplItem(..) - | Node::ForeignItem(..) - | Node::TraitItem(..) - | Node::Variant(..) - | Node::AnonConst(..) - | Node::OpaqueTy(..) - ) + match tcx.def_kind(def_id) { + DefKind::Mod + | DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::Variant + | DefKind::Trait + | DefKind::TyAlias + | DefKind::ForeignTy + | DefKind::TraitAlias + | DefKind::AssocTy + | DefKind::Fn + | DefKind::Const + | DefKind::Static { .. } + | DefKind::AssocFn + | DefKind::AssocConst + | DefKind::Macro(_) + | DefKind::GlobalAsm + | DefKind::Impl { .. } + | DefKind::OpaqueTy + | DefKind::AnonConst + | DefKind::InlineConst + | DefKind::ExternCrate + | DefKind::Use + | DefKind::ForeignMod => true, + + DefKind::TyParam + | DefKind::ConstParam + | DefKind::Ctor(..) + | DefKind::Field + | DefKind::LifetimeParam + | DefKind::Closure + | DefKind::SyntheticCoroutineBody => false, + } } /// Returns the local def id of the ADT if the given ty refers to a local one. From 99ee62305a4fbbf56f6be7b1ea3c315d4f0ac268 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 3 Aug 2025 01:44:20 +0000 Subject: [PATCH 3/9] Remove struct_constructors. --- compiler/rustc_passes/src/dead.rs | 46 ++++++++----------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index bf5a31f7dde9e..cc39d3c346e24 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -57,11 +57,11 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { | DefKind::InlineConst | DefKind::ExternCrate | DefKind::Use + | DefKind::Ctor(..) | DefKind::ForeignMod => true, DefKind::TyParam | DefKind::ConstParam - | DefKind::Ctor(..) | DefKind::Field | DefKind::LifetimeParam | DefKind::Closure @@ -103,8 +103,6 @@ struct MarkSymbolVisitor<'tcx> { repr_has_repr_simd: bool, in_pat: bool, ignore_variant_stack: Vec, - // maps from tuple struct constructors to tuple struct items - struct_constructors: LocalDefIdMap, // 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) @@ -123,7 +121,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { fn check_def_id(&mut self, def_id: DefId) { if let Some(def_id) = def_id.as_local() { - if should_explore(self.tcx, def_id) || self.struct_constructors.contains_key(&def_id) { + if should_explore(self.tcx, def_id) { self.worklist.push((def_id, ComesFromAllowExpect::No)); } self.live_symbols.insert(def_id); @@ -348,7 +346,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { continue; } - let (id, comes_from_allow_expect) = work; + let (mut id, comes_from_allow_expect) = work; // Avoid accessing the HIR for the synthesized associated type generated for RPITITs. if self.tcx.is_impl_trait_in_trait(id.to_def_id()) { @@ -356,9 +354,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { continue; } - // in the case of tuple struct constructors we want to check the item, not the generated - // tuple struct constructor function - let id = self.struct_constructors.get(&id).copied().unwrap_or(id); + // in the case of tuple struct constructors we want to check the item, + // not the generated tuple struct constructor function + if let DefKind::Ctor(..) = self.tcx.def_kind(id) { + id = self.tcx.local_parent(id); + } // When using `#[allow]` or `#[expect]` of `dead_code`, we do a QOL improvement // by declaring fn calls, statics, ... within said items as live, as well as @@ -751,7 +751,6 @@ fn has_allow_dead_code_or_lang_attr( fn check_item<'tcx>( tcx: TyCtxt<'tcx>, worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, - struct_constructors: &mut LocalDefIdMap, unsolved_items: &mut Vec<(hir::ItemId, LocalDefId)>, id: hir::ItemId, ) { @@ -769,12 +768,6 @@ fn check_item<'tcx>( enum_def.variants.iter().map(|variant| (variant.def_id, comes_from_allow)), ); } - - for variant in enum_def.variants { - if let Some(ctor_def_id) = variant.data.ctor_def_id() { - struct_constructors.insert(ctor_def_id, variant.def_id); - } - } } } DefKind::Impl { of_trait } => { @@ -802,14 +795,6 @@ fn check_item<'tcx>( } } } - DefKind::Struct => { - let item = tcx.hir_item(id); - if let hir::ItemKind::Struct(_, _, ref variant_data) = item.kind - && let Some(ctor_def_id) = variant_data.ctor_def_id() - { - struct_constructors.insert(ctor_def_id, item.owner_id.def_id); - } - } DefKind::GlobalAsm => { // global_asm! is always live. worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No)); @@ -859,15 +844,9 @@ fn check_foreign_item( fn create_and_seed_worklist( tcx: TyCtxt<'_>, -) -> ( - Vec<(LocalDefId, ComesFromAllowExpect)>, - LocalDefIdMap, - Vec<(hir::ItemId, LocalDefId)>, -) { +) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, 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() .filter_map(|(&id, effective_vis)| { @@ -885,7 +864,7 @@ fn create_and_seed_worklist( let crate_items = tcx.hir_crate_items(()); for id in crate_items.free_items() { - check_item(tcx, &mut worklist, &mut struct_constructors, &mut unsolved_impl_item, id); + check_item(tcx, &mut worklist, &mut unsolved_impl_item, id); } for id in crate_items.trait_items() { @@ -896,14 +875,14 @@ fn create_and_seed_worklist( check_foreign_item(tcx, &mut worklist, id); } - (worklist, struct_constructors, unsolved_impl_item) + (worklist, unsolved_impl_item) } fn live_symbols_and_ignored_derived_traits( tcx: TyCtxt<'_>, (): (), ) -> (LocalDefIdSet, LocalDefIdMap>) { - let (worklist, struct_constructors, mut unsolved_items) = create_and_seed_worklist(tcx); + let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx); let mut symbol_visitor = MarkSymbolVisitor { worklist, tcx, @@ -913,7 +892,6 @@ fn live_symbols_and_ignored_derived_traits( repr_has_repr_simd: false, in_pat: false, ignore_variant_stack: vec![], - struct_constructors, ignored_derived_traits: Default::default(), }; symbol_visitor.mark_live_symbols(); From 6c39b30f80ff8d970645288f7ada21036f2cab4b Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 2 Aug 2025 22:32:19 +0000 Subject: [PATCH 4/9] Simplify handling of unsolved items. --- compiler/rustc_passes/src/dead.rs | 90 +++++++++++++------------------ 1 file changed, 38 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index cc39d3c346e24..e99bea57ff13f 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -14,7 +14,7 @@ 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, ImplItem, ImplItemKind, Node, PatKind, QPath, TyKind}; +use rustc_hir::{self as hir, ImplItem, ImplItemKind, Node, PatKind, QPath}; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::privacy::Level; use rustc_middle::query::Providers; @@ -69,23 +69,6 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { } } -/// 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 { - 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, - } -} - /// Determine if a work from the worklist is coming from a `#[allow]` /// or a `#[expect]` of `dead_code` #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -499,24 +482,24 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { /// `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 { - let trait_def_id = match self.tcx.def_kind(local_def_id) { + fn check_impl_or_impl_item_live(&mut self, local_def_id: LocalDefId) -> bool { + let (impl_block_id, 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::AssocConst | DefKind::AssocTy | DefKind::AssocFn => self - .tcx - .associated_item(local_def_id) - .trait_item_def_id - .and_then(|def_id| def_id.as_local()), + DefKind::AssocConst | DefKind::AssocTy | DefKind::AssocFn => ( + self.tcx.local_parent(local_def_id), + self.tcx + .associated_item(local_def_id) + .trait_item_def_id + .and_then(|def_id| def_id.as_local()), + ), // 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| trait_ref.skip_binder().def_id.as_local()), - _ => None, + DefKind::Impl { of_trait: true } => ( + local_def_id, + self.tcx + .impl_trait_ref(local_def_id) + .and_then(|trait_ref| trait_ref.skip_binder().def_id.as_local()), + ), + _ => bug!(), }; if let Some(trait_def_id) = trait_def_id @@ -526,9 +509,9 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { } // 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) + if let ty::Adt(adt, _) = self.tcx.type_of(impl_block_id).instantiate_identity().kind() + && let Some(adt_def_id) = adt.did().as_local() + && !self.live_symbols.contains(&adt_def_id) { return false; } @@ -751,7 +734,7 @@ fn has_allow_dead_code_or_lang_attr( fn check_item<'tcx>( tcx: TyCtxt<'tcx>, worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, - unsolved_items: &mut Vec<(hir::ItemId, LocalDefId)>, + unsolved_items: &mut Vec, id: hir::ItemId, ) { let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id); @@ -776,7 +759,7 @@ fn check_item<'tcx>( { worklist.push((id.owner_id.def_id, comes_from_allow)); } else if of_trait { - unsolved_items.push((id, id.owner_id.def_id)); + unsolved_items.push(id.owner_id.def_id); } for def_id in tcx.associated_item_def_ids(id.owner_id) { @@ -791,7 +774,7 @@ fn check_item<'tcx>( // 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)); + unsolved_items.push(local_def_id); } } } @@ -844,7 +827,7 @@ fn check_foreign_item( fn create_and_seed_worklist( tcx: TyCtxt<'_>, -) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, Vec<(hir::ItemId, LocalDefId)>) { +) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, Vec) { let effective_visibilities = &tcx.effective_visibilities(()); let mut unsolved_impl_item = Vec::new(); let mut worklist = effective_visibilities @@ -895,21 +878,24 @@ fn live_symbols_and_ignored_derived_traits( ignored_derived_traits: Default::default(), }; symbol_visitor.mark_live_symbols(); - 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) - }); + + // We have marked the primary seeds as live. We now need to process unsolved items from traits + // and trait impls: add them to the work list if the trait or the implemented type is live. + let mut items_to_check: Vec<_> = unsolved_items + .extract_if(.., |&mut local_def_id| { + symbol_visitor.check_impl_or_impl_item_live(local_def_id) + }) + .collect(); while !items_to_check.is_empty() { - symbol_visitor.worklist = - items_to_check.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect(); + symbol_visitor + .worklist + .extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No))); 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) - }); + items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| { + symbol_visitor.check_impl_or_impl_item_live(local_def_id) + })); } (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits) From f601717c5b52b5b0fbe468d911e663fe8740c7a9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 2 Aug 2025 22:11:22 +0000 Subject: [PATCH 5/9] Use less HIR when seeding work list. --- compiler/rustc_passes/src/dead.rs | 140 +++++++++++------------------- 1 file changed, 50 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index e99bea57ff13f..43d2361aad891 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -718,113 +718,81 @@ fn has_allow_dead_code_or_lang_attr( } } -// These check_* functions seeds items that -// 1) We want to explicitly consider as live: -// * Item annotated with #[allow(dead_code)] -// - This is done so that if we want to suppress warnings for a -// group of dead functions, we only have to annotate the "root". -// For example, if both `f` and `g` are dead and `f` calls `g`, -// then annotating `f` with `#[allow(dead_code)]` will suppress -// warning for both `f` and `g`. -// * Item annotated with #[lang=".."] -// - This is because lang items are always callable from elsewhere. -// or -// 2) We are not sure to be live or not -// * Implementations of traits and trait methods -fn check_item<'tcx>( +/// Examine the given definition and record it in the worklist if it should be considered live. +/// +/// We want to explicitly consider as live: +/// * Item annotated with #[allow(dead_code)] +/// This is done so that if we want to suppress warnings for a +/// group of dead functions, we only have to annotate the "root". +/// For example, if both `f` and `g` are dead and `f` calls `g`, +/// then annotating `f` with `#[allow(dead_code)]` will suppress +/// warning for both `f` and `g`. +/// +/// * Item annotated with #[lang=".."] +/// Lang items are always callable from elsewhere. +/// +/// For trait methods and implementations of traits, we are not certain that the definitions are +/// live at this stage. We record them in `unsolved_items` for later examination. +fn maybe_record_as_seed<'tcx>( tcx: TyCtxt<'tcx>, + owner_id: hir::OwnerId, worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, unsolved_items: &mut Vec, - id: hir::ItemId, ) { - let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id); + let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id); if let Some(comes_from_allow) = allow_dead_code { - worklist.push((id.owner_id.def_id, comes_from_allow)); + worklist.push((owner_id.def_id, comes_from_allow)); } - match tcx.def_kind(id.owner_id) { + match tcx.def_kind(owner_id) { DefKind::Enum => { - let item = tcx.hir_item(id); - if let hir::ItemKind::Enum(_, _, ref enum_def) = item.kind { - if let Some(comes_from_allow) = allow_dead_code { - worklist.extend( - enum_def.variants.iter().map(|variant| (variant.def_id, comes_from_allow)), - ); + let adt = tcx.adt_def(owner_id); + if let Some(comes_from_allow) = allow_dead_code { + worklist.extend( + adt.variants() + .iter() + .map(|variant| (variant.def_id.expect_local(), comes_from_allow)), + ); + } + } + DefKind::AssocFn | DefKind::AssocConst | DefKind::AssocTy => { + if allow_dead_code.is_none() { + let parent = tcx.local_parent(owner_id.def_id); + match tcx.def_kind(parent) { + DefKind::Impl { of_trait: false } | DefKind::Trait => {} + DefKind::Impl { of_trait: true } => { + // 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(owner_id.def_id); + } + _ => bug!(), } } } DefKind::Impl { of_trait } => { - 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.owner_id.def_id); - } - - for def_id in tcx.associated_item_def_ids(id.owner_id) { - let local_def_id = def_id.expect_local(); - - 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 { - // 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(local_def_id); - } + if allow_dead_code.is_none() && of_trait { + unsolved_items.push(owner_id.def_id); } } DefKind::GlobalAsm => { // global_asm! is always live. - worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No)); + worklist.push((owner_id.def_id, ComesFromAllowExpect::No)); } DefKind::Const => { - let item = tcx.hir_item(id); - if let hir::ItemKind::Const(ident, ..) = item.kind - && ident.name == kw::Underscore - { + if tcx.item_name(owner_id.def_id) == kw::Underscore { // `const _` is always live, as that syntax only exists for the side effects // of type checking and evaluating the constant expression, and marking them // as dead code would defeat that purpose. - worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No)); + worklist.push((owner_id.def_id, ComesFromAllowExpect::No)); } } _ => {} } } -fn check_trait_item( - tcx: TyCtxt<'_>, - worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, - id: hir::TraitItemId, -) { - use hir::TraitItemKind::{Const, Fn, Type}; - - let trait_item = tcx.hir_trait_item(id); - if matches!(trait_item.kind, Const(_, Some(_)) | Type(_, Some(_)) | Fn(..)) - && let Some(comes_from_allow) = - has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id) - { - worklist.push((trait_item.owner_id.def_id, comes_from_allow)); - } -} - -fn check_foreign_item( - tcx: TyCtxt<'_>, - worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>, - id: hir::ForeignItemId, -) { - if matches!(tcx.def_kind(id.owner_id), DefKind::Static { .. } | DefKind::Fn) - && 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)); - } -} - fn create_and_seed_worklist( tcx: TyCtxt<'_>, ) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, Vec) { @@ -846,16 +814,8 @@ fn create_and_seed_worklist( .collect::>(); let crate_items = tcx.hir_crate_items(()); - for id in crate_items.free_items() { - check_item(tcx, &mut worklist, &mut unsolved_impl_item, id); - } - - for id in crate_items.trait_items() { - check_trait_item(tcx, &mut worklist, id); - } - - for id in crate_items.foreign_items() { - check_foreign_item(tcx, &mut worklist, id); + for id in crate_items.owners() { + maybe_record_as_seed(tcx, id, &mut worklist, &mut unsolved_impl_item); } (worklist, unsolved_impl_item) From 377728a4043e7337a29f9485c8c65c978662c6e2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 3 Aug 2025 02:09:55 +0000 Subject: [PATCH 6/9] Keep scanned set across calls to mark_live_symbols. --- compiler/rustc_passes/src/dead.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 43d2361aad891..8eb834289370b 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -81,6 +81,7 @@ struct MarkSymbolVisitor<'tcx> { worklist: Vec<(LocalDefId, ComesFromAllowExpect)>, tcx: TyCtxt<'tcx>, maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>, + scanned: UnordSet<(LocalDefId, ComesFromAllowExpect)>, live_symbols: LocalDefIdSet, repr_unconditionally_treats_fields_as_live: bool, repr_has_repr_simd: bool, @@ -323,9 +324,8 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { } fn mark_live_symbols(&mut self) { - let mut scanned = UnordSet::default(); while let Some(work) = self.worklist.pop() { - if !scanned.insert(work) { + if !self.scanned.insert(work) { continue; } @@ -830,6 +830,7 @@ fn live_symbols_and_ignored_derived_traits( worklist, tcx, maybe_typeck_results: None, + scanned: Default::default(), live_symbols: Default::default(), repr_unconditionally_treats_fields_as_live: false, repr_has_repr_simd: false, From effc509b40d90a5fe0fa7964cd739b1c887da19f Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 3 Aug 2025 02:36:01 +0000 Subject: [PATCH 7/9] Simplify lint emission. --- compiler/rustc_passes/src/dead.rs | 130 +++++++++++++----------------- 1 file changed, 56 insertions(+), 74 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 8eb834289370b..88b3f742748ee 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -5,7 +5,6 @@ use std::mem; -use hir::ItemKind; use hir::def_id::{LocalDefIdMap, LocalDefIdSet}; use rustc_abi::FieldIdx; use rustc_data_structures::fx::FxIndexSet; @@ -14,7 +13,7 @@ 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, ImplItem, ImplItemKind, Node, PatKind, QPath}; +use rustc_hir::{self as hir, Node, PatKind, QPath}; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::privacy::Level; use rustc_middle::query::Providers; @@ -930,25 +929,7 @@ impl<'tcx> DeadVisitor<'tcx> { parent_item: Option, report_on: ReportOn, ) { - fn get_parent_if_enum_variant<'tcx>( - tcx: TyCtxt<'tcx>, - may_variant: LocalDefId, - ) -> LocalDefId { - if let Node::Variant(_) = tcx.hir_node_by_def_id(may_variant) - && let Some(enum_did) = tcx.opt_parent(may_variant.to_def_id()) - && let Some(enum_local_id) = enum_did.as_local() - && let Node::Item(item) = tcx.hir_node_by_def_id(enum_local_id) - && let ItemKind::Enum(..) = item.kind - { - enum_local_id - } else { - may_variant - } - } - - let Some(&first_item) = dead_codes.first() else { - return; - }; + let Some(&first_item) = dead_codes.first() else { return }; let tcx = self.tcx; let first_lint_level = first_item.level; @@ -957,40 +938,40 @@ impl<'tcx> DeadVisitor<'tcx> { let names: Vec<_> = dead_codes.iter().map(|item| item.name).collect(); let spans: Vec<_> = dead_codes .iter() - .map(|item| match tcx.def_ident_span(item.def_id) { - Some(s) => s.with_ctxt(tcx.def_span(item.def_id).ctxt()), - None => tcx.def_span(item.def_id), + .map(|item| { + let span = tcx.def_span(item.def_id); + let ident_span = tcx.def_ident_span(item.def_id); + // FIXME(cjgillot) this SyntaxContext manipulation does not make any sense. + ident_span.map(|s| s.with_ctxt(span.ctxt())).unwrap_or(span) }) .collect(); - let descr = tcx.def_descr(first_item.def_id.to_def_id()); + let mut descr = tcx.def_descr(first_item.def_id.to_def_id()); // `impl` blocks are "batched" and (unlike other batching) might // contain different kinds of associated items. - let descr = if dead_codes.iter().any(|item| tcx.def_descr(item.def_id.to_def_id()) != descr) - { - "associated item" - } else { - descr - }; + if dead_codes.iter().any(|item| tcx.def_descr(item.def_id.to_def_id()) != descr) { + descr = "associated item" + } + let num = dead_codes.len(); let multiple = num > 6; let name_list = names.into(); - let parent_info = if let Some(parent_item) = parent_item { + let parent_info = parent_item.map(|parent_item| { let parent_descr = tcx.def_descr(parent_item.to_def_id()); let span = if let DefKind::Impl { .. } = tcx.def_kind(parent_item) { tcx.def_span(parent_item) } else { tcx.def_ident_span(parent_item).unwrap() }; - Some(ParentInfo { num, descr, parent_descr, span }) - } else { - None - }; + ParentInfo { num, descr, parent_descr, span } + }); - let encl_def_id = parent_item.unwrap_or(first_item.def_id); - // If parent of encl_def_id is an enum, use the parent ID instead. - let encl_def_id = get_parent_if_enum_variant(tcx, encl_def_id); + let mut encl_def_id = parent_item.unwrap_or(first_item.def_id); + // `ignored_derived_traits` is computed for the enum, not for the variants. + if let DefKind::Variant = tcx.def_kind(encl_def_id) { + encl_def_id = tcx.local_parent(encl_def_id); + } let ignored_derived_impls = self.ignored_derived_traits.get(&encl_def_id).map(|ign_traits| { @@ -1006,31 +987,6 @@ impl<'tcx> DeadVisitor<'tcx> { } }); - let enum_variants_with_same_name = dead_codes - .iter() - .filter_map(|dead_item| { - if let Node::ImplItem(ImplItem { - kind: ImplItemKind::Fn(..) | ImplItemKind::Const(..), - .. - }) = tcx.hir_node_by_def_id(dead_item.def_id) - && let Some(impl_did) = tcx.opt_parent(dead_item.def_id.to_def_id()) - && let DefKind::Impl { of_trait: false } = tcx.def_kind(impl_did) - && let ty::Adt(maybe_enum, _) = tcx.type_of(impl_did).skip_binder().kind() - && maybe_enum.is_enum() - && let Some(variant) = - maybe_enum.variants().iter().find(|i| i.name == dead_item.name) - { - Some(crate::errors::EnumVariantSameName { - dead_descr: tcx.def_descr(dead_item.def_id.to_def_id()), - dead_name: dead_item.name, - variant_span: tcx.def_span(variant.def_id), - }) - } else { - None - } - }) - .collect(); - let diag = match report_on { ReportOn::TupleField => { let tuple_fields = if let Some(parent_id) = parent_item @@ -1076,16 +1032,42 @@ impl<'tcx> DeadVisitor<'tcx> { ignored_derived_impls, } } - ReportOn::NamedField => MultipleDeadCodes::DeadCodes { - multiple, - num, - descr, - participle, - name_list, - parent_info, - ignored_derived_impls, - enum_variants_with_same_name, - }, + ReportOn::NamedField => { + let enum_variants_with_same_name = dead_codes + .iter() + .filter_map(|dead_item| { + if let DefKind::AssocFn | DefKind::AssocConst = + tcx.def_kind(dead_item.def_id) + && let impl_did = tcx.local_parent(dead_item.def_id) + && let DefKind::Impl { of_trait: false } = tcx.def_kind(impl_did) + && let ty::Adt(maybe_enum, _) = + tcx.type_of(impl_did).instantiate_identity().kind() + && maybe_enum.is_enum() + && let Some(variant) = + maybe_enum.variants().iter().find(|i| i.name == dead_item.name) + { + Some(crate::errors::EnumVariantSameName { + dead_descr: tcx.def_descr(dead_item.def_id.to_def_id()), + dead_name: dead_item.name, + variant_span: tcx.def_span(variant.def_id), + }) + } else { + None + } + }) + .collect(); + + MultipleDeadCodes::DeadCodes { + multiple, + num, + descr, + participle, + name_list, + parent_info, + ignored_derived_impls, + enum_variants_with_same_name, + } + } }; let hir_id = tcx.local_def_id_to_hir_id(first_item.def_id); From e0a89c4b1475b2cd8d7f835e43e79543e262a1ec Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 4 Aug 2025 23:00:45 +0000 Subject: [PATCH 8/9] Simplify maybe_record_as_seed. --- compiler/rustc_passes/src/dead.rs | 6 +++--- compiler/rustc_passes/src/lib.rs | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 88b3f742748ee..9a163fa6063a1 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -745,8 +745,8 @@ fn maybe_record_as_seed<'tcx>( match tcx.def_kind(owner_id) { DefKind::Enum => { - let adt = tcx.adt_def(owner_id); if let Some(comes_from_allow) = allow_dead_code { + let adt = tcx.adt_def(owner_id); worklist.extend( adt.variants() .iter() @@ -771,8 +771,8 @@ fn maybe_record_as_seed<'tcx>( } } } - DefKind::Impl { of_trait } => { - if allow_dead_code.is_none() && of_trait { + DefKind::Impl { of_trait: true } => { + if allow_dead_code.is_none() { unsolved_items.push(owner_id.def_id); } } diff --git a/compiler/rustc_passes/src/lib.rs b/compiler/rustc_passes/src/lib.rs index af7ecf0830c22..2ad0b5ff60e84 100644 --- a/compiler/rustc_passes/src/lib.rs +++ b/compiler/rustc_passes/src/lib.rs @@ -8,6 +8,7 @@ #![allow(internal_features)] #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] +#![feature(if_let_guard)] #![feature(map_try_insert)] #![feature(rustdoc_internals)] // tidy-alphabetical-end From d0da6ca110157dee080afc39ca55fb6a944a9e99 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 4 Aug 2025 23:02:27 +0000 Subject: [PATCH 9/9] Update doc-comment. --- compiler/rustc_passes/src/dead.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 9a163fa6063a1..fa9d0c7b1b7d4 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -27,10 +27,9 @@ use crate::errors::{ ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment, }; -// Any local node that may call something in its body block should be -// explored. For example, if it's a live Node::Item that is a -// function, then we should explore its block to check for codes that -// may need to be marked as live. +/// Any local definition that may call something in its body block should be explored. For example, +/// if it's a live function, then we should explore its block to check for codes that may need to +/// be marked as live. fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { match tcx.def_kind(def_id) { DefKind::Mod