Apply suggestions, squash before merging

mu001999 · mu001999 · commit b19a6156059e · 2025-05-26T16:14:35.000+08:00
diff --git 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, 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;
@@ -46,10 +46,10 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
 }
 
 /// Returns the local def id and def kind of the adt,
-/// if the given ty refers to one local adt definition
-fn adt_def_of_ty<'tcx>(ty: &hir::Ty<'tcx>) -> Option<(LocalDefId, DefKind)> {
+/// if the given ty refers to one local adt definition.
+fn local_adt_def_of_ty<'tcx>(ty: &hir::Ty<'tcx>) -> Option<(LocalDefId, DefKind)> {
     match ty.kind {
-        TyKind::Path(hir::QPath::Resolved(_, path)) => {
+        TyKind::Path(QPath::Resolved(_, path)) => {
             if let Res::Def(def_kind, def_id) = path.res
                 && let Some(local_def_id) = def_id.as_local()
             {
@@ -366,7 +366,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                 && 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
             {
@@ -426,7 +426,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     intravisit::walk_item(self, item)
                 }
                 hir::ItemKind::ForeignMod { .. } => {}
-                hir::ItemKind::Trait(_, _, _, _, _, trait_item_refs) => {
+                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) {
@@ -485,12 +485,15 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         }
     }
 
-    /// Returns an impl or impl item should be checked or not
-    /// `impl_id` is the `ItemId` of the impl or the impl item belongs to,
-    /// `local_def_id` may point to the impl or the impl item,
-    /// both impl and impl item that may pass to this function are of trait,
+    /// Returns whether `local_def_id` is live 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 item_should_be_checked(&mut self, impl_id: hir::ItemId, local_def_id: LocalDefId) -> bool {
+    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;
         }
@@ -513,9 +516,9 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                 return false;
             }
 
-            // FIXME: legacy logic to check the fn may construct self,
-            // this can be removed after supporting marking adt appears
-            // in patterns as live, then we can check private impls of
+            // 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))
@@ -526,9 +529,9 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
             }
         }
 
-        // the impl/impl item is used if the trait/trait item is used and the ty is used
+        // The impl/impl item is used if the trait/trait item is used and the ty is used.
         if let Some((local_def_id, def_kind)) =
-            adt_def_of_ty(self.tcx.hir_item(impl_id).expect_impl().self_ty)
+            local_adt_def_of_ty(self.tcx.hir_item(impl_id).expect_impl().self_ty)
             && matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
             && !self.live_symbols.contains(&local_def_id)
         {
@@ -570,7 +573,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'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);
             }
@@ -750,12 +753,6 @@ fn check_item<'tcx>(
             }
         }
         DefKind::Impl { of_trait } => {
-            // get DefIds from another query
-            let associated_item_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)
             {
@@ -764,23 +761,26 @@ fn check_item<'tcx>(
                 unsolved_items.push((id, id.owner_id.def_id));
             }
 
-            // And we access the Map here to get HirId from LocalDefId
-            for local_def_id in associated_item_def_ids {
-                // FIXME: this condition can be removed
-                // if we support dead check for assoc consts and tys
-                if of_trait && !matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) {
-                    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)
+            // We access the Map here to get HirId from LocalDefId
+            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 assoc items of trait,
-                    // because they cannot be visited directly
-                    // so we mark them based on the trait/trait item
-                    // and self ty are checked first and both live,
-                    // but inherent assoc items can be visited and marked directly
-                    unsolved_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 assoc items of trait,
+                        // because they cannot be visited directly
+                        // so we mark them based on the trait/trait item
+                        // and self ty are checked first and both live,
+                        // but inherent assoc items can be visited and marked directly.
+                        unsolved_items.push((id, local_def_id));
+                    }
                 }
             }
         }
@@ -889,22 +889,20 @@ fn live_symbols_and_ignored_derived_traits(
         ignored_derived_traits: Default::default(),
     };
     symbol_visitor.mark_live_symbols();
-    let mut rest_items_should_be_checked;
-    (rest_items_should_be_checked, unsolved_items) =
+    let mut items_to_check;
+    (items_to_check, unsolved_items) =
         unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
-            symbol_visitor.item_should_be_checked(impl_id, local_def_id)
+            symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
         });
 
-    while !rest_items_should_be_checked.is_empty() {
-        symbol_visitor.worklist = rest_items_should_be_checked
-            .into_iter()
-            .map(|(_, id)| (id, ComesFromAllowExpect::No))
-            .collect();
+    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();
 
-        (rest_items_should_be_checked, unsolved_items) =
+        (items_to_check, unsolved_items) =
             unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
-                symbol_visitor.item_should_be_checked(impl_id, local_def_id)
+                symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
             });
     }
 
@@ -1187,10 +1185,11 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
         let def_kind = tcx.def_kind(item.owner_id);
 
         let mut dead_codes = Vec::new();
-        // only diagnose unused assoc items for 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
+        // 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))
         {
diff --git a/tests/ui/deriving/deriving-in-macro.rs b/tests/ui/deriving/deriving-in-macro.rs
@@ -1,11 +1,12 @@
+//@ check-pass
 #![allow(non_camel_case_types)]
-#![deny(dead_code)]
+#![allow(dead_code)]
 
 macro_rules! define_vec {
     () => (
         mod foo {
             #[derive(PartialEq)]
-            pub struct bar; //~ ERROR struct `bar` is never constructed
+            pub struct bar;
         }
     )
 }
diff --git a/tests/ui/deriving/deriving-in-macro.stderr b/tests/ui/deriving/deriving-in-macro.stderr

Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,12 @@`
	`1`	`+//@ check-pass`
`1`	`2`	`#![allow(non_camel_case_types)]`
`2`		`-#![deny(dead_code)]`
	`3`	`+#![allow(dead_code)]`
`3`	`4`
`4`	`5`	`macro_rules! define_vec {`
`5`	`6`	`() => (`
`6`	`7`	`mod foo {`
`7`	`8`	`#[derive(PartialEq)]`
`8`		- pub struct bar; //~ ERROR struct `bar` is never constructed
	`9`	`+ pub struct bar;`
`9`	`10`	`}`
`10`	`11`	`)`
`11`	`12`	`}`