Make traits / trait methods detected by the dead code lint!

Author: mu001999

compiler/rustc_passes/src/dead.rs

@@ -4,7 +4,7 @@
 // is dead.
 
 use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
-use hir::AssocItemKind;
+use hir::ItemKind;
 use itertools::Itertools;
 use rustc_data_structures::unord::UnordSet;
 use rustc_errors::MultiSpan;
@@ -16,7 +16,7 @@ use rustc_hir::{Node, PatKind, TyKind};
 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};
+use rustc_middle::ty::{self, TyCtxt, Visibility};
 use rustc_session::lint;
 use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::FieldIdx;
@@ -377,20 +377,42 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     intravisit::walk_item(self, item)
                 }
                 hir::ItemKind::ForeignMod { .. } => {}
+                hir::ItemKind::Trait(..) => {
+                    // mark dependent traits live
+                    for impl_def_id in self.tcx.all_impls(item.owner_id.to_def_id()) {
+                        if let Some(local_def_id) = impl_def_id.as_local()
+                            && let ItemKind::Impl(impl_ref) =
+                                self.tcx.hir().expect_item(local_def_id).kind
+                        {
+                            intravisit::walk_generics(self, impl_ref.generics);
+                        }
+                    }
+
+                    intravisit::walk_item(self, item)
+                }
                 _ => intravisit::walk_item(self, item),
             },
             Node::TraitItem(trait_item) => {
-                // FIXME: could we get all impl terms for the trait item?
-                // then we can add them into worklist when the trait item is live,
-                // so that we won't lose any lost chain in the impl terms.
-                // this also allows us to lint such code:
-                // ```rust
-                // struct UnusedStruct; //~ WARN
-                // trait UnusedTrait { //~ WARN
-                //     fn foo() {}
-                // }
-                // impl UnusedTrait for UnusedStruct { fn foo() {} }
-                // ```
+                // mark corresponing ImplTerm live
+                let def_id = trait_item.owner_id.to_def_id();
+                if let Some(trait_def_id) = self.tcx.trait_of_item(def_id) {
+                    // for assoc fn without self, mark the trait live
+                    self.check_def_id(trait_def_id);
+
+                    for impl_def in self.tcx.all_impls(trait_def_id) {
+                        if let Some(impl_def_id) = impl_def.as_local()
+                            && let ItemKind::Impl(impl_ref) =
+                                self.tcx.hir().expect_item(impl_def_id).kind
+                        {
+                            self.check_def_id(impl_def);
+                            for impl_item in impl_ref.items {
+                                if Some(def_id) == impl_item.trait_item_def_id {
+                                    self.check_def_id(impl_item.id.owner_id.to_def_id());
+                                }
+                            }
+                        }
+                    }
+                }
                 intravisit::walk_trait_item(self, trait_item);
             }
             Node::ImplItem(impl_item) => {
@@ -639,23 +661,6 @@ fn check_item<'tcx>(
             }
         }
         DefKind::Impl { of_trait } => {
-            // lints unused struct and traits after 2024, e.g.,
-            // ```rust
-            // #[derive(Debug)]
-            // struct Unused; //~ WARN
-            //
-            // trait Foo {} //~ WARN
-            // impl Foo for () {}
-            // ```
-            // but we still cannot lint unused traits whose impl blocks have methods:
-            // ```rust
-            // trait Foo { fn foo(); }
-            // impl Foo for () { fn foo() {} }
-            // ```
-            if of_trait && !tcx.hir().item(id).span.source_callsite().at_least_rust_2024() {
-                worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No));
-            }
-
             // get DefIds from another query
             let local_def_ids = tcx
                 .associated_item_def_ids(id.owner_id)
@@ -664,9 +669,9 @@ fn check_item<'tcx>(
 
             // And we access the Map here to get HirId from LocalDefId
             for id in local_def_ids {
-                if of_trait {
-                    // FIXME: not push impl item into worklist by default,
-                    // pushed when corresponding trait items are reachable.
+                if tcx.local_visibility(id) == Visibility::Public
+                    || of_trait && !matches!(tcx.def_kind(id), DefKind::AssocFn)
+                {
                     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));
@@ -697,7 +702,7 @@ fn check_trait_item(
     use hir::TraitItemKind::{Const, Fn};
     if matches!(tcx.def_kind(id.owner_id), DefKind::AssocConst | DefKind::AssocFn) {
         let trait_item = tcx.hir().trait_item(id);
-        if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_)))
+        if matches!(trait_item.kind, Const(_, Some(_)) | Fn(..))
             && let Some(comes_from_allow) =
                 has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
         {
@@ -965,14 +970,8 @@ impl<'tcx> DeadVisitor<'tcx> {
             | DefKind::TyAlias
             | DefKind::Enum
             | DefKind::Union
-            | DefKind::ForeignTy => self.warn_dead_code(def_id, "used"),
-            DefKind::Trait => {
-                if let Some(Node::Item(item)) = self.tcx.hir().find_by_def_id(def_id)
-                    && item.span.at_least_rust_2024()
-                {
-                    self.warn_dead_code(def_id, "used")
-                }
-            }
+            | DefKind::ForeignTy
+            | DefKind::Trait => self.warn_dead_code(def_id, "used"),
             DefKind::Struct => self.warn_dead_code(def_id, "constructed"),
             DefKind::Variant | DefKind::Field => bug!("should be handled specially"),
             _ => {}
@@ -1001,24 +1000,7 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
             let mut dead_items = Vec::new();
             for item in impl_item.items {
                 let def_id = item.id.owner_id.def_id;
-                let is_dead_code = if !visitor.is_live_code(def_id) {
-                    true
-                } else if item.span.edition().at_least_rust_2024() // temporary
-                    && let AssocItemKind::Fn { .. } = item.kind
-                    && let Some(local_def_id) = item.trait_item_def_id.and_then(DefId::as_local)
-                    && !visitor.is_live_code(local_def_id)
-                    && has_allow_dead_code_or_lang_attr(tcx, local_def_id).is_none()
-                {
-                    // lint methods in impl if we are sure the corresponding methods in trait are dead,
-                    // but the chain of dead code within the methods in impl would be lost.
-
-                    // FIXME: the better way is to mark trait items and corresponding impl items active,
-                    // then the rests are dead, which requires the above FIXME at line 383
-                    true
-                } else {
-                    false
-                };
-                if is_dead_code {
+                if !visitor.is_live_code(def_id) {
                     let name = tcx.item_name(def_id.to_def_id());
                     let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
                     let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0;
@@ -1093,9 +1075,7 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     }
 
     for trait_item in module_items.trait_items() {
-        if tcx.hir().trait_item(trait_item).span.at_least_rust_2024() {
-            visitor.check_definition(trait_item.owner_id.def_id);
-        }
+        visitor.check_definition(trait_item.owner_id.def_id);
     }
 }
 

library/core/src/fmt/num.rs

@@ -15,7 +15,9 @@ trait DisplayInt:
     fn zero() -> Self;
     fn from_u8(u: u8) -> Self;
     fn to_u8(&self) -> u8;
+    #[allow(dead_code)]
     fn to_u16(&self) -> u16;
+    #[allow(dead_code)]
     fn to_u32(&self) -> u32;
     fn to_u64(&self) -> u64;
     fn to_u128(&self) -> u128;

library/std/src/error.rs

@@ -17,6 +17,7 @@ mod private {
     // implementations, since that can enable unsound downcasting.
     #[unstable(feature = "error_type_id", issue = "60784")]
     #[derive(Debug)]
+    #[allow(dead_code)]
     pub struct Internal;
 }
 

tests/ui/lint/dead-code/issue-41883.rs

@@ -1,28 +1,24 @@
-// edition: 2024
-// compile-flags: -Zunstable-options
 #![deny(dead_code)]
 
 enum Category {
     Dead, //~ ERROR variant `Dead` is never constructed
     Used,
 }
 
-trait Demo {
+trait UnusedTrait { //~ ERROR trait `UnusedTrait` is never used
     fn this_is_unused(&self) -> Category { //~ ERROR method `this_is_unused` is never used
         Category::Dead
     }
 }
 
-impl Demo for () {
+struct UnusedStruct; //~ ERROR struct `UnusedStruct` is never constructed
+
+impl UnusedTrait for UnusedStruct {
     fn this_is_unused(&self) -> Category { //~ ERROR method `this_is_unused` is never used
         Category::Used
     }
 }
 
-impl UnusedTrait for () {}
-
-trait UnusedTrait {} //~ ERROR trait `UnusedTrait` is never used
-
 mod private {
     #[derive(Debug)]
     struct UnusedStruct; //~ ERROR struct `UnusedStruct` is never constructed

tests/ui/lint/dead-code/issue-41883.stderr

@@ -1,44 +1,50 @@
 error: variant `Dead` is never constructed
-  --> $DIR/issue-41883.rs:6:5
+  --> $DIR/issue-41883.rs:4:5
    |
 LL | enum Category {
    |      -------- variant in this enum
 LL |     Dead,
    |     ^^^^
    |
 note: the lint level is defined here
-  --> $DIR/issue-41883.rs:3:9
+  --> $DIR/issue-41883.rs:1:9
    |
 LL | #![deny(dead_code)]
    |         ^^^^^^^^^
 
+error: trait `UnusedTrait` is never used
+  --> $DIR/issue-41883.rs:8:7
+   |
+LL | trait UnusedTrait {
+   |       ^^^^^^^^^^^
+
+error: struct `UnusedStruct` is never constructed
+  --> $DIR/issue-41883.rs:14:8
+   |
+LL | struct UnusedStruct;
+   |        ^^^^^^^^^^^^
+
 error: method `this_is_unused` is never used
   --> $DIR/issue-41883.rs:17:8
    |
-LL | impl Demo for () {
-   | ---------------- method in this implementation
+LL | impl UnusedTrait for UnusedStruct {
+   | --------------------------------- method in this implementation
 LL |     fn this_is_unused(&self) -> Category {
    |        ^^^^^^^^^^^^^^
 
-error: trait `UnusedTrait` is never used
-  --> $DIR/issue-41883.rs:24:7
-   |
-LL | trait UnusedTrait {}
-   |       ^^^^^^^^^^^
-
 error: method `this_is_unused` is never used
-  --> $DIR/issue-41883.rs:11:8
+  --> $DIR/issue-41883.rs:9:8
    |
 LL |     fn this_is_unused(&self) -> Category {
    |        ^^^^^^^^^^^^^^
 
 error: struct `UnusedStruct` is never constructed
-  --> $DIR/issue-41883.rs:28:12
+  --> $DIR/issue-41883.rs:24:12
    |
 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 5 previous errors
+error: aborting due to 6 previous errors