Skip to content

Commit

Permalink
Add detection of unused enum variants
Browse files Browse the repository at this point in the history
  • Loading branch information
Jakub Wieczorek committed Sep 24, 2014
1 parent 9e3bf02 commit 2ec795b
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 39 deletions.
98 changes: 62 additions & 36 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,20 @@
// from live codes are live, and everything else is dead.

use middle::def;
use lint;
use middle::pat_util;
use middle::privacy;
use middle::ty;
use middle::typeck;
use lint;
use util::nodemap::NodeSet;

use std::collections::HashSet;
use syntax::ast;
use syntax::ast_map;
use syntax::ast_util::{local_def, is_local, PostExpansionMethod};
use syntax::attr::AttrMetaMethods;
use syntax::attr;
use syntax::attr::{mod, AttrMetaMethods};
use syntax::codemap;
use syntax::visit::Visitor;
use syntax::visit;
use syntax::visit::{mod, Visitor};

// Any local node that may call something in its body block should be
// explored. For example, if it's a live NodeItem that is a
Expand All @@ -51,7 +50,8 @@ struct MarkSymbolVisitor<'a, 'tcx: 'a> {
worklist: Vec<ast::NodeId>,
tcx: &'a ty::ctxt<'tcx>,
live_symbols: Box<HashSet<ast::NodeId>>,
struct_has_extern_repr: bool
struct_has_extern_repr: bool,
ignore_paths: bool
}

impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
Expand All @@ -61,7 +61,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
worklist: worklist,
tcx: tcx,
live_symbols: box HashSet::new(),
struct_has_extern_repr: false
struct_has_extern_repr: false,
ignore_paths: false
}
}

Expand All @@ -73,19 +74,18 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
}

fn lookup_and_handle_definition(&mut self, id: &ast::NodeId) {
let def = match self.tcx.def_map.borrow().find(id) {
Some(&def) => def,
None => return
};
let def_id = match def {
def::DefVariant(enum_id, _, _) => Some(enum_id),
def::DefPrimTy(_) => None,
_ => Some(def.def_id())
};
match def_id {
Some(def_id) => self.check_def_id(def_id),
None => (),
}
self.tcx.def_map.borrow().find(id).map(|def| {
match def {
&def::DefPrimTy(_) => (),
&def::DefVariant(enum_id, variant_id, _) => {
self.check_def_id(enum_id);
self.check_def_id(variant_id);
}
_ => {
self.check_def_id(def.def_id());
}
}
});
}

fn lookup_and_handle_method(&mut self, id: ast::NodeId,
Expand Down Expand Up @@ -275,22 +275,27 @@ impl<'a, 'tcx, 'v> Visitor<'v> for MarkSymbolVisitor<'a, 'tcx> {
}

fn visit_pat(&mut self, pat: &ast::Pat) {
let def_map = &self.tcx.def_map;
match pat.node {
ast::PatStruct(_, ref fields, _) => {
self.handle_field_pattern_match(pat, fields.as_slice());
}
ast::PatIdent(_, _, _) => {
_ if pat_util::pat_is_const(def_map, pat) => {
// it might be the only use of a static:
self.lookup_and_handle_definition(&pat.id)
}
_ => ()
}

self.ignore_paths = true;
visit::walk_pat(self, pat);
self.ignore_paths = false;
}

fn visit_path(&mut self, path: &ast::Path, id: ast::NodeId) {
self.lookup_and_handle_definition(&id);
if !self.ignore_paths {
self.lookup_and_handle_definition(&id);
}
visit::walk_path(self, path);
}

Expand Down Expand Up @@ -330,15 +335,19 @@ fn has_allow_dead_code_or_lang_attr(attrs: &[ast::Attribute]) -> bool {
// 2) We are not sure to be live or not
// * Implementation of a trait method
struct LifeSeeder {
worklist: Vec<ast::NodeId> ,
worklist: Vec<ast::NodeId>
}

impl<'v> Visitor<'v> for LifeSeeder {
fn visit_item(&mut self, item: &ast::Item) {
if has_allow_dead_code_or_lang_attr(item.attrs.as_slice()) {
let allow_dead_code = has_allow_dead_code_or_lang_attr(item.attrs.as_slice());
if allow_dead_code {
self.worklist.push(item.id);
}
match item.node {
ast::ItemEnum(ref enum_def, _) if allow_dead_code => {
self.worklist.extend(enum_def.variants.iter().map(|variant| variant.node.id));
}
ast::ItemImpl(_, Some(ref _trait_ref), _, ref impl_items) => {
for impl_item in impl_items.iter() {
match *impl_item {
Expand Down Expand Up @@ -415,16 +424,6 @@ fn find_live(tcx: &ty::ctxt,
symbol_visitor.live_symbols
}

fn should_warn(item: &ast::Item) -> bool {
match item.node {
ast::ItemStatic(..)
| ast::ItemFn(..)
| ast::ItemEnum(..)
| ast::ItemStruct(..) => true,
_ => false
}
}

fn get_struct_ctor_id(item: &ast::Item) -> Option<ast::NodeId> {
match item.node {
ast::ItemStruct(ref struct_def, _) => struct_def.ctor_id,
Expand All @@ -438,6 +437,18 @@ struct DeadVisitor<'a, 'tcx: 'a> {
}

impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {
fn should_warn_about_item(&mut self, item: &ast::Item) -> bool {
let should_warn = match item.node {
ast::ItemStatic(..)
| ast::ItemFn(..)
| ast::ItemEnum(..)
| ast::ItemStruct(..) => true,
_ => false
};
let ctor_id = get_struct_ctor_id(item);
should_warn && !self.symbol_is_live(item.id, ctor_id)
}

fn should_warn_about_field(&mut self, node: &ast::StructField_) -> bool {
let is_named = node.ident().is_some();
let field_type = ty::node_id_to_type(self.tcx, node.id);
Expand All @@ -451,6 +462,11 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {
&& !has_allow_dead_code_or_lang_attr(node.attrs.as_slice())
}

fn should_warn_about_variant(&mut self, variant: &ast::Variant_) -> bool {
!self.symbol_is_live(variant.id, None)
&& !has_allow_dead_code_or_lang_attr(variant.attrs.as_slice())
}

// id := node id of an item's definition.
// ctor_id := `Some` if the item is a struct_ctor (tuple struct),
// `None` otherwise.
Expand Down Expand Up @@ -506,9 +522,19 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {

impl<'a, 'tcx, 'v> Visitor<'v> for DeadVisitor<'a, 'tcx> {
fn visit_item(&mut self, item: &ast::Item) {
let ctor_id = get_struct_ctor_id(item);
if !self.symbol_is_live(item.id, ctor_id) && should_warn(item) {
if self.should_warn_about_item(item) {
self.warn_dead_code(item.id, item.span, item.ident);
} else {
match item.node {
ast::ItemEnum(ref enum_def, _) => {
for variant in enum_def.variants.iter() {
if self.should_warn_about_variant(&variant.node) {
self.warn_dead_code(variant.node.id, variant.span, variant.node.name);
}
}
},
_ => ()
}
}
visit::walk_item(self, item);
}
Expand Down
6 changes: 5 additions & 1 deletion src/test/compile-fail/lint-dead-code-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ pub struct PubStruct2 {
pub enum pub_enum { foo1, bar1 }
pub enum pub_enum2 { a(*const StructUsedInEnum) }
pub enum pub_enum3 { Foo = STATIC_USED_IN_ENUM_DISCRIMINANT }

enum priv_enum { foo2, bar2 } //~ ERROR: code is never used
enum used_enum { foo3, bar3 }
enum used_enum {
foo3,
bar3 //~ ERROR code is never used
}

fn f<T>() {}

Expand Down
4 changes: 2 additions & 2 deletions src/test/compile-fail/lint-dead-code-4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ fn field_read(f: Foo) -> uint {
}

enum XYZ {
X,
Y {
X, //~ ERROR variant is never used
Y { //~ ERROR variant is never used
a: String,
b: int //~ ERROR: code is never used
},
Expand Down
41 changes: 41 additions & 0 deletions src/test/compile-fail/lint-dead-code-5.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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.

#![feature(struct_variant)]
#![allow(unused_variable)]
#![deny(dead_code)]

enum Enum1 {
Variant1(int),
Variant2 //~ ERROR: code is never used
}

enum Enum2 {
Variant3(bool),
#[allow(dead_code)]
Variant4(int),
Variant5 { _x: int }, //~ ERROR: variant is never used: `Variant5`
Variant6(int), //~ ERROR: variant is never used: `Variant6`
_Variant7,
}

enum Enum3 { //~ ERROR: enum is never used
Variant8,
Variant9
}

fn main() {
let v = Variant1(1);
match v {
Variant1(_) => (),
Variant2 => ()
}
let x = Variant3(true);
}

0 comments on commit 2ec795b

Please sign in to comment.