Auto merge of rust-lang#161571 - mu001999-contrib:dead-code-refactor, r=chenyukang

Refactor the `#[allow(dead_code)]` propagation for impl items of traits



Extracted from rust-lang#157885.

This PR does the refactor and corrects the previous implementation.

The following will fail currently ([play](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=3279902b2d2e6b0fe75c5af565b1cddd)):
```rust
#![deny(dead_code)]
#![deny(unfulfilled_lint_expectations)]

#[allow(dead_code)]
pub trait Tr {
    fn foo(&self);
}

struct Foo;

impl Tr for Foo {
    fn foo(&self) {
        bar();
    }
}

#[expect(dead_code)]
fn bar() {}

fn main() {}
```

After this PR, we could handle the `#[allow(dead_code)]` propagation correctly, and should get perf improvement.

Author: bors

compiler/rustc_passes/src/dead.rs

@@ -9,7 +9,7 @@ use std::sync::atomic::Ordering;
 
 use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
 use rustc_abi::FieldIdx;
-use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
 use rustc_errors::{ErrorGuaranteed, MultiSpan};
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalModId};
@@ -114,6 +114,11 @@ struct WorkItem {
     own: ComesFromAllowExpect,
 }
 
+enum ImplItemCheckResult {
+    Live(ComesFromAllowExpect),
+    Dead { require: LocalDefId },
+}
+
 struct MarkSymbolVisitor<'tcx> {
     worklist: Vec<WorkItem>,
     tcx: TyCtxt<'tcx>,
@@ -129,6 +134,7 @@ struct MarkSymbolVisitor<'tcx> {
     // macro)
     ignored_derived_traits: LocalDefIdMap<FxIndexSet<DefId>>,
     propagated_comes_from_allow_expect: ComesFromAllowExpect,
+    unsolved_items: Vec<LocalDefId>,
 }
 
 impl<'tcx> MarkSymbolVisitor<'tcx> {
@@ -419,6 +425,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
 
             if !self.scanned.insert((id, propagated)) {
                 continue;
+            } else if propagated == ComesFromAllowExpect::No {
+                // If the item is not coming from an `#[allow]` or `#[expect]`,
+                // we also mark it as scanned with `ComesFromAllowExpect::Yes`
+                // to avoid re-scanning it in the future.
+                self.scanned.insert((id, ComesFromAllowExpect::Yes));
             }
 
             // Avoid accessing the HIR for the synthesized associated type generated for RPITITs.
@@ -545,13 +556,17 @@ 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, local_def_id: LocalDefId) -> bool {
+    fn check_impl_or_impl_item_live(
+        &self,
+        local_def_id: LocalDefId,
+        defer_seeds_come_from_allow: bool,
+    ) -> ImplItemCheckResult {
         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 => {
-                let trait_item_id =
+                let trait_def_id =
                     self.tcx.trait_item_of(local_def_id).and_then(|def_id| def_id.as_local());
-                (self.tcx.local_parent(local_def_id), trait_item_id)
+                (self.tcx.local_parent(local_def_id), trait_def_id)
             }
             // impl items are live if the corresponding traits are live
             DefKind::Impl { of_trait: true } => {
@@ -560,10 +575,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
             _ => bug!(),
         };
 
-        if let Some(trait_def_id) = trait_def_id
-            && !self.live_symbols.contains(&trait_def_id)
-        {
-            return false;
+        let mut trait_comes_from_allow = None;
+        if let Some(trait_def_id) = trait_def_id {
+            if defer_seeds_come_from_allow {
+                if !self.live_symbols.contains(&trait_def_id) {
+                    return ImplItemCheckResult::Dead { require: trait_def_id };
+                }
+            } else {
+                trait_comes_from_allow = has_allow_dead_code_or_lang_attr(self.tcx, trait_def_id);
+
+                if !self.live_symbols.contains(&trait_def_id) {
+                    return match trait_comes_from_allow {
+                        Some(comes_from_allow) => ImplItemCheckResult::Live(comes_from_allow),
+                        None => ImplItemCheckResult::Dead { require: trait_def_id },
+                    };
+                }
+            }
         }
 
         // The impl or impl item is used if the corresponding trait or trait item is used and the ty is used.
@@ -572,10 +599,95 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
             && let Some(adt_def_id) = adt.did().as_local()
             && !self.live_symbols.contains(&adt_def_id)
         {
-            return false;
+            if defer_seeds_come_from_allow {
+                return ImplItemCheckResult::Dead { require: adt_def_id };
+            } else {
+                let comes_from_allow = trait_comes_from_allow
+                    .or_else(|| has_allow_dead_code_or_lang_attr(self.tcx, adt_def_id));
+
+                return match comes_from_allow {
+                    Some(comes_from_allow) => ImplItemCheckResult::Live(comes_from_allow),
+                    None => ImplItemCheckResult::Dead { require: adt_def_id },
+                };
+            }
         }
 
-        true
+        ImplItemCheckResult::Live(ComesFromAllowExpect::No)
+    }
+
+    fn collect_live_items_from_unsolved_items(
+        &mut self,
+        defer_seeds_come_from_allow: bool,
+        unsolved_items: Vec<LocalDefId>,
+        unsolved_map: &mut FxHashMap<LocalDefId, Vec<LocalDefId>>,
+    ) -> Vec<(LocalDefId, ComesFromAllowExpect)> {
+        let mut items_to_check = vec![];
+
+        for def_id in unsolved_items {
+            match self.check_impl_or_impl_item_live(def_id, defer_seeds_come_from_allow) {
+                ImplItemCheckResult::Live(comes_from_allow) => {
+                    items_to_check.push((def_id, comes_from_allow));
+                }
+                ImplItemCheckResult::Dead { require } => {
+                    unsolved_map.entry(require).or_default().push(def_id);
+                }
+            }
+        }
+        items_to_check
+    }
+
+    #[expect(
+        rustc::potential_query_instability,
+        reason = "The order of the unsolved items is not important, so we can just collect them into a vector."
+    )]
+    fn mark_live_symbols_and_ignored_derived_traits(
+        &mut self,
+        defer_seeds_come_from_allow: bool,
+    ) -> Result<(), ErrorGuaranteed> {
+        if let ControlFlow::Break(guar) = self.mark_live_symbols() {
+            return Err(guar);
+        }
+
+        // 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 unsolved_items = std::mem::take(&mut self.unsolved_items);
+        let mut unsolved_map = FxHashMap::default();
+        let mut items_to_check = self.collect_live_items_from_unsolved_items(
+            defer_seeds_come_from_allow,
+            unsolved_items,
+            &mut unsolved_map,
+        );
+
+        while !items_to_check.is_empty() {
+            self.worklist.extend(items_to_check.into_iter().map(|(id, comes_from_allow)| {
+                let own = if defer_seeds_come_from_allow {
+                    ComesFromAllowExpect::No
+                } else {
+                    has_allow_dead_code_or_lang_attr(self.tcx, id)
+                        .unwrap_or(ComesFromAllowExpect::No)
+                };
+
+                WorkItem { id, propagated: comes_from_allow, own }
+            }));
+            if let ControlFlow::Break(guar) = self.mark_live_symbols() {
+                return Err(guar);
+            }
+
+            let unsolved_items = unsolved_map
+                .extract_if(|require, _| self.live_symbols.contains(require))
+                .flat_map(|(_, items)| items)
+                .collect();
+
+            items_to_check = self.collect_live_items_from_unsolved_items(
+                defer_seeds_come_from_allow,
+                unsolved_items,
+                &mut unsolved_map,
+            );
+        }
+
+        self.unsolved_items = unsolved_map.into_values().flatten().collect();
+
+        Ok(())
     }
 }
 
@@ -850,19 +962,6 @@ fn maybe_record_as_seed<'tcx>(
                 match tcx.def_kind(parent) {
                     DefKind::Impl { of_trait: false } | DefKind::Trait => {}
                     DefKind::Impl { of_trait: true } => {
-                        if let Some(trait_item_def_id) =
-                            tcx.associated_item(owner_id.def_id).trait_item_def_id()
-                            && let Some(trait_item_local_def_id) = trait_item_def_id.as_local()
-                            && let Some(comes_from_allow) =
-                                has_allow_dead_code_or_lang_attr(tcx, trait_item_local_def_id)
-                        {
-                            push_into_worklist(WorkItem {
-                                id: owner_id.def_id,
-                                propagated: comes_from_allow,
-                                own: comes_from_allow,
-                            });
-                        }
-
                         // 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
@@ -874,22 +973,8 @@ fn maybe_record_as_seed<'tcx>(
                 }
             }
         }
-        DefKind::Impl { of_trait: true } => {
-            if allow_dead_code.is_none() {
-                if let Some(trait_def_id) =
-                    tcx.impl_trait_ref(owner_id.def_id).skip_binder().def_id.as_local()
-                    && let Some(comes_from_allow) =
-                        has_allow_dead_code_or_lang_attr(tcx, trait_def_id)
-                {
-                    push_into_worklist(WorkItem {
-                        id: owner_id.def_id,
-                        propagated: comes_from_allow,
-                        own: comes_from_allow,
-                    });
-                }
-
-                unsolved_items.push(owner_id.def_id);
-            }
+        DefKind::Impl { of_trait: true } if allow_dead_code.is_none() => {
+            unsolved_items.push(owner_id.def_id);
         }
         DefKind::GlobalAsm => {
             // global_asm! is always live.
@@ -915,15 +1000,21 @@ fn maybe_record_as_seed<'tcx>(
     }
 }
 
+#[derive(Default)]
+struct DeferredSeeds {
+    pub_reachables: Vec<WorkItem>,
+    come_from_allow: Vec<WorkItem>,
+}
+
 struct SeedWorklists {
     worklist: Vec<WorkItem>,
-    deferred_seeds: Vec<WorkItem>,
+    deferred_seeds: DeferredSeeds,
     unsolved_items: Vec<LocalDefId>,
 }
 
 fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
     let mut unsolved_items = Vec::new();
-    let mut deferred_seeds = Vec::new();
+    let mut deferred_seeds = DeferredSeeds::default();
     let mut worklist = Vec::new();
 
     if let Some((def_id, _)) = tcx.entry_fn(())
@@ -953,7 +1044,7 @@ fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
 
     for (id, effective_vis) in tcx.effective_visibilities(()).iter() {
         if effective_vis.is_public_at_level(Level::Reachable) {
-            deferred_seeds.push(WorkItem {
+            deferred_seeds.pub_reachables.push(WorkItem {
                 id: *id,
                 propagated: ComesFromAllowExpect::No,
                 own: ComesFromAllowExpect::No,
@@ -962,7 +1053,7 @@ fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
     }
 
     let mut push_into_worklist = |work_item: WorkItem| match work_item.own {
-        ComesFromAllowExpect::Yes => deferred_seeds.push(work_item),
+        ComesFromAllowExpect::Yes => deferred_seeds.come_from_allow.push(work_item),
         ComesFromAllowExpect::No => worklist.push(work_item),
     };
     let crate_items = tcx.hir_crate_items(());
@@ -977,8 +1068,7 @@ fn live_symbols_and_ignored_derived_traits(
     tcx: TyCtxt<'_>,
     (): (),
 ) -> Result<DeadCodeLivenessSummary, ErrorGuaranteed> {
-    let SeedWorklists { worklist, deferred_seeds, mut unsolved_items } =
-        create_and_seed_worklist(tcx);
+    let SeedWorklists { worklist, deferred_seeds, unsolved_items } = create_and_seed_worklist(tcx);
     let mut symbol_visitor = MarkSymbolVisitor {
         worklist,
         tcx,
@@ -991,15 +1081,23 @@ fn live_symbols_and_ignored_derived_traits(
         ignore_variant_stack: vec![],
         ignored_derived_traits: Default::default(),
         propagated_comes_from_allow_expect: ComesFromAllowExpect::No,
+        unsolved_items,
     };
-    mark_live_symbols_and_ignored_derived_traits(&mut symbol_visitor, &mut unsolved_items)?;
+    symbol_visitor.mark_live_symbols_and_ignored_derived_traits(true)?;
     let pre_deferred_seeding = DeadCodeLivenessSnapshot {
         live_symbols: symbol_visitor.live_symbols.clone(),
         ignored_derived_traits: symbol_visitor.ignored_derived_traits.clone(),
     };
 
-    symbol_visitor.worklist.extend(deferred_seeds);
-    mark_live_symbols_and_ignored_derived_traits(&mut symbol_visitor, &mut unsolved_items)?;
+    if !deferred_seeds.pub_reachables.is_empty() {
+        symbol_visitor.worklist.extend(deferred_seeds.pub_reachables);
+        symbol_visitor.mark_live_symbols_and_ignored_derived_traits(true)?;
+    }
+
+    if !deferred_seeds.come_from_allow.is_empty() {
+        symbol_visitor.worklist.extend(deferred_seeds.come_from_allow);
+        symbol_visitor.mark_live_symbols_and_ignored_derived_traits(false)?;
+    }
 
     Ok(DeadCodeLivenessSummary {
         pre_deferred_seeding,
@@ -1010,40 +1108,6 @@ fn live_symbols_and_ignored_derived_traits(
     })
 }
 
-fn mark_live_symbols_and_ignored_derived_traits(
-    symbol_visitor: &mut MarkSymbolVisitor<'_>,
-    unsolved_items: &mut Vec<LocalDefId>,
-) -> Result<(), ErrorGuaranteed> {
-    if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
-        return Err(guar);
-    }
-
-    // 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.extend(items_to_check.drain(..).map(|id| WorkItem {
-            id,
-            propagated: ComesFromAllowExpect::No,
-            own: ComesFromAllowExpect::No,
-        }));
-        if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
-            return Err(guar);
-        }
-
-        items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
-            symbol_visitor.check_impl_or_impl_item_live(local_def_id)
-        }));
-    }
-
-    Ok(())
-}
-
 struct DeadItem {
     def_id: LocalDefId,
     name: Symbol,

tests/ui/lint/dead-code/allow-dead-code-propagation-to-impls.rs

@@ -0,0 +1,23 @@
+//@ check-pass
+
+#![deny(dead_code)]
+#![deny(unfulfilled_lint_expectations)]
+
+#[allow(dead_code)]
+pub trait Tr {
+    fn foo(&self);
+}
+
+#[expect(dead_code)]
+struct Foo;
+
+impl Tr for Foo {
+    fn foo(&self) {
+        bar();
+    }
+}
+
+#[expect(dead_code)]
+fn bar() {}
+
+fn main() {}