Skip to content

Commit 0d37323

Browse files
committed
Rollup merge of #21468 - sanxiyn:dead-variant, r=
This implements a wish suggested in #17410, detecting enum variants that are never constructed, even in the presence of `#[derive(Clone)]`. The implementation is general and not specific to `#[derive(Clone)]`. r? @jakub-
2 parents 0e4b8d6 + b042ffc commit 0d37323

File tree

4 files changed

+94
-1
lines changed

4 files changed

+94
-1
lines changed

src/librustc/middle/dead.rs

+27-1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ struct MarkSymbolVisitor<'a, 'tcx: 'a> {
4747
struct_has_extern_repr: bool,
4848
ignore_non_const_paths: bool,
4949
inherited_pub_visibility: bool,
50+
ignore_variant_stack: Vec<ast::NodeId>,
5051
}
5152

5253
impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
@@ -59,6 +60,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
5960
struct_has_extern_repr: false,
6061
ignore_non_const_paths: false,
6162
inherited_pub_visibility: false,
63+
ignore_variant_stack: vec![],
6264
}
6365
}
6466

@@ -79,7 +81,9 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
7981
def::DefPrimTy(_) => (),
8082
def::DefVariant(enum_id, variant_id, _) => {
8183
self.check_def_id(enum_id);
82-
self.check_def_id(variant_id);
84+
if !self.ignore_variant_stack.contains(&variant_id.node) {
85+
self.check_def_id(variant_id);
86+
}
8387
}
8488
_ => {
8589
self.check_def_id(def.def_id());
@@ -278,6 +282,23 @@ impl<'a, 'tcx, 'v> Visitor<'v> for MarkSymbolVisitor<'a, 'tcx> {
278282
visit::walk_expr(self, expr);
279283
}
280284

285+
fn visit_arm(&mut self, arm: &ast::Arm) {
286+
if arm.pats.len() == 1 {
287+
let pat = &*arm.pats[0];
288+
let variants = pat_util::necessary_variants(&self.tcx.def_map, pat);
289+
290+
// Inside the body, ignore constructions of variants
291+
// necessary for the pattern to match. Those construction sites
292+
// can't be reached unless the variant is constructed elsewhere.
293+
let len = self.ignore_variant_stack.len();
294+
self.ignore_variant_stack.push_all(&*variants);
295+
visit::walk_arm(self, arm);
296+
self.ignore_variant_stack.truncate(len);
297+
} else {
298+
visit::walk_arm(self, arm);
299+
}
300+
}
301+
281302
fn visit_pat(&mut self, pat: &ast::Pat) {
282303
let def_map = &self.tcx.def_map;
283304
match pat.node {
@@ -397,6 +418,11 @@ fn create_and_seed_worklist(tcx: &ty::ctxt,
397418
worklist.push(*id);
398419
}
399420
for id in reachable_symbols {
421+
// Reachable variants can be dead, because we warn about
422+
// variants never constructed, not variants never used.
423+
if let Some(ast_map::NodeVariant(..)) = tcx.map.find(*id) {
424+
continue;
425+
}
400426
worklist.push(*id);
401427
}
402428

src/librustc/middle/pat_util.rs

+24
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,27 @@ pub fn def_to_path(tcx: &ty::ctxt, id: ast::DefId) -> ast::Path {
155155
span: DUMMY_SP,
156156
})
157157
}
158+
159+
/// Return variants that are necessary to exist for the pattern to match.
160+
pub fn necessary_variants(dm: &DefMap, pat: &ast::Pat) -> Vec<ast::NodeId> {
161+
let mut variants = vec![];
162+
walk_pat(pat, |p| {
163+
match p.node {
164+
ast::PatEnum(_, _) |
165+
ast::PatIdent(_, _, None) |
166+
ast::PatStruct(..) => {
167+
match dm.borrow().get(&p.id) {
168+
Some(&DefVariant(_, id, _)) => {
169+
variants.push(id.node);
170+
}
171+
_ => ()
172+
}
173+
}
174+
_ => ()
175+
}
176+
true
177+
});
178+
variants.sort();
179+
variants.dedup();
180+
variants
181+
}

src/libtest/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ impl fmt::Display for TestName {
121121
#[derive(Clone, Copy)]
122122
enum NamePadding {
123123
PadNone,
124+
#[allow(dead_code)]
124125
PadOnLeft,
125126
PadOnRight,
126127
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![deny(dead_code)]
12+
13+
#[derive(Copy)]
14+
enum Enum {
15+
Variant1, //~ ERROR: variant is never used
16+
Variant2,
17+
Variant3,
18+
}
19+
20+
fn copy(e: Enum) -> Enum {
21+
use Enum::*;
22+
match e {
23+
Variant1 => Variant1,
24+
Variant2 => Variant2,
25+
Variant3 => Variant3,
26+
}
27+
}
28+
29+
fn max(e: Enum) -> Enum {
30+
use Enum::*;
31+
match e {
32+
Variant1 => Variant3,
33+
Variant2 => Variant3,
34+
Variant3 => Variant3,
35+
}
36+
}
37+
38+
fn main() {
39+
let e = Enum::Variant2;
40+
copy(e);
41+
max(e);
42+
}

0 commit comments

Comments
 (0)