From 18e41299f9368c593be688792962a6d0c299c9bc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 10 Oct 2014 08:31:13 -0700 Subject: [PATCH 1/2] rustc: Warn about dead constants A few catch-all blocks ended up not having this case for constants. Closes #17925 --- src/librustc/middle/dead.rs | 1 + src/libsyntax/ast.rs | 4 +++- src/test/compile-fail/lint-dead-code-1.rs | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index ff372038100..61b013d795e 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -441,6 +441,7 @@ 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::ItemConst(..) | ast::ItemFn(..) | ast::ItemEnum(..) | ast::ItemStruct(..) => true, diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 274bb2e39e0..be79b18bfe9 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1334,6 +1334,7 @@ impl Item_ { pub fn descriptive_variant(&self) -> &str { match *self { ItemStatic(..) => "static item", + ItemConst(..) => "constant item", ItemFn(..) => "function", ItemMod(..) => "module", ItemForeignMod(..) => "foreign module", @@ -1341,7 +1342,8 @@ impl Item_ { ItemEnum(..) => "enum", ItemStruct(..) => "struct", ItemTrait(..) => "trait", - _ => "item" + ItemMac(..) | + ItemImpl(..) => "item" } } } diff --git a/src/test/compile-fail/lint-dead-code-1.rs b/src/test/compile-fail/lint-dead-code-1.rs index a4320b8dc77..998e7c01792 100644 --- a/src/test/compile-fail/lint-dead-code-1.rs +++ b/src/test/compile-fail/lint-dead-code-1.rs @@ -35,6 +35,13 @@ pub static used_static2: int = used_static; const USED_STATIC: int = 0; const STATIC_USED_IN_ENUM_DISCRIMINANT: int = 10; +pub const pub_const: int = 0; +const priv_const: int = 0; //~ ERROR: constant item is never used +const used_const: int = 0; +pub const used_const2: int = used_const; +const USED_CONST: int = 0; +const CONST_USED_IN_ENUM_DISCRIMINANT: int = 10; + pub type typ = *const UsedStruct4; pub struct PubStruct; struct PrivStruct; //~ ERROR: struct is never used From c56c9fcf08603ab88ec36a7e0757d29129339097 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 10 Oct 2014 08:49:12 -0700 Subject: [PATCH 2/2] rustc: Remove the dummy hack from check_match Turns out you can create &'static T quite easily in a constant, I just forgot about this! --- src/librustc/middle/check_match.rs | 34 ++++++++----------- src/librustc/middle/trans/_match.rs | 22 ++---------- .../compile-fail/issue-17718-const-naming.rs | 1 + src/test/compile-fail/lint-dead-code-1.rs | 10 ++++-- 4 files changed, 25 insertions(+), 42 deletions(-) diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index 29764070768..1c06bc1ceba 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -32,7 +32,7 @@ use syntax::ptr::P; use syntax::visit::{mod, Visitor, FnKind}; use util::ppaux::ty_to_string; -pub const DUMMY_WILD_PAT: Pat = Pat { +pub const DUMMY_WILD_PAT: &'static Pat = &Pat { id: DUMMY_NODE_ID, node: PatWild(PatWildSingle), span: DUMMY_SP @@ -297,12 +297,11 @@ fn raw_pat<'a>(p: &'a Pat) -> &'a Pat { } fn check_exhaustive(cx: &MatchCheckCtxt, sp: Span, matrix: &Matrix) { - match is_useful(cx, matrix, &[&DUMMY_WILD_PAT], ConstructWitness) { + match is_useful(cx, matrix, &[DUMMY_WILD_PAT], ConstructWitness) { UsefulWithWitness(pats) => { - let dummy = DUMMY_WILD_PAT.clone(); let witness = match pats.as_slice() { [ref witness] => &**witness, - [] => &dummy, + [] => DUMMY_WILD_PAT, _ => unreachable!() }; span_err!(cx.tcx.sess, sp, E0004, @@ -556,9 +555,8 @@ fn is_useful(cx: &MatchCheckCtxt, let arity = constructor_arity(cx, &c, left_ty); let mut result = { let pat_slice = pats.as_slice(); - let dummy = DUMMY_WILD_PAT.clone(); let subpats = Vec::from_fn(arity, |i| { - pat_slice.get(i).map_or(&dummy, |p| &**p) + pat_slice.get(i).map_or(DUMMY_WILD_PAT, |p| &**p) }); vec![construct_witness(cx, &c, subpats, left_ty)] }; @@ -580,9 +578,8 @@ fn is_useful(cx: &MatchCheckCtxt, }).collect(); match is_useful(cx, &matrix, v.tail(), witness) { UsefulWithWitness(pats) => { - let dummy = DUMMY_WILD_PAT.clone(); let arity = constructor_arity(cx, &constructor, left_ty); - let wild_pats = Vec::from_elem(arity, &dummy); + let wild_pats = Vec::from_elem(arity, DUMMY_WILD_PAT); let enum_pat = construct_witness(cx, &constructor, wild_pats, left_ty); let mut new_pats = vec![enum_pat]; new_pats.extend(pats.into_iter()); @@ -603,11 +600,10 @@ fn is_useful_specialized(cx: &MatchCheckCtxt, &Matrix(ref m): &Matrix, v: &[&Pat], ctor: Constructor, lty: ty::t, witness: WitnessPreference) -> Usefulness { let arity = constructor_arity(cx, &ctor, lty); - let dummy = DUMMY_WILD_PAT.clone(); let matrix = Matrix(m.iter().filter_map(|r| { - specialize(cx, r.as_slice(), &dummy, &ctor, 0u, arity) + specialize(cx, r.as_slice(), &ctor, 0u, arity) }).collect()); - match specialize(cx, v, &dummy, &ctor, 0u, arity) { + match specialize(cx, v, &ctor, 0u, arity) { Some(v) => is_useful(cx, &matrix, v.as_slice(), witness), None => NotUseful } @@ -729,7 +725,7 @@ fn range_covered_by_constructor(ctor: &Constructor, /// different patterns. /// Structure patterns with a partial wild pattern (Foo { a: 42, .. }) have their missing /// fields filled with wild patterns. -pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, +pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], constructor: &Constructor, col: uint, arity: uint) -> Option> { let &Pat { id: pat_id, node: ref node, span: pat_span @@ -737,7 +733,7 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, let head: Option> = match node { &PatWild(_) => - Some(Vec::from_elem(arity, dummy)), + Some(Vec::from_elem(arity, DUMMY_WILD_PAT)), &PatIdent(_, _, _) => { let opt_def = cx.tcx.def_map.borrow().find_copy(&pat_id); @@ -750,7 +746,7 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, } else { None }, - _ => Some(Vec::from_elem(arity, dummy)) + _ => Some(Vec::from_elem(arity, DUMMY_WILD_PAT)) } } @@ -764,7 +760,7 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, DefVariant(..) | DefStruct(..) => { Some(match args { &Some(ref args) => args.iter().map(|p| &**p).collect(), - &None => Vec::from_elem(arity, dummy) + &None => Vec::from_elem(arity, DUMMY_WILD_PAT) }) } _ => None @@ -800,7 +796,7 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, let args = struct_fields.iter().map(|sf| { match pattern_fields.iter().find(|f| f.ident.name == sf.name) { Some(ref f) => &*f.pat, - _ => dummy + _ => DUMMY_WILD_PAT } }).collect(); args @@ -843,13 +839,13 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat], dummy: &'a Pat, // Fixed-length vectors. Single => { let mut pats: Vec<&Pat> = before.iter().map(|p| &**p).collect(); - pats.grow_fn(arity - before.len() - after.len(), |_| dummy); + pats.grow_fn(arity - before.len() - after.len(), |_| DUMMY_WILD_PAT); pats.extend(after.iter().map(|p| &**p)); Some(pats) }, Slice(length) if before.len() + after.len() <= length && slice.is_some() => { let mut pats: Vec<&Pat> = before.iter().map(|p| &**p).collect(); - pats.grow_fn(arity - before.len() - after.len(), |_| dummy); + pats.grow_fn(arity - before.len() - after.len(), |_| DUMMY_WILD_PAT); pats.extend(after.iter().map(|p| &**p)); Some(pats) }, @@ -919,7 +915,7 @@ fn check_fn(cx: &mut MatchCheckCtxt, fn is_refutable(cx: &MatchCheckCtxt, pat: &Pat, refutable: |&Pat| -> A) -> Option { let pats = Matrix(vec!(vec!(pat))); - match is_useful(cx, &pats, [&DUMMY_WILD_PAT], ConstructWitness) { + match is_useful(cx, &pats, [DUMMY_WILD_PAT], ConstructWitness) { UsefulWithWitness(pats) => { assert_eq!(pats.len(), 1); Some(refutable(&*pats[0])) diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs index 9eb02717f04..4f7d165d159 100644 --- a/src/librustc/middle/trans/_match.rs +++ b/src/librustc/middle/trans/_match.rs @@ -351,19 +351,6 @@ struct Match<'a, 'p: 'a, 'blk: 'a, 'tcx: 'blk> { pats: Vec<&'p ast::Pat>, data: &'a ArmData<'p, 'blk, 'tcx>, bound_ptrs: Vec<(Ident, ValueRef)>, - - // This is a pointer to an instance of check_match::DUMMY_WILD_PAT. The - // check_match code requires that we pass this in (with the same lifetime as - // the patterns passed in). Unfortunately this is required to be propagated - // into this structure in order to get the lifetimes to work. - // - // Lots of the `check_match` code will deal with &DUMMY_WILD_PAT when - // returning references, which used to have the `'static` lifetime before - // const was added to the language. The DUMMY_WILD_PAT does not implement - // Sync, however, so it must be a const, which longer has a static lifetime, - // hence we're passing it in here. This certainly isn't crucial, and if it - // can be removed, please do! - dummy: &'p ast::Pat, } impl<'a, 'p, 'blk, 'tcx> Repr for Match<'a, 'p, 'blk, 'tcx> { @@ -416,7 +403,6 @@ fn expand_nested_bindings<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>, *pats.get_mut(col) = pat; Match { pats: pats, - dummy: br.dummy, data: &*br.data, bound_ptrs: bound_ptrs } @@ -464,7 +450,6 @@ fn enter_match<'a, 'b, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>, } Match { pats: pats, - dummy: br.dummy, data: br.data, bound_ptrs: bound_ptrs } @@ -559,8 +544,7 @@ fn enter_opt<'a, 'p, 'blk, 'tcx>( let mcx = check_match::MatchCheckCtxt { tcx: bcx.tcx() }; enter_match(bcx, dm, m, col, val, |pats| - check_match::specialize(&mcx, pats.as_slice(), m[0].dummy, &ctor, col, - variant_size) + check_match::specialize(&mcx, pats.as_slice(), &ctor, col, variant_size) ) } @@ -1041,7 +1025,7 @@ fn compile_submatch_continue<'a, 'p, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, match adt_vals { Some(field_vals) => { let pats = enter_match(bcx, dm, m, col, val, |pats| - check_match::specialize(&mcx, pats, m[0].dummy, + check_match::specialize(&mcx, pats, &check_match::Single, col, field_vals.len()) ); @@ -1365,7 +1349,6 @@ fn trans_match_inner<'blk, 'tcx>(scope_cx: Block<'blk, 'tcx>, bindings_map: create_bindings_map(bcx, &**arm.pats.get(0), discr_expr, &*arm.body) }).collect(); - let dummy = check_match::DUMMY_WILD_PAT.clone(); let mut static_inliner = StaticInliner::new(scope_cx.tcx()); let arm_pats: Vec>> = arm_datas.iter().map(|arm_data| { arm_data.arm.pats.iter().map(|p| static_inliner.fold_pat((*p).clone())).collect() @@ -1374,7 +1357,6 @@ fn trans_match_inner<'blk, 'tcx>(scope_cx: Block<'blk, 'tcx>, for (arm_data, pats) in arm_datas.iter().zip(arm_pats.iter()) { matches.extend(pats.iter().map(|p| Match { pats: vec![&**p], - dummy: &dummy, data: arm_data, bound_ptrs: Vec::new(), })); diff --git a/src/test/compile-fail/issue-17718-const-naming.rs b/src/test/compile-fail/issue-17718-const-naming.rs index 046f038847b..0cfee6daf3f 100644 --- a/src/test/compile-fail/issue-17718-const-naming.rs +++ b/src/test/compile-fail/issue-17718-const-naming.rs @@ -12,5 +12,6 @@ const foo: int = 3; //~^ ERROR: should have an uppercase name such as +//~^^ ERROR: constant item is never used fn main() {} diff --git a/src/test/compile-fail/lint-dead-code-1.rs b/src/test/compile-fail/lint-dead-code-1.rs index 998e7c01792..96d40c52657 100644 --- a/src/test/compile-fail/lint-dead-code-1.rs +++ b/src/test/compile-fail/lint-dead-code-1.rs @@ -39,8 +39,8 @@ pub const pub_const: int = 0; const priv_const: int = 0; //~ ERROR: constant item is never used const used_const: int = 0; pub const used_const2: int = used_const; -const USED_CONST: int = 0; -const CONST_USED_IN_ENUM_DISCRIMINANT: int = 10; +const USED_CONST: int = 1; +const CONST_USED_IN_ENUM_DISCRIMINANT: int = 11; pub type typ = *const UsedStruct4; pub struct PubStruct; @@ -68,7 +68,10 @@ 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 } +pub enum pub_enum3 { + Foo = STATIC_USED_IN_ENUM_DISCRIMINANT, + Bar = CONST_USED_IN_ENUM_DISCRIMINANT, +} enum priv_enum { foo2, bar2 } //~ ERROR: enum is never used enum used_enum { @@ -89,6 +92,7 @@ pub fn pub_fn() { let i = 1i; match i { USED_STATIC => (), + USED_CONST => (), _ => () } f::();