Add detection of unused enum variants

Author: Jakub Wieczorek

src/librustc/middle/dead.rs

@@ -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
@@ -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> {
@@ -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
         }
     }
 
@@ -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,
@@ -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);
     }
 
@@ -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 {
@@ -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,
@@ -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);
@@ -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.
@@ -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);
     }

src/test/compile-fail/lint-dead-code-1.rs

@@ -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>() {}
 

src/test/compile-fail/lint-dead-code-4.rs

@@ -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
     },

src/test/compile-fail/lint-dead-code-5.rs

@@ -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);
+}