collapse dead code warnings into a single diagnostic

add comments in `store_dead_field_or_variant`

support multiple log level

add a item ident label

fix ui tests

fix a ui test

fix a rustdoc ui test

use let chain

refactor: remove `store_dead_field_or_variant`

fix a tiny bug

Author: TaKO8Ki

Cargo.lock

@@ -4286,6 +4286,7 @@ dependencies = [
 name = "rustc_passes"
 version = "0.0.0"
 dependencies = [
+ "itertools",
  "rustc_ast",
  "rustc_ast_pretty",
  "rustc_attr",

compiler/rustc_passes/Cargo.toml

@@ -5,6 +5,7 @@ edition = "2021"
 
 [dependencies]
 tracing = "0.1"
+itertools = "0.10.1"
 rustc_middle = { path = "../rustc_middle" }
 rustc_attr = { path = "../rustc_attr" }
 rustc_data_structures = { path = "../rustc_data_structures" }

compiler/rustc_passes/src/dead.rs

@@ -2,8 +2,9 @@
 // closely. The idea is that all reachable symbols are live, codes called
 // from live codes are live, and everything else is dead.
 
+use itertools::Itertools;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_errors::pluralize;
+use rustc_errors::{pluralize, MultiSpan};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId};
@@ -164,9 +165,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     if let (Res::Local(id_l), Res::Local(id_r)) = (
                         typeck_results.qpath_res(qpath_l, lhs.hir_id),
                         typeck_results.qpath_res(qpath_r, rhs.hir_id),
-                    ) && id_l == id_r
-                    {
-                        return true;
+                    ) {
+                        if id_l == id_r {
+                            return true;
+                        }
                     }
                     return false;
                 }
@@ -269,13 +271,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
     }
 
     fn visit_node(&mut self, node: Node<'tcx>) {
-        match node {
-            Node::ImplItem(hir::ImplItem { def_id, .. })
-                if self.should_ignore_item(def_id.to_def_id()) =>
-            {
-                return;
-            }
-            _ => (),
+        if let Node::ImplItem(hir::ImplItem { def_id, .. }) = node
+            && self.should_ignore_item(def_id.to_def_id())
+        {
+            return;
         }
 
         let had_repr_c = self.repr_has_repr_c;
@@ -624,11 +623,17 @@ fn live_symbols_and_ignored_derived_traits<'tcx>(
     (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
 }
 
+struct DeadVariant {
+    hir_id: hir::HirId,
+    span: Span,
+    name: Symbol,
+    level: lint::Level,
+}
+
 struct DeadVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
     live_symbols: &'tcx FxHashSet<LocalDefId>,
     ignored_derived_traits: &'tcx FxHashMap<LocalDefId, Vec<(DefId, DefId)>>,
-    ignored_struct_def: FxHashSet<LocalDefId>,
 }
 
 impl<'tcx> DeadVisitor<'tcx> {
@@ -686,57 +691,119 @@ impl<'tcx> DeadVisitor<'tcx> {
         false
     }
 
+    fn warn_multiple_dead_codes(
+        &self,
+        dead_codes: &[(hir::HirId, Span, Symbol)],
+        participle: &str,
+        parent_hir_id: Option<hir::HirId>,
+    ) {
+        if let Some((id, _, name)) = dead_codes.first()
+            && !name.as_str().starts_with('_')
+        {
+            self.tcx.struct_span_lint_hir(
+                lint::builtin::DEAD_CODE,
+                *id,
+                MultiSpan::from_spans(
+                    dead_codes.iter().map(|(_, span, _)| *span).collect(),
+                ),
+                |lint| {
+                    let def_id = self.tcx.hir().local_def_id(*id);
+                    let descr = self.tcx.def_kind(def_id).descr(def_id.to_def_id());
+                    let span_len = dead_codes.len();
+                    let names = match &dead_codes.iter().map(|(_, _, n)| n.to_string()).collect::<Vec<_>>()[..]
+                    {
+                        _ if span_len > 6 => String::new(),
+                        [name] => format!("`{name}` "),
+                        [names @ .., last] => {
+                            format!("{} and `{last}` ", names.iter().map(|name| format!("`{name}`")).join(", "))
+                        }
+                        [] => unreachable!(),
+                    };
+                    let mut err = lint.build(&format!(
+                        "{these}{descr}{s} {names}{are} never {participle}",
+                        these = if span_len > 6 { "multiple " } else { "" },
+                        s = pluralize!(span_len),
+                        are = pluralize!("is", span_len),
+                    ));
+                    let hir = self.tcx.hir();
+                    if let Some(parent_hir_id) = parent_hir_id
+                        && let Some(parent_node) = hir.find(parent_hir_id)
+                        && let Node::Item(item) = parent_node
+                    {
+                        let def_id = self.tcx.hir().local_def_id(parent_hir_id);
+                        let parent_descr = self.tcx.def_kind(def_id).descr(def_id.to_def_id());
+                        err.span_label(
+                            item.ident.span,
+                            format!(
+                                "{descr}{s} in this {parent_descr}",
+                                s = pluralize!(span_len)
+                            ),
+                        );
+                    }
+                    if let Some(encl_scope) = hir.get_enclosing_scope(*id)
+                        && let Some(encl_def_id) = hir.opt_local_def_id(encl_scope)
+                        && let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id)
+                    {
+                        let traits_str = ign_traits
+                            .iter()
+                            .map(|(trait_id, _)| format!("`{}`", self.tcx.item_name(*trait_id)))
+                            .collect::<Vec<_>>()
+                            .join(" and ");
+                        let plural_s = pluralize!(ign_traits.len());
+                        let article = if ign_traits.len() > 1 { "" } else { "a " };
+                        let is_are = if ign_traits.len() > 1 { "these are" } else { "this is" };
+                        let msg = format!(
+                            "`{}` has {}derived impl{} for the trait{} {}, but {} \
+                            intentionally ignored during dead code analysis",
+                            self.tcx.item_name(encl_def_id.to_def_id()),
+                            article,
+                            plural_s,
+                            plural_s,
+                            traits_str,
+                            is_are
+                        );
+                        err.note(&msg);
+                    }
+                        err.emit();
+                    },
+            );
+        }
+    }
+
+    fn warn_dead_fields_and_variants(
+        &self,
+        hir_id: hir::HirId,
+        participle: &str,
+        dead_codes: Vec<DeadVariant>,
+    ) {
+        let mut dead_codes = dead_codes
+            .iter()
+            .filter(|v| !v.name.as_str().starts_with('_'))
+            .map(|v| v)
+            .collect::<Vec<&DeadVariant>>();
+        if dead_codes.is_empty() {
+            return;
+        }
+        dead_codes.sort_by_key(|v| v.level);
+        for (_, group) in &dead_codes.into_iter().group_by(|v| v.level) {
+            self.warn_multiple_dead_codes(
+                &group
+                    .map(|v| (v.hir_id, v.span, v.name))
+                    .collect::<Vec<(hir::HirId, Span, Symbol)>>(),
+                participle,
+                Some(hir_id),
+            );
+        }
+    }
+
     fn warn_dead_code(
         &mut self,
         id: hir::HirId,
         span: rustc_span::Span,
         name: Symbol,
         participle: &str,
     ) {
-        if !name.as_str().starts_with('_') {
-            self.tcx.struct_span_lint_hir(lint::builtin::DEAD_CODE, id, span, |lint| {
-                let def_id = self.tcx.hir().local_def_id(id);
-                let descr = self.tcx.def_kind(def_id).descr(def_id.to_def_id());
-                let mut err = lint.build(&format!("{descr} is never {participle}: `{name}`"));
-                let hir = self.tcx.hir();
-                let is_field_in_same_struct =
-                    if let Some(parent_hir_id) = self.tcx.hir().find_parent_node(id)
-                        && let Some(parent_node) = self.tcx.hir().find(parent_hir_id)
-                        && let Node::Item(hir::Item{kind: hir::ItemKind::Struct(..), ..}) = parent_node
-                        && let Some(did) = self.tcx.hir().opt_local_def_id(parent_hir_id)
-                    {
-                        !self.ignored_struct_def.insert(did)
-                    } else {
-                        false
-                    };
-                if !is_field_in_same_struct
-                    && let Some(encl_scope) = hir.get_enclosing_scope(id)
-                    && let Some(encl_def_id) = hir.opt_local_def_id(encl_scope)
-                    && let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id)
-                {
-                    let traits_str = ign_traits
-                        .iter()
-                        .map(|(trait_id, _)| format!("`{}`", self.tcx.item_name(*trait_id)))
-                        .collect::<Vec<_>>()
-                        .join(" and ");
-                    let plural_s = pluralize!(ign_traits.len());
-                    let article = if ign_traits.len() > 1 { "" } else { "a " };
-                    let is_are = if ign_traits.len() > 1 { "these are" } else { "this is" };
-                    let msg = format!(
-                        "`{}` has {}derived impl{} for the trait{} {}, but {} \
-                        intentionally ignored during dead code analysis",
-                        self.tcx.item_name(encl_def_id.to_def_id()),
-                        article,
-                        plural_s,
-                        plural_s,
-                        traits_str,
-                        is_are
-                    );
-                    err.note(&msg);
-                }
-                err.emit();
-            });
-        }
+        self.warn_multiple_dead_codes(&[(id, span, name)], participle, None);
     }
 }
 
@@ -790,15 +857,40 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
     // This visitor should only visit a single module at a time.
     fn visit_mod(&mut self, _: &'tcx hir::Mod<'tcx>, _: Span, _: hir::HirId) {}
 
+    fn visit_enum_def(
+        &mut self,
+        enum_definition: &'tcx hir::EnumDef<'tcx>,
+        generics: &'tcx hir::Generics<'tcx>,
+        item_id: hir::HirId,
+        _: Span,
+    ) {
+        intravisit::walk_enum_def(self, enum_definition, generics, item_id);
+        let dead_variants = enum_definition
+            .variants
+            .iter()
+            .filter_map(|variant| {
+                if self.should_warn_about_variant(&variant) {
+                    Some(DeadVariant {
+                        hir_id: variant.id,
+                        span: variant.span,
+                        name: variant.ident.name,
+                        level: self.tcx.lint_level_at_node(lint::builtin::DEAD_CODE, variant.id).0,
+                    })
+                } else {
+                    None
+                }
+            })
+            .collect();
+        self.warn_dead_fields_and_variants(item_id, "constructed", dead_variants)
+    }
+
     fn visit_variant(
         &mut self,
         variant: &'tcx hir::Variant<'tcx>,
         g: &'tcx hir::Generics<'tcx>,
         id: hir::HirId,
     ) {
-        if self.should_warn_about_variant(&variant) {
-            self.warn_dead_code(variant.id, variant.span, variant.ident.name, "constructed");
-        } else {
+        if !self.should_warn_about_variant(&variant) {
             intravisit::walk_variant(self, variant, g, id);
         }
     }
@@ -810,11 +902,35 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
         intravisit::walk_foreign_item(self, fi);
     }
 
-    fn visit_field_def(&mut self, field: &'tcx hir::FieldDef<'tcx>) {
-        if self.should_warn_about_field(&field) {
-            self.warn_dead_code(field.hir_id, field.span, field.ident.name, "read");
-        }
-        intravisit::walk_field_def(self, field);
+    fn visit_variant_data(
+        &mut self,
+        def: &'tcx hir::VariantData<'tcx>,
+        _: Symbol,
+        _: &hir::Generics<'_>,
+        id: hir::HirId,
+        _: rustc_span::Span,
+    ) {
+        intravisit::walk_struct_def(self, def);
+        let dead_fields = def
+            .fields()
+            .iter()
+            .filter_map(|field| {
+                if self.should_warn_about_field(&field) {
+                    Some(DeadVariant {
+                        hir_id: field.hir_id,
+                        span: field.span,
+                        name: field.ident.name,
+                        level: self
+                            .tcx
+                            .lint_level_at_node(lint::builtin::DEAD_CODE, field.hir_id)
+                            .0,
+                    })
+                } else {
+                    None
+                }
+            })
+            .collect();
+        self.warn_dead_fields_and_variants(id, "read", dead_fields)
     }
 
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) {
@@ -867,12 +983,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
 
 fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) {
     let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(());
-    let mut visitor = DeadVisitor {
-        tcx,
-        live_symbols,
-        ignored_derived_traits,
-        ignored_struct_def: FxHashSet::default(),
-    };
+    let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits };
     let (module, _, module_id) = tcx.hir().get_module(module);
     // Do not use an ItemLikeVisitor since we may want to skip visiting some items
     // when a surrounding one is warned against or `_`.

src/test/rustdoc-ui/display-output.stdout

@@ -24,7 +24,7 @@ warning: unused variable: `x`
 LL | fn foo(x: &dyn std::fmt::Display) {}
    |        ^ help: if this is intentional, prefix it with an underscore: `_x`
 
-warning: function is never used: `foo`
+warning: function `foo` is never used
   --> $DIR/display-output.rs:13:4
    |
 LL | fn foo(x: &dyn std::fmt::Display) {}

src/test/ui/associated-consts/associated-const-dead-code.rs

@@ -4,7 +4,7 @@ struct MyFoo;
 
 impl MyFoo {
     const BAR: u32 = 1;
-    //~^ ERROR associated constant is never used: `BAR`
+    //~^ ERROR associated constant `BAR` is never used
 }
 
 fn main() {

src/test/ui/associated-consts/associated-const-dead-code.stderr

@@ -1,4 +1,4 @@
-error: associated constant is never used: `BAR`
+error: associated constant `BAR` is never used
   --> $DIR/associated-const-dead-code.rs:6:5
    |
 LL |     const BAR: u32 = 1;

src/test/ui/closures/2229_closure_analysis/issue-87987.rs

@@ -2,16 +2,13 @@
 // edition:2021
 
 struct Props {
-    field_1: u32, //~ WARNING: field is never read: `field_1`
-    field_2: u32, //~ WARNING: field is never read: `field_2`
+    field_1: u32, //~ WARNING: fields `field_1` and `field_2` are never read
+    field_2: u32,
 }
 
 fn main() {
     // Test 1
-    let props_2 = Props {
-        field_1: 1,
-        field_2: 1,
-    };
+    let props_2 = Props { field_1: 1, field_2: 1 };
 
     let _ = || {
         let _: Props = props_2;
@@ -23,7 +20,7 @@ fn main() {
     let mref = &mut arr;
 
     let _c = || match arr {
-        [_, _, _, _] => println!("A")
+        [_, _, _, _] => println!("A"),
     };
 
     println!("{:#?}", mref);