Rollup merge of #85200 - FabianWolff:issue-84647, r=nikomatsakis

Ignore derived Clone and Debug implementations during dead code analysis

This pull request fixes #84647. Derived implementations of `Clone` and `Debug` always trivially read all fields, so "field is never read" dead code warnings are never triggered. Arguably, though, a user most likely will only be interested in whether _their_ code ever reads those fields, which is the behavior I have implemented here.

Note that implementations of `Clone` and `Debug` are only ignored if they are `#[derive(...)]`d; a custom `impl Clone/Debug for ...` will still be analyzed normally (i.e. if a custom `Clone` implementation uses all fields of the struct, this will continue to suppress dead code warnings about unused fields); this seemed like the least intrusive change to me (although it would be easy to change — just drop the `&& [impl_]item.span.in_derive_expansion()` in the if conditions).

The only thing that I am slightly unsure about is that in #84647, `@matklad` said
> Doesn't seem easy to fix though :(

However, it _was_ pretty straightforward to fix, so did I perhaps overlook something obvious? `@matklad,` could you weigh in on this?

Author: Manishearth

compiler/rustc_feature/src/accepted.rs

@@ -16,7 +16,6 @@ macro_rules! declare_features {
                     since: $ver,
                     issue: to_nonzero($issue),
                     edition: None,
-                    description: concat!($($doc,)*),
                 }
             ),+
         ];

compiler/rustc_feature/src/active.rs

@@ -37,7 +37,6 @@ macro_rules! declare_features {
                     since: $ver,
                     issue: to_nonzero($issue),
                     edition: $edition,
-                    description: concat!($($doc,)*),
                 }
             ),+];
 

compiler/rustc_feature/src/builtin_attrs.rs

@@ -453,6 +453,9 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ),
     // Enumerates "identity-like" conversion methods to suggest on type mismatch.
     rustc_attr!(rustc_conversion_suggestion, Normal, template!(Word), INTERNAL_UNSTABLE),
+    // Prevents field reads in the marked trait or method to be considered
+    // during dead code analysis.
+    rustc_attr!(rustc_trivial_field_reads, Normal, template!(Word), INTERNAL_UNSTABLE),
 
     // ==========================================================================
     // Internal attributes, Const related:

compiler/rustc_feature/src/lib.rs

@@ -51,7 +51,6 @@ pub struct Feature {
     pub since: &'static str,
     issue: Option<NonZeroU32>,
     pub edition: Option<Edition>,
-    description: &'static str,
 }
 
 #[derive(Copy, Clone, Debug)]

compiler/rustc_feature/src/removed.rs

@@ -16,7 +16,6 @@ macro_rules! declare_features {
                     since: $ver,
                     issue: to_nonzero($issue),
                     edition: None,
-                    description: concat!($($doc,)*),
                 }
             ),+
         ];
@@ -34,7 +33,6 @@ macro_rules! declare_features {
                     since: $ver,
                     issue: to_nonzero($issue),
                     edition: None,
-                    description: concat!($($doc,)*),
                 }
             ),+
         ];

compiler/rustc_middle/src/hir/map/collector.rs

@@ -62,13 +62,6 @@ fn hash_body(
     stable_hasher.finish()
 }
 
-/// Represents an entry and its parent `HirId`.
-#[derive(Copy, Clone, Debug)]
-pub struct Entry<'hir> {
-    parent: HirId,
-    node: Node<'hir>,
-}
-
 impl<'a, 'hir> NodeCollector<'a, 'hir> {
     pub(super) fn root(
         sess: &'a Session,

compiler/rustc_middle/src/ich/hcx.rs

@@ -28,7 +28,6 @@ fn compute_ignored_attr_names() -> FxHashSet<Symbol> {
 /// things (e.g., each `DefId`/`DefPath` is only hashed once).
 #[derive(Clone)]
 pub struct StableHashingContext<'a> {
-    sess: &'a Session,
     definitions: &'a Definitions,
     cstore: &'a dyn CrateStore,
     pub(super) body_resolver: BodyResolver<'a>,
@@ -78,7 +77,6 @@ impl<'a> StableHashingContext<'a> {
             !always_ignore_spans && !sess.opts.debugging_opts.incremental_ignore_spans;
 
         StableHashingContext {
-            sess,
             body_resolver: BodyResolver(krate),
             definitions,
             cstore,

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -900,10 +900,7 @@ fn traverse_candidate<'pat, 'tcx: 'pat, C, T, I>(
 struct Binding<'tcx> {
     span: Span,
     source: Place<'tcx>,
-    name: Symbol,
     var_id: HirId,
-    var_ty: Ty<'tcx>,
-    mutability: Mutability,
     binding_mode: BindingMode,
 }
 

compiler/rustc_mir_build/src/build/matches/simplify.rs

@@ -176,17 +176,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 Ok(())
             }
 
-            PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => {
+            PatKind::Binding {
+                name: _,
+                mutability: _,
+                mode,
+                var,
+                ty: _,
+                ref subpattern,
+                is_primary: _,
+            } => {
                 if let Ok(place_resolved) =
                     match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results)
                 {
                     candidate.bindings.push(Binding {
-                        name,
-                        mutability,
                         span: match_pair.pattern.span,
                         source: place_resolved.into_place(self.tcx, self.typeck_results),
                         var_id: var,
-                        var_ty: ty,
                         binding_mode: mode,
                     });
                 }

compiler/rustc_mir_build/src/build/scope.rs

@@ -118,9 +118,6 @@ struct Scope {
     /// the region span of this scope within source code.
     region_scope: region::Scope,
 
-    /// the span of that region_scope
-    region_scope_span: Span,
-
     /// set of places to drop when exiting this scope. This starts
     /// out empty but grows as variables are declared during the
     /// building process. This is a stack, so we always drop from the
@@ -420,7 +417,6 @@ impl<'tcx> Scopes<'tcx> {
         self.scopes.push(Scope {
             source_scope: vis_scope,
             region_scope: region_scope.0,
-            region_scope_span: region_scope.1.span,
             drops: vec![],
             moved_locals: vec![],
             cached_unwind_block: None,

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -263,7 +263,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         }
 
         if let Err(e) = result {
-            bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e)
+            bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message)
         };
 
         // Depending on current `debug_options()`, `alert_on_unused_expressions()` could panic, so

compiler/rustc_passes/src/dead.rs

@@ -239,7 +239,69 @@ 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 {
+        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 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) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     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::ImplItem(hir::ImplItem { def_id, .. }) => Some(def_id.to_def_id()),
+            _ => None,
+        } {
+            if self.should_ignore_item(item_def_id) {
+                return;
+            }
+        }
+
         let had_repr_c = self.repr_has_repr_c;
         let had_inherited_pub_visibility = self.inherited_pub_visibility;
         let had_pub_visibility = self.pub_visibility;

compiler/rustc_span/src/symbol.rs

@@ -1134,6 +1134,7 @@ symbols! {
         rustc_synthetic,
         rustc_test_marker,
         rustc_then_this_would_need,
+        rustc_trivial_field_reads,
         rustc_unsafe_specialization_marker,
         rustc_variance,
         rustdoc,

library/core/src/clone.rs

@@ -105,6 +105,7 @@
 #[stable(feature = "rust1", since = "1.0.0")]
 #[lang = "clone"]
 #[rustc_diagnostic_item = "Clone"]
+#[cfg_attr(not(bootstrap), rustc_trivial_field_reads)]
 pub trait Clone: Sized {
     /// Returns a copy of the value.
     ///

library/core/src/fmt/mod.rs

@@ -582,6 +582,7 @@ impl Display for Arguments<'_> {
 )]
 #[doc(alias = "{:?}")]
 #[rustc_diagnostic_item = "debug_trait"]
+#[cfg_attr(not(bootstrap), rustc_trivial_field_reads)]
 pub trait Debug {
     /// Formats the value using the given formatter.
     ///

library/core/tests/fmt/builders.rs

@@ -653,6 +653,7 @@ mod debug_list {
 fn test_formatting_parameters_are_forwarded() {
     use std::collections::{BTreeMap, BTreeSet};
     #[derive(Debug)]
+    #[allow(dead_code)]
     struct Foo {
         bar: u32,
         baz: u32,

library/std/src/io/buffered/tests.rs

@@ -468,9 +468,6 @@ struct ProgrammableSink {
     // Writes append to this slice
     pub buffer: Vec<u8>,
 
-    // Flush sets this flag
-    pub flushed: bool,
-
     // If true, writes will always be an error
     pub always_write_error: bool,
 
@@ -520,7 +517,6 @@ impl Write for ProgrammableSink {
         if self.always_flush_error {
             Err(io::Error::new(io::ErrorKind::Other, "test - always_flush_error"))
         } else {
-            self.flushed = true;
             Ok(())
         }
     }

library/test/src/term/terminfo/mod.rs

@@ -16,6 +16,7 @@ use parser::compiled::{msys_terminfo, parse};
 use searcher::get_dbpath_for_term;
 
 /// A parsed terminfo database entry.
+#[allow(unused)]
 #[derive(Debug)]
 pub(crate) struct TermInfo {
     /// Names for the terminal

src/librustdoc/clean/inline.rs

@@ -557,7 +557,6 @@ fn build_macro(
     name: Symbol,
     import_def_id: Option<DefId>,
 ) -> clean::ItemKind {
-    let imported_from = cx.tcx.crate_name(def_id.krate);
     match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.sess()) {
         LoadedMacro::MacroDef(item_def, _) => {
             if let ast::ItemKind::MacroDef(ref def) = item_def.kind {
@@ -569,7 +568,6 @@ fn build_macro(
                         def_id,
                         cx.tcx.visibility(import_def_id.unwrap_or(def_id)),
                     ),
-                    imported_from: Some(imported_from),
                 })
             } else {
                 unreachable!()

src/librustdoc/clean/mod.rs

@@ -1859,7 +1859,6 @@ impl Clean<Vec<Item>> for (&hir::Item<'_>, Option<Symbol>) {
                 }
                 ItemKind::Macro(ref macro_def) => MacroItem(Macro {
                     source: display_macro_source(cx, name, &macro_def, def_id, &item.vis),
-                    imported_from: None,
                 }),
                 ItemKind::Trait(is_auto, unsafety, ref generics, ref bounds, ref item_ids) => {
                     let items = item_ids

src/librustdoc/clean/types.rs

@@ -2202,7 +2202,6 @@ crate struct ImportSource {
 #[derive(Clone, Debug)]
 crate struct Macro {
     crate source: String,
-    crate imported_from: Option<Symbol>,
 }
 
 #[derive(Clone, Debug)]

src/librustdoc/html/render/mod.rs

@@ -224,7 +224,6 @@ struct AllTypes {
     opaque_tys: FxHashSet<ItemEntry>,
     statics: FxHashSet<ItemEntry>,
     constants: FxHashSet<ItemEntry>,
-    keywords: FxHashSet<ItemEntry>,
     attributes: FxHashSet<ItemEntry>,
     derives: FxHashSet<ItemEntry>,
     trait_aliases: FxHashSet<ItemEntry>,
@@ -245,7 +244,6 @@ impl AllTypes {
             opaque_tys: new_set(100),
             statics: new_set(100),
             constants: new_set(100),
-            keywords: new_set(100),
             attributes: new_set(100),
             derives: new_set(100),
             trait_aliases: new_set(100),

src/test/ui/array-slice-vec/slice_binary_search.rs

@@ -2,6 +2,7 @@
 
 // Test binary_search_by_key lifetime. Issue #34683
 
+#[allow(dead_code)]
 #[derive(Debug)]
 struct Assignment {
     topic: String,

src/test/ui/borrowck/borrowck-unused-mut-locals.rs

@@ -1,4 +1,5 @@
 // run-pass
+#![allow(dead_code)]
 #![deny(unused_mut)]
 
 #[derive(Debug)]

src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.rs

@@ -3,6 +3,7 @@
 // check-pass
 #![allow(unreachable_code)]
 #![warn(unused)]
+#![allow(dead_code)]
 
 #[derive(Debug)]
 struct Point {