From bd1f09d417c8b0e460f90bcd23b9067d55f7f7dd Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Tue, 11 Jan 2022 21:06:18 +0100 Subject: [PATCH 1/2] Annotate dead code lint with notes about ignored derived impls --- compiler/rustc_passes/src/dead.rs | 52 ++++++++++++++++--- .../ui/derive-uninhabited-enum-38885.stderr | 5 ++ .../ui/derives/clone-debug-dead-code.stderr | 18 +++++++ .../ui/lint/dead-code/unused-variant.stderr | 5 ++ 4 files changed, 74 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 3b15332c678fd..9db9140f71ea8 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -3,6 +3,7 @@ // from live codes are live, and everything else is dead. use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::pluralize; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -47,6 +48,8 @@ struct MarkSymbolVisitor<'tcx> { ignore_variant_stack: Vec, // maps from tuple struct constructors to tuple struct items struct_constructors: FxHashMap, + // maps from ADTs to ignored derived traits (e.g. Debug and Clone) + ignored_derived_traits: FxHashMap>, } impl<'tcx> MarkSymbolVisitor<'tcx> { @@ -242,7 +245,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { /// Automatically generated items marked with `rustc_trivial_field_reads` /// will be ignored for the purposes of dead code analysis (see PR #85200 /// for discussion). - fn should_ignore_item(&self, def_id: DefId) -> bool { + fn should_ignore_item(&mut self, def_id: DefId) -> bool { if let Some(impl_of) = self.tcx.impl_of_method(def_id) { if !self.tcx.has_attr(impl_of, sym::automatically_derived) { return false; @@ -250,6 +253,14 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { if let Some(trait_of) = self.tcx.trait_id_of_impl(impl_of) { if self.tcx.has_attr(trait_of, sym::rustc_trivial_field_reads) { + let trait_ref = self.tcx.impl_trait_ref(impl_of).unwrap(); + if let ty::Adt(adt_def, _) = trait_ref.self_ty().kind() { + if let Some(v) = self.ignored_derived_traits.get_mut(&adt_def.did) { + v.push(trait_of); + } else { + self.ignored_derived_traits.insert(adt_def.did, vec![trait_of]); + } + } return true; } } @@ -577,7 +588,7 @@ fn create_and_seed_worklist<'tcx>( fn find_live<'tcx>( tcx: TyCtxt<'tcx>, access_levels: &privacy::AccessLevels, -) -> FxHashSet { +) -> (FxHashSet, FxHashMap>) { let (worklist, struct_constructors) = create_and_seed_worklist(tcx, access_levels); let mut symbol_visitor = MarkSymbolVisitor { worklist, @@ -590,14 +601,16 @@ fn find_live<'tcx>( pub_visibility: false, ignore_variant_stack: vec![], struct_constructors, + ignored_derived_traits: FxHashMap::default(), }; symbol_visitor.mark_live_symbols(); - symbol_visitor.live_symbols + (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits) } struct DeadVisitor<'tcx> { tcx: TyCtxt<'tcx>, live_symbols: FxHashSet, + ignored_derived_traits: FxHashMap>, } impl<'tcx> DeadVisitor<'tcx> { @@ -666,7 +679,34 @@ impl<'tcx> DeadVisitor<'tcx> { 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()); - lint.build(&format!("{} is never {}: `{}`", descr, participle, name)).emit() + let mut err = lint.build(&format!("{} is never {}: `{}`", descr, participle, name)); + let hir = self.tcx.hir(); + if let Some(encl_scope) = hir.get_enclosing_scope(id) { + if let Some(encl_def_id) = hir.opt_local_def_id(encl_scope) { + if let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id.to_def_id()) { + let traits_str = ign_traits + .iter() + .map(|t| format!("`{}`", self.tcx.item_name(*t))).collect::>() + .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 {} ignored during dead code analysis", + self.tcx.item_name(encl_def_id.to_def_id()), + article, + plural_s, + plural_s, + traits_str, + is_are); + if let Some(span) = self.tcx.def_ident_span(encl_def_id) { + err.span_note(span, &msg); + } else { + err.note(&msg); + } + } + } + } + err.emit(); }); } } @@ -796,7 +836,7 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> { pub fn check_crate(tcx: TyCtxt<'_>) { let access_levels = &tcx.privacy_access_levels(()); - let live_symbols = find_live(tcx, access_levels); - let mut visitor = DeadVisitor { tcx, live_symbols }; + let (live_symbols, ignored_derived_traits) = find_live(tcx, access_levels); + let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits }; tcx.hir().walk_toplevel_module(&mut visitor); } diff --git a/src/test/ui/derive-uninhabited-enum-38885.stderr b/src/test/ui/derive-uninhabited-enum-38885.stderr index 72607629d3c10..feaadd201b306 100644 --- a/src/test/ui/derive-uninhabited-enum-38885.stderr +++ b/src/test/ui/derive-uninhabited-enum-38885.stderr @@ -5,6 +5,11 @@ LL | Void(Void), | ^^^^^^^^^^ | = note: `-W dead-code` implied by `-W unused` +note: `Foo` has a derived impl for the trait `Debug`, but this is ignored during dead code analysis + --> $DIR/derive-uninhabited-enum-38885.rs:11:6 + | +LL | enum Foo { + | ^^^ warning: 1 warning emitted diff --git a/src/test/ui/derives/clone-debug-dead-code.stderr b/src/test/ui/derives/clone-debug-dead-code.stderr index 226007f3647b1..d7cab0815b8f0 100644 --- a/src/test/ui/derives/clone-debug-dead-code.stderr +++ b/src/test/ui/derives/clone-debug-dead-code.stderr @@ -15,18 +15,36 @@ error: field is never read: `f` | LL | struct B { f: () } | ^^^^^ + | +note: `B` has a derived impl for the trait `Clone`, but this is ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:10:8 + | +LL | struct B { f: () } + | ^ error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:14:12 | LL | struct C { f: () } | ^^^^^ + | +note: `C` has a derived impl for the trait `Debug`, but this is ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:14:8 + | +LL | struct C { f: () } + | ^ error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:18:12 | LL | struct D { f: () } | ^^^^^ + | +note: `D` has derived impls for the traits `Clone` and `Debug`, but these are ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:18:8 + | +LL | struct D { f: () } + | ^ error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:21:12 diff --git a/src/test/ui/lint/dead-code/unused-variant.stderr b/src/test/ui/lint/dead-code/unused-variant.stderr index a547f5af4b082..abac1f29ce24b 100644 --- a/src/test/ui/lint/dead-code/unused-variant.stderr +++ b/src/test/ui/lint/dead-code/unused-variant.stderr @@ -9,6 +9,11 @@ note: the lint level is defined here | LL | #![deny(dead_code)] | ^^^^^^^^^ +note: `Enum` has a derived impl for the trait `Clone`, but this is ignored during dead code analysis + --> $DIR/unused-variant.rs:4:6 + | +LL | enum Enum { + | ^^^^ error: aborting due to previous error From 8b459dd73887732d4b2741b2459a2edd60a47229 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sat, 15 Jan 2022 21:01:44 +0100 Subject: [PATCH 2/2] Use span of ignored impls for explanatory note --- compiler/rustc_passes/src/dead.rs | 46 +++++++++++-------- .../ui/derive-uninhabited-enum-38885.stderr | 9 ++-- .../ui/derives/clone-debug-dead-code.stderr | 27 ++++++----- .../ui/lint/dead-code/unused-variant.stderr | 9 ++-- 4 files changed, 52 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 9db9140f71ea8..3e4e9f205a45c 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -16,6 +16,7 @@ use rustc_middle::middle::privacy; use rustc_middle::ty::{self, DefIdTree, TyCtxt}; use rustc_session::lint; use rustc_span::symbol::{sym, Symbol}; +use rustc_span::Span; use std::mem; // Any local node that may call something in its body block should be @@ -49,7 +50,9 @@ struct MarkSymbolVisitor<'tcx> { // maps from tuple struct constructors to tuple struct items struct_constructors: FxHashMap, // maps from ADTs to ignored derived traits (e.g. Debug and Clone) - ignored_derived_traits: FxHashMap>, + // and the span of their respective impl (i.e., part of the derive + // macro) + ignored_derived_traits: FxHashMap>, } impl<'tcx> MarkSymbolVisitor<'tcx> { @@ -255,10 +258,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { if self.tcx.has_attr(trait_of, sym::rustc_trivial_field_reads) { let trait_ref = self.tcx.impl_trait_ref(impl_of).unwrap(); if let ty::Adt(adt_def, _) = trait_ref.self_ty().kind() { + let impl_span = self.tcx.def_span(impl_of); if let Some(v) = self.ignored_derived_traits.get_mut(&adt_def.did) { - v.push(trait_of); + v.push((impl_span, trait_of)); } else { - self.ignored_derived_traits.insert(adt_def.did, vec![trait_of]); + self.ignored_derived_traits + .insert(adt_def.did, vec![(impl_span, trait_of)]); } } return true; @@ -588,7 +593,7 @@ fn create_and_seed_worklist<'tcx>( fn find_live<'tcx>( tcx: TyCtxt<'tcx>, access_levels: &privacy::AccessLevels, -) -> (FxHashSet, FxHashMap>) { +) -> (FxHashSet, FxHashMap>) { let (worklist, struct_constructors) = create_and_seed_worklist(tcx, access_levels); let mut symbol_visitor = MarkSymbolVisitor { worklist, @@ -610,7 +615,7 @@ fn find_live<'tcx>( struct DeadVisitor<'tcx> { tcx: TyCtxt<'tcx>, live_symbols: FxHashSet, - ignored_derived_traits: FxHashMap>, + ignored_derived_traits: FxHashMap>, } impl<'tcx> DeadVisitor<'tcx> { @@ -683,26 +688,29 @@ impl<'tcx> DeadVisitor<'tcx> { let hir = self.tcx.hir(); if let Some(encl_scope) = hir.get_enclosing_scope(id) { if let Some(encl_def_id) = hir.opt_local_def_id(encl_scope) { - if let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id.to_def_id()) { + if let Some(ign_traits) = + self.ignored_derived_traits.get(&encl_def_id.to_def_id()) + { let traits_str = ign_traits .iter() - .map(|t| format!("`{}`", self.tcx.item_name(*t))).collect::>() + .map(|(_, t)| format!("`{}`", self.tcx.item_name(*t))) + .collect::>() .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 {} ignored during dead code analysis", - self.tcx.item_name(encl_def_id.to_def_id()), - article, - plural_s, - plural_s, - traits_str, - is_are); - if let Some(span) = self.tcx.def_ident_span(encl_def_id) { - err.span_note(span, &msg); - } else { - err.note(&msg); - } + 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 + ); + let multispan = ign_traits.iter().map(|(s, _)| *s).collect::>(); + err.span_note(multispan, &msg); } } } diff --git a/src/test/ui/derive-uninhabited-enum-38885.stderr b/src/test/ui/derive-uninhabited-enum-38885.stderr index feaadd201b306..2a44e56a3302c 100644 --- a/src/test/ui/derive-uninhabited-enum-38885.stderr +++ b/src/test/ui/derive-uninhabited-enum-38885.stderr @@ -5,11 +5,12 @@ LL | Void(Void), | ^^^^^^^^^^ | = note: `-W dead-code` implied by `-W unused` -note: `Foo` has a derived impl for the trait `Debug`, but this is ignored during dead code analysis - --> $DIR/derive-uninhabited-enum-38885.rs:11:6 +note: `Foo` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis + --> $DIR/derive-uninhabited-enum-38885.rs:10:10 | -LL | enum Foo { - | ^^^ +LL | #[derive(Debug)] + | ^^^^^ + = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) warning: 1 warning emitted diff --git a/src/test/ui/derives/clone-debug-dead-code.stderr b/src/test/ui/derives/clone-debug-dead-code.stderr index d7cab0815b8f0..67bb574315a72 100644 --- a/src/test/ui/derives/clone-debug-dead-code.stderr +++ b/src/test/ui/derives/clone-debug-dead-code.stderr @@ -16,11 +16,12 @@ error: field is never read: `f` LL | struct B { f: () } | ^^^^^ | -note: `B` has a derived impl for the trait `Clone`, but this is ignored during dead code analysis - --> $DIR/clone-debug-dead-code.rs:10:8 +note: `B` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:9:10 | -LL | struct B { f: () } - | ^ +LL | #[derive(Clone)] + | ^^^^^ + = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:14:12 @@ -28,11 +29,12 @@ error: field is never read: `f` LL | struct C { f: () } | ^^^^^ | -note: `C` has a derived impl for the trait `Debug`, but this is ignored during dead code analysis - --> $DIR/clone-debug-dead-code.rs:14:8 +note: `C` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:13:10 | -LL | struct C { f: () } - | ^ +LL | #[derive(Debug)] + | ^^^^^ + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:18:12 @@ -40,11 +42,12 @@ error: field is never read: `f` LL | struct D { f: () } | ^^^^^ | -note: `D` has derived impls for the traits `Clone` and `Debug`, but these are ignored during dead code analysis - --> $DIR/clone-debug-dead-code.rs:18:8 +note: `D` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis + --> $DIR/clone-debug-dead-code.rs:17:10 | -LL | struct D { f: () } - | ^ +LL | #[derive(Debug,Clone)] + | ^^^^^ ^^^^^ + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) error: field is never read: `f` --> $DIR/clone-debug-dead-code.rs:21:12 diff --git a/src/test/ui/lint/dead-code/unused-variant.stderr b/src/test/ui/lint/dead-code/unused-variant.stderr index abac1f29ce24b..3b5683a7748fa 100644 --- a/src/test/ui/lint/dead-code/unused-variant.stderr +++ b/src/test/ui/lint/dead-code/unused-variant.stderr @@ -9,11 +9,12 @@ note: the lint level is defined here | LL | #![deny(dead_code)] | ^^^^^^^^^ -note: `Enum` has a derived impl for the trait `Clone`, but this is ignored during dead code analysis - --> $DIR/unused-variant.rs:4:6 +note: `Enum` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis + --> $DIR/unused-variant.rs:3:10 | -LL | enum Enum { - | ^^^^ +LL | #[derive(Clone)] + | ^^^^^ + = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to previous error