Skip to content

Commit

Permalink
Rollup merge of #92783 - FabianWolff:issue-92726, r=nikomatsakis
Browse files Browse the repository at this point in the history
Annotate dead code lint with notes about ignored derived impls

Fixes #92726. CC `@pmetzger,` is this what you had in mind?

r? `@nikomatsakis`
  • Loading branch information
matthiaskrgr authored Jan 19, 2022
2 parents 3148a32 + 8b459dd commit 9a82f74
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 6 deletions.
60 changes: 54 additions & 6 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -15,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
Expand Down Expand Up @@ -47,6 +49,10 @@ struct MarkSymbolVisitor<'tcx> {
ignore_variant_stack: Vec<DefId>,
// maps from tuple struct constructors to tuple struct items
struct_constructors: FxHashMap<LocalDefId, LocalDefId>,
// maps from ADTs to ignored derived traits (e.g. Debug and Clone)
// and the span of their respective impl (i.e., part of the derive
// macro)
ignored_derived_traits: FxHashMap<DefId, Vec<(Span, DefId)>>,
}

impl<'tcx> MarkSymbolVisitor<'tcx> {
Expand Down Expand Up @@ -242,14 +248,24 @@ 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;
}

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() {
let impl_span = self.tcx.def_span(impl_of);
if let Some(v) = self.ignored_derived_traits.get_mut(&adt_def.did) {
v.push((impl_span, trait_of));
} else {
self.ignored_derived_traits
.insert(adt_def.did, vec![(impl_span, trait_of)]);
}
}
return true;
}
}
Expand Down Expand Up @@ -571,7 +587,7 @@ fn create_and_seed_worklist<'tcx>(
fn find_live<'tcx>(
tcx: TyCtxt<'tcx>,
access_levels: &privacy::AccessLevels,
) -> FxHashSet<LocalDefId> {
) -> (FxHashSet<LocalDefId>, FxHashMap<DefId, Vec<(Span, DefId)>>) {
let (worklist, struct_constructors) = create_and_seed_worklist(tcx, access_levels);
let mut symbol_visitor = MarkSymbolVisitor {
worklist,
Expand All @@ -584,14 +600,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<LocalDefId>,
ignored_derived_traits: FxHashMap<DefId, Vec<(Span, DefId)>>,
}

impl<'tcx> DeadVisitor<'tcx> {
Expand Down Expand Up @@ -660,7 +678,37 @@ 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::<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
);
let multispan = ign_traits.iter().map(|(s, _)| *s).collect::<Vec<_>>();
err.span_note(multispan, &msg);
}
}
}
err.emit();
});
}
}
Expand Down Expand Up @@ -790,7 +838,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);
}
6 changes: 6 additions & 0 deletions src/test/ui/derive-uninhabited-enum-38885.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +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 intentionally ignored during dead code analysis
--> $DIR/derive-uninhabited-enum-38885.rs:10:10
|
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

21 changes: 21 additions & 0 deletions src/test/ui/derives/clone-debug-dead-code.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,39 @@ error: field is never read: `f`
|
LL | struct B { f: () }
| ^^^^^
|
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 | #[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
|
LL | struct C { f: () }
| ^^^^^
|
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 | #[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
|
LL | struct D { f: () }
| ^^^^^
|
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 | #[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
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/lint/dead-code/unused-variant.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +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 intentionally ignored during dead code analysis
--> $DIR/unused-variant.rs:3:10
|
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

0 comments on commit 9a82f74

Please sign in to comment.