Uniform spans in dead code lint.

Author: cjgillot

compiler/rustc_passes/src/dead.rs

@@ -635,8 +635,7 @@ fn live_symbols_and_ignored_derived_traits<'tcx>(
 }
 
 struct DeadVariant {
-    hir_id: hir::HirId,
-    span: Span,
+    def_id: LocalDefId,
     name: Symbol,
     level: lint::Level,
 }
@@ -687,29 +686,39 @@ impl<'tcx> DeadVisitor<'tcx> {
 
     fn warn_multiple_dead_codes(
         &self,
-        dead_codes: &[(hir::HirId, Span, Symbol)],
+        dead_codes: &[LocalDefId],
         participle: &str,
-        parent_hir_id: Option<hir::HirId>,
+        parent_item: Option<LocalDefId>,
     ) {
-        if let Some((id, _, name)) = dead_codes.first()
-            && !name.as_str().starts_with('_')
-        {
-            self.tcx.struct_span_lint_hir(
+        if let Some(&first_id) = dead_codes.first() {
+            let tcx = self.tcx;
+            let names: Vec<_> = dead_codes
+                .iter()
+                .map(|&def_id| tcx.item_name(def_id.to_def_id()).to_string())
+                .collect();
+            let spans = dead_codes
+                .iter()
+                .map(|&def_id| match tcx.def_ident_span(def_id) {
+                    Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()),
+                    None => tcx.def_span(def_id),
+                })
+                .collect();
+
+            tcx.struct_span_lint_hir(
                 lint::builtin::DEAD_CODE,
-                *id,
-                MultiSpan::from_spans(
-                    dead_codes.iter().map(|(_, span, _)| *span).collect(),
-                ),
+                tcx.hir().local_def_id_to_hir_id(first_id),
+                MultiSpan::from_spans(spans),
                 |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 descr = tcx.def_kind(first_id).descr(first_id.to_def_id());
                     let span_len = dead_codes.len();
-                    let names = match &dead_codes.iter().map(|(_, _, n)| n.to_string()).collect::<Vec<_>>()[..]
-                    {
+                    let names = match &names[..] {
                         _ if span_len > 6 => String::new(),
                         [name] => format!("`{name}` "),
                         [names @ .., last] => {
-                            format!("{} and `{last}` ", names.iter().map(|name| format!("`{name}`")).join(", "))
+                            format!(
+                                "{} and `{last}` ",
+                                names.iter().map(|name| format!("`{name}`")).join(", ")
+                            )
                         }
                         [] => unreachable!(),
                     };
@@ -719,25 +728,17 @@ impl<'tcx> DeadVisitor<'tcx> {
                         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());
+
+                    if let Some(parent_item) = parent_item {
+                        let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id());
                         err.span_label(
-                            item.ident.span,
-                            format!(
-                                "{descr}{s} in this {parent_descr}",
-                                s = pluralize!(span_len)
-                            ),
+                            tcx.def_ident_span(parent_item).unwrap(),
+                            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 encl_def_id = parent_item.unwrap_or(first_id);
+                    if 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)))
@@ -758,15 +759,15 @@ impl<'tcx> DeadVisitor<'tcx> {
                         );
                         err.note(&msg);
                     }
-                        err.emit();
-                    },
+                    err.emit();
+                },
             );
         }
     }
 
     fn warn_dead_fields_and_variants(
         &self,
-        hir_id: hir::HirId,
+        def_id: LocalDefId,
         participle: &str,
         dead_codes: Vec<DeadVariant>,
     ) {
@@ -781,23 +782,18 @@ impl<'tcx> DeadVisitor<'tcx> {
         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)>>(),
+                &group.map(|v| v.def_id).collect::<Vec<_>>(),
                 participle,
-                Some(hir_id),
+                Some(def_id),
             );
         }
     }
 
-    fn warn_dead_code(
-        &mut self,
-        id: hir::HirId,
-        span: rustc_span::Span,
-        name: Symbol,
-        participle: &str,
-    ) {
-        self.warn_multiple_dead_codes(&[(id, span, name)], participle, None);
+    fn warn_dead_code(&mut self, id: LocalDefId, participle: &str) {
+        if self.tcx.item_name(id.to_def_id()).as_str().starts_with('_') {
+            return;
+        }
+        self.warn_multiple_dead_codes(&[id], participle, None);
     }
 }
 
@@ -815,33 +811,11 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
         if self.should_warn_about_item(item) {
             // For most items, we want to highlight its identifier
-            let span = match item.kind {
-                hir::ItemKind::Fn(..)
-                | hir::ItemKind::Mod(..)
-                | hir::ItemKind::Enum(..)
-                | hir::ItemKind::Struct(..)
-                | hir::ItemKind::Union(..)
-                | hir::ItemKind::Trait(..)
-                | hir::ItemKind::Impl { .. } => {
-                    // FIXME(66095): Because item.span is annotated with things
-                    // like expansion data, and ident.span isn't, we use the
-                    // def_span method if it's part of a macro invocation
-                    // (and thus has a source_callee set).
-                    // We should probably annotate ident.span with the macro
-                    // context, but that's a larger change.
-                    if item.span.source_callee().is_some() {
-                        self.tcx.sess.source_map().guess_head_span(item.span)
-                    } else {
-                        item.ident.span
-                    }
-                }
-                _ => item.span,
-            };
             let participle = match item.kind {
                 hir::ItemKind::Struct(..) => "constructed", // Issue #52325
                 _ => "used",
             };
-            self.warn_dead_code(item.hir_id(), span, item.ident.name, participle);
+            self.warn_dead_code(item.def_id, participle);
         } else {
             // Only continue if we didn't warn
             intravisit::walk_item(self, item);
@@ -865,8 +839,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
             .filter_map(|variant| {
                 if self.should_warn_about_variant(&variant) {
                     Some(DeadVariant {
-                        hir_id: variant.id,
-                        span: variant.span,
+                        def_id: self.tcx.hir().local_def_id(variant.id),
                         name: variant.ident.name,
                         level: self.tcx.lint_level_at_node(lint::builtin::DEAD_CODE, variant.id).0,
                     })
@@ -875,7 +848,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
                 }
             })
             .collect();
-        self.warn_dead_fields_and_variants(item_id, "constructed", dead_variants)
+        self.warn_dead_fields_and_variants(item_id.expect_owner(), "constructed", dead_variants)
     }
 
     fn visit_variant(
@@ -891,7 +864,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
 
     fn visit_foreign_item(&mut self, fi: &'tcx hir::ForeignItem<'tcx>) {
         if self.should_warn_about_foreign_item(fi) {
-            self.warn_dead_code(fi.hir_id(), fi.span, fi.ident.name, "used");
+            self.warn_dead_code(fi.def_id, "used");
         }
         intravisit::walk_foreign_item(self, fi);
     }
@@ -911,8 +884,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
             .filter_map(|field| {
                 if self.should_warn_about_field(&field) {
                     Some(DeadVariant {
-                        hir_id: field.hir_id,
-                        span: field.span,
+                        def_id: self.tcx.hir().local_def_id(field.hir_id),
                         name: field.ident.name,
                         level: self
                             .tcx
@@ -924,36 +896,20 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> {
                 }
             })
             .collect();
-        self.warn_dead_fields_and_variants(id, "read", dead_fields)
+        self.warn_dead_fields_and_variants(self.tcx.hir().local_def_id(id), "read", dead_fields)
     }
 
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) {
         match impl_item.kind {
             hir::ImplItemKind::Const(_, body_id) => {
                 if !self.symbol_is_live(impl_item.def_id) {
-                    self.warn_dead_code(
-                        impl_item.hir_id(),
-                        impl_item.span,
-                        impl_item.ident.name,
-                        "used",
-                    );
+                    self.warn_dead_code(impl_item.def_id, "used");
                 }
                 self.visit_nested_body(body_id)
             }
             hir::ImplItemKind::Fn(_, body_id) => {
                 if !self.symbol_is_live(impl_item.def_id) {
-                    // FIXME(66095): Because impl_item.span is annotated with things
-                    // like expansion data, and ident.span isn't, we use the
-                    // def_span method if it's part of a macro invocation
-                    // (and thus has a source_callee set).
-                    // We should probably annotate ident.span with the macro
-                    // context, but that's a larger change.
-                    let span = if impl_item.span.source_callee().is_some() {
-                        self.tcx.sess.source_map().guess_head_span(impl_item.span)
-                    } else {
-                        impl_item.ident.span
-                    };
-                    self.warn_dead_code(impl_item.hir_id(), span, impl_item.ident.name, "used");
+                    self.warn_dead_code(impl_item.def_id, "used");
                 }
                 self.visit_nested_body(body_id)
             }

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

@@ -1,8 +1,8 @@
 error: associated constant `BAR` is never used
-  --> $DIR/associated-const-dead-code.rs:6:5
+  --> $DIR/associated-const-dead-code.rs:6:11
    |
 LL |     const BAR: u32 = 1;
-   |     ^^^^^^^^^^^^^^^^^^^
+   |           ^^^
    |
 note: the lint level is defined here
   --> $DIR/associated-const-dead-code.rs:1:9

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

@@ -4,9 +4,9 @@ warning: fields `field_1` and `field_2` are never read
 LL | struct Props {
    |        ----- fields in this struct
 LL |     field_1: u32,
-   |     ^^^^^^^^^^^^
+   |     ^^^^^^^
 LL |     field_2: u32,
-   |     ^^^^^^^^^^^^
+   |     ^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default
 

src/test/ui/derive-uninhabited-enum-38885.stderr

@@ -5,7 +5,7 @@ LL | enum Foo {
    |      --- variant in this enum
 LL |     Bar(u8),
 LL |     Void(Void),
-   |     ^^^^^^^^^^
+   |     ^^^^
    |
    = note: `-W dead-code` implied by `-W unused`
    = note: `Foo` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis

src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr

@@ -5,13 +5,13 @@ LL | pub struct Whatever {
    |            -------- fields in this struct
 LL |     pub field0: (),
 LL |     field1: (),
-   |     ^^^^^^^^^^
+   |     ^^^^^^
 LL |     field2: (),
-   |     ^^^^^^^^^^
+   |     ^^^^^^
 LL |     field3: (),
-   |     ^^^^^^^^^^
+   |     ^^^^^^
 LL |     field4: (),
-   |     ^^^^^^^^^^
+   |     ^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/clone-debug-dead-code-in-the-same-struct.rs:1:11

src/test/ui/derives/clone-debug-dead-code.stderr

@@ -2,7 +2,7 @@ error: field `f` is never read
   --> $DIR/clone-debug-dead-code.rs:6:12
    |
 LL | struct A { f: () }
-   |        -   ^^^^^
+   |        -   ^
    |        |
    |        field in this struct
    |
@@ -16,7 +16,7 @@ error: field `f` is never read
   --> $DIR/clone-debug-dead-code.rs:10:12
    |
 LL | struct B { f: () }
-   |        -   ^^^^^
+   |        -   ^
    |        |
    |        field in this struct
    |
@@ -26,7 +26,7 @@ error: field `f` is never read
   --> $DIR/clone-debug-dead-code.rs:14:12
    |
 LL | struct C { f: () }
-   |        -   ^^^^^
+   |        -   ^
    |        |
    |        field in this struct
    |
@@ -36,7 +36,7 @@ error: field `f` is never read
   --> $DIR/clone-debug-dead-code.rs:18:12
    |
 LL | struct D { f: () }
-   |        -   ^^^^^
+   |        -   ^
    |        |
    |        field in this struct
    |
@@ -46,7 +46,7 @@ error: field `f` is never read
   --> $DIR/clone-debug-dead-code.rs:21:12
    |
 LL | struct E { f: () }
-   |        -   ^^^^^
+   |        -   ^
    |        |
    |        field in this struct
 

src/test/ui/issues/issue-37515.stderr

@@ -1,8 +1,8 @@
 warning: type alias `Z` is never used
-  --> $DIR/issue-37515.rs:5:1
+  --> $DIR/issue-37515.rs:5:6
    |
 LL | type Z = dyn for<'x> Send;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |      ^
    |
 note: the lint level is defined here
   --> $DIR/issue-37515.rs:3:9

src/test/ui/lint/dead-code/impl-trait.stderr

@@ -1,8 +1,8 @@
 error: type alias `Unused` is never used
-  --> $DIR/impl-trait.rs:12:1
+  --> $DIR/impl-trait.rs:12:6
    |
 LL | type Unused = ();
-   | ^^^^^^^^^^^^^^^^^
+   |      ^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/impl-trait.rs:1:9