Auto merge of #118257 - mu001999:dead_code/trait, r=cjgillot

Make traits / trait methods detected by the dead code lint

Fixes #118139 and #41883

Author: bors

compiler/rustc_passes/src/dead.rs

@@ -4,6 +4,7 @@
 // is dead.
 
 use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
+use hir::ItemKind;
 use rustc_data_structures::unord::UnordSet;
 use rustc_errors::MultiSpan;
 use rustc_hir as hir;
@@ -14,7 +15,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_session::lint::builtin::DEAD_CODE;
 use rustc_span::symbol::{sym, Symbol};
@@ -381,9 +382,46 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     intravisit::walk_item(self, item)
                 }
                 hir::ItemKind::ForeignMod { .. } => {}
+                hir::ItemKind::Trait(..) => {
+                    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
+                        {
+                            // 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);
+                        }
+                    }
+
+                    intravisit::walk_item(self, item)
+                }
                 _ => intravisit::walk_item(self, item),
             },
             Node::TraitItem(trait_item) => {
+                // mark corresponing ImplTerm 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
+                        {
+                            // mark self_ty live
+                            intravisit::walk_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);
             }
             Node::ImplItem(impl_item) => {
@@ -636,10 +674,6 @@ fn check_item<'tcx>(
             }
         }
         DefKind::Impl { of_trait } => {
-            if of_trait {
-                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)
@@ -648,7 +682,11 @@ fn check_item<'tcx>(
 
             // And we access the Map here to get HirId from LocalDefId
             for id in local_def_ids {
-                if of_trait {
+                // for impl trait blocks, mark associate functions live if the trait is public
+                if of_trait
+                    && (!matches!(tcx.def_kind(id), DefKind::AssocFn)
+                        || tcx.local_visibility(id) == Visibility::Public)
+                {
                     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));
@@ -679,7 +717,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)
         {
@@ -948,7 +986,8 @@ impl<'tcx> DeadVisitor<'tcx> {
             | DefKind::TyAlias
             | DefKind::Enum
             | DefKind::Union
-            | DefKind::ForeignTy => 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"),
             _ => {}
@@ -973,18 +1012,33 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     let module_items = tcx.hir_module_items(module);
 
     for item in module_items.items() {
-        if let hir::ItemKind::Impl(impl_item) = tcx.hir().item(item).kind {
-            let mut dead_items = Vec::new();
-            for item in impl_item.items {
-                let def_id = item.id.owner_id.def_id;
-                if !visitor.is_live_code(def_id) {
-                    let name = tcx.item_name(def_id.to_def_id());
-                    let level = visitor.def_lint_level(def_id);
+        let def_kind = tcx.def_kind(item.owner_id);
 
-                    dead_items.push(DeadItem { def_id, name, level })
+        let mut dead_codes = Vec::new();
+        // if we have diagnosed the trait, do not diagnose unused methods
+        if matches!(def_kind, DefKind::Impl { .. })
+            || (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)
+                {
+                    let name = tcx.item_name(def_id);
+                    let level = visitor.def_lint_level(local_def_id);
+                    dead_codes.push(DeadItem { def_id: local_def_id, name, level });
                 }
             }
-            visitor.warn_multiple(item.owner_id.def_id, "used", dead_items, ReportOn::NamedField);
+        }
+        if !dead_codes.is_empty() {
+            visitor.warn_multiple(item.owner_id.def_id, "used", dead_codes, ReportOn::NamedField);
         }
 
         if !live_symbols.contains(&item.owner_id.def_id) {
@@ -997,7 +1051,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
             continue;
         }
 
-        let def_kind = tcx.def_kind(item.owner_id);
         if let DefKind::Struct | DefKind::Union | DefKind::Enum = def_kind {
             let adt = tcx.adt_def(item.owner_id);
             let mut dead_variants = Vec::new();
@@ -1044,8 +1097,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     for foreign_item in module_items.foreign_items() {
         visitor.check_definition(foreign_item.owner_id.def_id);
     }
-
-    // We do not warn trait items.
 }
 
 pub(crate) fn provide(providers: &mut Providers) {

compiler/rustc_span/src/source_map/tests.rs

@@ -1,5 +1,7 @@
 use super::*;
 
+use rustc_data_structures::sync::FreezeLock;
+
 fn init_source_map() -> SourceMap {
     let sm = SourceMap::new(FilePathMapping::empty());
     sm.new_source_file(PathBuf::from("blork.rs").into(), "first line.\nsecond line".to_string());
@@ -263,53 +265,6 @@ fn t10() {
     );
 }
 
-/// Returns the span corresponding to the `n`th occurrence of `substring` in `source_text`.
-trait SourceMapExtension {
-    fn span_substr(
-        &self,
-        file: &Lrc<SourceFile>,
-        source_text: &str,
-        substring: &str,
-        n: usize,
-    ) -> Span;
-}
-
-impl SourceMapExtension for SourceMap {
-    fn span_substr(
-        &self,
-        file: &Lrc<SourceFile>,
-        source_text: &str,
-        substring: &str,
-        n: usize,
-    ) -> Span {
-        eprintln!(
-            "span_substr(file={:?}/{:?}, substring={:?}, n={})",
-            file.name, file.start_pos, substring, n
-        );
-        let mut i = 0;
-        let mut hi = 0;
-        loop {
-            let offset = source_text[hi..].find(substring).unwrap_or_else(|| {
-                panic!(
-                    "source_text `{}` does not have {} occurrences of `{}`, only {}",
-                    source_text, n, substring, i
-                );
-            });
-            let lo = hi + offset;
-            hi = lo + substring.len();
-            if i == n {
-                let span = Span::with_root_ctxt(
-                    BytePos(lo as u32 + file.start_pos.0),
-                    BytePos(hi as u32 + file.start_pos.0),
-                );
-                assert_eq!(&self.span_to_snippet(span).unwrap()[..], substring);
-                return span;
-            }
-            i += 1;
-        }
-    }
-}
-
 // Takes a unix-style path and returns a platform specific path.
 fn path(p: &str) -> PathBuf {
     path_str(p).into()

library/core/tests/macros.rs

@@ -1,3 +1,4 @@
+#[allow(dead_code)]
 trait Trait {
     fn blah(&self);
 }

library/std/src/sys_common/mod.rs

@@ -59,12 +59,14 @@ cfg_if::cfg_if! {
 
 /// A trait for viewing representations from std types
 #[doc(hidden)]
+#[allow(dead_code)] // not used on all platforms
 pub trait AsInner<Inner: ?Sized> {
     fn as_inner(&self) -> &Inner;
 }
 
 /// A trait for viewing representations from std types
 #[doc(hidden)]
+#[allow(dead_code)] // not used on all platforms
 pub trait AsInnerMut<Inner: ?Sized> {
     fn as_inner_mut(&mut self) -> &mut Inner;
 }

src/tools/miri/tests/fail/dyn-call-trait-mismatch.rs

@@ -1,4 +1,5 @@
 trait T1 {
+    #[allow(dead_code)]
     fn method1(self: Box<Self>);
 }
 trait T2 {

src/tools/miri/tests/fail/dyn-upcast-trait-mismatch.rs

@@ -2,30 +2,36 @@
 #![allow(incomplete_features)]
 
 trait Foo: PartialEq<i32> + std::fmt::Debug + Send + Sync {
+    #[allow(dead_code)]
     fn a(&self) -> i32 {
         10
     }
 
+    #[allow(dead_code)]
     fn z(&self) -> i32 {
         11
     }
 
+    #[allow(dead_code)]
     fn y(&self) -> i32 {
         12
     }
 }
 
 trait Bar: Foo {
+    #[allow(dead_code)]
     fn b(&self) -> i32 {
         20
     }
 
+    #[allow(dead_code)]
     fn w(&self) -> i32 {
         21
     }
 }
 
 trait Baz: Bar {
+    #[allow(dead_code)]
     fn c(&self) -> i32 {
         30
     }

src/tools/miri/tests/pass/cast-rfc0401-vtable-kinds.rs

@@ -9,6 +9,7 @@ trait Foo<T> {
     }
 }
 
+#[allow(dead_code)]
 trait Bar {
     fn bar(&self) {
         println!("Bar!");

src/tools/miri/tests/pass/dyn-upcast.rs

@@ -383,14 +383,17 @@ fn struct_() {
 
 fn replace_vptr() {
     trait A {
+        #[allow(dead_code)]
         fn foo_a(&self);
     }
 
     trait B {
+        #[allow(dead_code)]
         fn foo_b(&self);
     }
 
     trait C: A + B {
+        #[allow(dead_code)]
         fn foo_c(&self);
     }
 

src/tools/miri/tests/pass/weak_memory/weak.rs

@@ -11,6 +11,7 @@ use std::sync::atomic::Ordering::*;
 use std::sync::atomic::{fence, AtomicUsize};
 use std::thread::spawn;
 
+#[allow(dead_code)]
 #[derive(Copy, Clone)]
 struct EvilSend<T>(pub T);
 

src/tools/tidy/src/ui_tests.rs

@@ -13,8 +13,9 @@ use std::path::{Path, PathBuf};
 // desirable, because large numbers of files are unwieldy in general. See issue
 // #73494.
 const ENTRY_LIMIT: usize = 900;
-const ISSUES_ENTRY_LIMIT: usize = 1807;
-const ROOT_ENTRY_LIMIT: usize = 868;
+// FIXME: The following limits should be reduced eventually.
+const ISSUES_ENTRY_LIMIT: usize = 1819;
+const ROOT_ENTRY_LIMIT: usize = 870;
 
 const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
     "rs",     // test source files