From eb4ba58572bfaec9a9ca843b368e05564d9b1cb0 Mon Sep 17 00:00:00 2001
From: Erik Desjardins <erikdesjardins@users.noreply.github.com>
Date: Fri, 1 Oct 2021 22:07:22 -0400
Subject: [PATCH 1/2] perf: only check for rustc_trivial_field_reads attribute
on traits
The checks removed here caused a small perf regression:
https://github.com/rust-lang/rust/pull/88824#issuecomment-932664761
Since the attribute is currently only applied to traits, I don't think
it's worth keeping the additional checks for now.
If/when we decide to apply the attribute somewhere else, we can
(partially) revert this and evaluate if the perf impact is acceptable.
---
compiler/rustc_passes/src/dead.rs | 24 +-----------------------
1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index d05961b7e37..c6f34aad978 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/compiler/rustc_passes/src/dead.rs
@@ -254,31 +254,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
let has_attr = |def_id| self.tcx.has_attr(def_id, sym::rustc_trivial_field_reads);
- if has_attr(def_id) {
- return true;
- }
-
if let Some(impl_of) = self.tcx.impl_of_method(def_id) {
- if has_attr(impl_of) {
- return true;
- }
-
if let Some(trait_of) = self.tcx.trait_id_of_impl(impl_of) {
if has_attr(trait_of) {
return true;
}
-
- if let Some(method_ident) = self.tcx.opt_item_name(def_id) {
- if let Some(trait_method) = self
- .tcx
- .associated_items(trait_of)
- .find_by_name_and_kind(self.tcx, method_ident, ty::AssocKind::Fn, trait_of)
- {
- if has_attr(trait_method.def_id) {
- return true;
- }
- }
- }
}
} else if let Some(trait_of) = self.tcx.trait_of_item(def_id) {
if has_attr(trait_of) {
@@ -291,9 +271,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
fn visit_node(&mut self, node: Node<'tcx>) {
if let Some(item_def_id) = match node {
- Node::Item(hir::Item { def_id, .. })
- | Node::ForeignItem(hir::ForeignItem { def_id, .. })
- | Node::TraitItem(hir::TraitItem { def_id, .. })
+ Node::TraitItem(hir::TraitItem { def_id, .. })
| Node::ImplItem(hir::ImplItem { def_id, .. }) => Some(def_id.to_def_id()),
_ => None,
} {
From bec5a91450d58a821c8a41a93dd2563fc7eb4606 Mon Sep 17 00:00:00 2001
From: Erik Desjardins <erikdesjardins@users.noreply.github.com>
Date: Sat, 2 Oct 2021 09:53:51 -0400
Subject: [PATCH 2/2] only check for automatically_derived on impls, not
individual methods
this matches behavior of existing code
https://github.com/rust-lang/rust/blob/b27661eb33c74cb514dba059b47d86b6582ac1c2/compiler/rustc_passes/src/liveness.rs#L326-L333
---
compiler/rustc_passes/src/dead.rs | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index c6f34aad978..77279132401 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/compiler/rustc_passes/src/dead.rs
@@ -243,27 +243,16 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
/// will be ignored for the purposes of dead code analysis (see PR #85200
/// for discussion).
fn should_ignore_item(&self, def_id: DefId) -> bool {
- if !self.tcx.has_attr(def_id, sym::automatically_derived)
- && !self
- .tcx
- .impl_of_method(def_id)
- .map_or(false, |impl_id| self.tcx.has_attr(impl_id, sym::automatically_derived))
- {
- return false;
- }
-
- let has_attr = |def_id| self.tcx.has_attr(def_id, sym::rustc_trivial_field_reads);
-
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 has_attr(trait_of) {
+ if self.tcx.has_attr(trait_of, sym::rustc_trivial_field_reads) {
return true;
}
}
- } else if let Some(trait_of) = self.tcx.trait_of_item(def_id) {
- if has_attr(trait_of) {
- return true;
- }
}
return false;
@@ -271,8 +260,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
fn visit_node(&mut self, node: Node<'tcx>) {
if let Some(item_def_id) = match node {
- Node::TraitItem(hir::TraitItem { def_id, .. })
- | Node::ImplItem(hir::ImplItem { def_id, .. }) => Some(def_id.to_def_id()),
+ Node::ImplItem(hir::ImplItem { def_id, .. }) => Some(def_id.to_def_id()),
_ => None,
} {
if self.should_ignore_item(item_def_id) {