Skip to content

Commit

Permalink
Rollup merge of rust-lang#56953 - oli-obk:dead_const, r=petrochenkov
Browse files Browse the repository at this point in the history
Mark tuple structs as live if their constructors are used

fixes rust-lang#56281
  • Loading branch information
Centril authored Dec 23, 2018
2 parents 8cd1817 + 405d8b0 commit 29f2fee
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 43 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
80 changes: 38 additions & 42 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(..) => {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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);
}
_ => ()
}
}
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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>,
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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");
}
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/dead-code-tuple-struct-field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// compile-pass

#![deny(dead_code)]

const LEN: usize = 4;

#[derive(Debug)]
struct Wrapper([u8; LEN]);

fn main() {
println!("{:?}", Wrapper([0, 1, 2, 3]));
}

0 comments on commit 29f2fee

Please sign in to comment.