Mark tuple structs as live if their constructors are used

oli-obk · oli-obk · commit f7314456d03d · 2018-12-18T15:54:06.000+01:00
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
@@ -2119,7 +2119,7 @@ impl StructField {
 /// Id of the whole enum lives in `Item`.
 ///
 /// For structs: `NodeId` represents an Id of the structure's constructor, so it is not actually
-/// used for `Struct`-structs (but still presents). Structures don't have an analogue of "Id of
+/// used for `Struct`-structs (but still present). Structures don't have an analogue of "Id of
 /// the variant itself" from enum variants.
 /// Id of the whole struct lives in `Item`.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs
@@ -25,6 +25,8 @@ use middle::privacy;
 use ty::{self, TyCtxt};
 use util::nodemap::FxHashSet;
 
+use rustc_data_structures::fx::FxHashMap;
+
 use syntax::{ast, source_map};
 use syntax::attr;
 use syntax_pos;
@@ -55,12 +57,15 @@ struct MarkSymbolVisitor<'a, 'tcx: 'a> {
     in_pat: bool,
     inherited_pub_visibility: bool,
     ignore_variant_stack: Vec<DefId>,
+    // maps from tuple struct constructors to tuple struct items
+    struct_constructors: FxHashMap<ast::NodeId, ast::NodeId>,
 }
 
 impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
     fn check_def_id(&mut self, def_id: DefId) {
         if let Some(node_id) = self.tcx.hir().as_local_node_id(def_id) {
-            if should_explore(self.tcx, node_id) {
+            if should_explore(self.tcx, node_id) ||
+               self.struct_constructors.contains_key(&node_id) {
                 self.worklist.push(node_id);
             }
             self.live_symbols.insert(node_id);
@@ -137,19 +142,23 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
                 continue
             }
 
-            if let Some(ref node) = self.tcx.hir().find(id) {
+            // in the case of tuple struct constructors we want to check the item, not the generated
+            // tuple struct constructor function
+            let id = self.struct_constructors.get(&id).cloned().unwrap_or(id);
+
+            if let Some(node) = self.tcx.hir().find(id) {
                 self.live_symbols.insert(id);
                 self.visit_node(node);
             }
         }
     }
 
-    fn visit_node(&mut self, node: &Node<'tcx>) {
+    fn visit_node(&mut self, node: Node<'tcx>) {
         let had_repr_c = self.repr_has_repr_c;
         self.repr_has_repr_c = false;
         let had_inherited_pub_visibility = self.inherited_pub_visibility;
         self.inherited_pub_visibility = false;
-        match *node {
+        match node {
             Node::Item(item) => {
                 match item.node {
                     hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
@@ -337,6 +346,8 @@ struct LifeSeeder<'k, 'tcx: 'k> {
     worklist: Vec<ast::NodeId>,
     krate: &'k hir::Crate,
     tcx: TyCtxt<'k, 'tcx, 'tcx>,
+    // see `MarkSymbolVisitor::struct_constructors`
+    struct_constructors: FxHashMap<ast::NodeId, ast::NodeId>,
 }
 
 impl<'v, 'k, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'k, 'tcx> {
@@ -379,6 +390,9 @@ impl<'v, 'k, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'k, 'tcx> {
                     }
                 }
             }
+            hir::ItemKind::Struct(ref variant_data, _) => {
+                self.struct_constructors.insert(variant_data.id(), item.id);
+            }
             _ => ()
         }
     }
@@ -392,11 +406,11 @@ impl<'v, 'k, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'k, 'tcx> {
     }
 }
 
-fn create_and_seed_worklist<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                                      access_levels: &privacy::AccessLevels,
-                                      krate: &hir::Crate)
-                                      -> Vec<ast::NodeId>
-{
+fn create_and_seed_worklist<'a, 'tcx>(
+    tcx: TyCtxt<'a, 'tcx, 'tcx>,
+    access_levels: &privacy::AccessLevels,
+    krate: &hir::Crate,
+) -> (Vec<ast::NodeId>, FxHashMap<ast::NodeId, ast::NodeId>) {
     let worklist = access_levels.map.iter().filter_map(|(&id, level)| {
         if level >= &privacy::AccessLevel::Reachable {
             Some(id)
@@ -413,17 +427,18 @@ fn create_and_seed_worklist<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         worklist,
         krate,
         tcx,
+        struct_constructors: Default::default(),
     };
     krate.visit_all_item_likes(&mut life_seeder);
 
-    return life_seeder.worklist;
+    (life_seeder.worklist, life_seeder.struct_constructors)
 }
 
 fn find_live<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                        access_levels: &privacy::AccessLevels,
                        krate: &hir::Crate)
                        -> FxHashSet<ast::NodeId> {
-    let worklist = create_and_seed_worklist(tcx, access_levels, krate);
+    let (worklist, struct_constructors) = create_and_seed_worklist(tcx, access_levels, krate);
     let mut symbol_visitor = MarkSymbolVisitor {
         worklist,
         tcx,
@@ -433,20 +448,12 @@ fn find_live<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         in_pat: false,
         inherited_pub_visibility: false,
         ignore_variant_stack: vec![],
+        struct_constructors,
     };
     symbol_visitor.mark_live_symbols();
     symbol_visitor.live_symbols
 }
 
-fn get_struct_ctor_id(item: &hir::Item) -> Option<ast::NodeId> {
-    match item.node {
-        hir::ItemKind::Struct(ref struct_def, _) if !struct_def.is_struct() => {
-            Some(struct_def.id())
-        }
-        _ => None
-    }
-}
-
 struct DeadVisitor<'a, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     live_symbols: FxHashSet<ast::NodeId>,
@@ -464,46 +471,35 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {
             | hir::ItemKind::Union(..) => true,
             _ => false
         };
-        let ctor_id = get_struct_ctor_id(item);
-        should_warn && !self.symbol_is_live(item.id, ctor_id)
+        should_warn && !self.symbol_is_live(item.id)
     }
 
     fn should_warn_about_field(&mut self, field: &hir::StructField) -> bool {
         let field_type = self.tcx.type_of(self.tcx.hir().local_def_id(field.id));
         !field.is_positional()
-            && !self.symbol_is_live(field.id, None)
+            && !self.symbol_is_live(field.id)
             && !field_type.is_phantom_data()
             && !has_allow_dead_code_or_lang_attr(self.tcx, field.id, &field.attrs)
     }
 
     fn should_warn_about_variant(&mut self, variant: &hir::VariantKind) -> bool {
-        !self.symbol_is_live(variant.data.id(), None)
+        !self.symbol_is_live(variant.data.id())
             && !has_allow_dead_code_or_lang_attr(self.tcx,
                                                  variant.data.id(),
                                                  &variant.attrs)
     }
 
     fn should_warn_about_foreign_item(&mut self, fi: &hir::ForeignItem) -> bool {
-        !self.symbol_is_live(fi.id, None)
+        !self.symbol_is_live(fi.id)
             && !has_allow_dead_code_or_lang_attr(self.tcx, fi.id, &fi.attrs)
     }
 
     // id := node id of an item's definition.
-    // ctor_id := `Some` if the item is a struct_ctor (tuple struct),
-    //            `None` otherwise.
-    // If the item is a struct_ctor, then either its `id` or
-    // `ctor_id` (unwrapped) is in the live_symbols set. More specifically,
-    // DefMap maps the ExprKind::Path of a struct_ctor to the node referred by
-    // `ctor_id`. On the other hand, in a statement like
-    // `type <ident> <generics> = <ty>;` where <ty> refers to a struct_ctor,
-    // DefMap maps <ty> to `id` instead.
-    fn symbol_is_live(&mut self,
-                      id: ast::NodeId,
-                      ctor_id: Option<ast::NodeId>)
-                      -> bool {
-        if self.live_symbols.contains(&id)
-           || ctor_id.map_or(false, |ctor| self.live_symbols.contains(&ctor))
-        {
+    fn symbol_is_live(
+        &mut self,
+        id: ast::NodeId,
+    ) -> bool {
+        if self.live_symbols.contains(&id) {
             return true;
         }
         // If it's a type whose items are live, then it's live, too.
@@ -611,7 +607,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DeadVisitor<'a, 'tcx> {
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) {
         match impl_item.node {
             hir::ImplItemKind::Const(_, body_id) => {
-                if !self.symbol_is_live(impl_item.id, None) {
+                if !self.symbol_is_live(impl_item.id) {
                     self.warn_dead_code(impl_item.id,
                                         impl_item.span,
                                         impl_item.ident.name,
@@ -621,7 +617,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DeadVisitor<'a, 'tcx> {
                 self.visit_nested_body(body_id)
             }
             hir::ImplItemKind::Method(_, body_id) => {
-                if !self.symbol_is_live(impl_item.id, None) {
+                if !self.symbol_is_live(impl_item.id) {
                     let span = self.tcx.sess.source_map().def_span(impl_item.span);
                     self.warn_dead_code(impl_item.id, span, impl_item.ident.name, "method", "used");
                 }
diff --git a/src/test/ui/dead-code-tuple-struct-field.rs b/src/test/ui/dead-code-tuple-struct-field.rs
@@ -0,0 +1,22 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-pass
+
+#![deny(dead_code)]
+
+const LEN: usize = 4;
+
+#[derive(Debug)]
+struct Wrapper([u8; LEN]);
+
+fn main() {
+    println!("{:?}", Wrapper([0, 1, 2, 3]));
+}
