auto merge of #5804 : alexcrichton/rust/issue-3266, r=graydon

This leaves the default lint modes at `warn`, but now the unused variable and dead assignment warnings are configurable on a per-item basis. As described in #3266, this just involved carrying around a couple ids to pass over to `span_lint`. I personally would prefer to keep the `_` prefix as well.

This closes #3266.
This commit is contained in:
bors 2013-04-09 20:36:56 -07:00
commit 6100bb5cba
3 changed files with 82 additions and 60 deletions

View File

@ -83,9 +83,8 @@ pub enum lint {
legacy_modes, legacy_modes,
// FIXME(#3266)--make liveness warnings lintable unused_variable,
// unused_variable, dead_assignment,
// dead_assignment
} }
pub fn level_to_str(lv: level) -> &'static str { pub fn level_to_str(lv: level) -> &'static str {
@ -257,21 +256,19 @@ pub fn get_lint_dict() -> LintDict {
default: deny default: deny
}), }),
/* FIXME(#3266)--make liveness warnings lintable (~"unused_variable",
(@~"unused_variable", LintSpec {
@LintSpec {
lint: unused_variable, lint: unused_variable,
desc: "detect variables which are not used in any way", desc: "detect variables which are not used in any way",
default: warn default: warn
}), }),
(@~"dead_assignment", (~"dead_assignment",
@LintSpec { LintSpec {
lint: dead_assignment, lint: dead_assignment,
desc: "detect assignments that will never be read", desc: "detect assignments that will never be read",
default: warn default: warn
}), }),
*/
]; ];
let mut map = HashMap::new(); let mut map = HashMap::new();
do vec::consume(v) |_, (k, v)| { do vec::consume(v) |_, (k, v)| {

View File

@ -105,6 +105,7 @@
use core::prelude::*; use core::prelude::*;
use middle::lint::{unused_variable, dead_assignment};
use middle::pat_util; use middle::pat_util;
use middle::ty; use middle::ty;
use middle::typeck; use middle::typeck;
@ -118,6 +119,7 @@ use core::ptr;
use core::to_str; use core::to_str;
use core::uint; use core::uint;
use core::vec; use core::vec;
use core::util::with;
use syntax::ast::*; use syntax::ast::*;
use syntax::codemap::span; use syntax::codemap::span;
use syntax::parse::token::special_idents; use syntax::parse::token::special_idents;
@ -169,6 +171,7 @@ pub fn check_crate(tcx: ty::ctxt,
visit_local: visit_local, visit_local: visit_local,
visit_expr: visit_expr, visit_expr: visit_expr,
visit_arm: visit_arm, visit_arm: visit_arm,
visit_item: visit_item,
.. *visit::default_visitor() .. *visit::default_visitor()
}); });
@ -177,7 +180,8 @@ pub fn check_crate(tcx: ty::ctxt,
method_map, method_map,
variable_moves_map, variable_moves_map,
capture_map, capture_map,
last_use_map); last_use_map,
0);
visit::visit_crate(*crate, initial_maps, visitor); visit::visit_crate(*crate, initial_maps, visitor);
tcx.sess.abort_if_errors(); tcx.sess.abort_if_errors();
return last_use_map; return last_use_map;
@ -269,13 +273,16 @@ struct IrMaps {
capture_info_map: HashMap<node_id, @~[CaptureInfo]>, capture_info_map: HashMap<node_id, @~[CaptureInfo]>,
var_kinds: ~[VarKind], var_kinds: ~[VarKind],
lnks: ~[LiveNodeKind], lnks: ~[LiveNodeKind],
cur_item: node_id,
} }
fn IrMaps(tcx: ty::ctxt, fn IrMaps(tcx: ty::ctxt,
method_map: typeck::method_map, method_map: typeck::method_map,
variable_moves_map: moves::VariableMovesMap, variable_moves_map: moves::VariableMovesMap,
capture_map: moves::CaptureMap, capture_map: moves::CaptureMap,
last_use_map: last_use_map) last_use_map: last_use_map,
cur_item: node_id)
-> IrMaps { -> IrMaps {
IrMaps { IrMaps {
tcx: tcx, tcx: tcx,
@ -289,7 +296,8 @@ fn IrMaps(tcx: ty::ctxt,
variable_map: HashMap::new(), variable_map: HashMap::new(),
capture_info_map: HashMap::new(), capture_info_map: HashMap::new(),
var_kinds: ~[], var_kinds: ~[],
lnks: ~[] lnks: ~[],
cur_item: cur_item,
} }
} }
@ -394,6 +402,12 @@ pub impl IrMaps {
} }
} }
fn visit_item(item: @item, &&self: @mut IrMaps, v: vt<@mut IrMaps>) {
do with(&mut self.cur_item, item.id) {
visit::visit_item(item, self, v)
}
}
fn visit_fn(fk: &visit::fn_kind, fn visit_fn(fk: &visit::fn_kind,
decl: &fn_decl, decl: &fn_decl,
body: &blk, body: &blk,
@ -409,7 +423,8 @@ fn visit_fn(fk: &visit::fn_kind,
self.method_map, self.method_map,
self.variable_moves_map, self.variable_moves_map,
self.capture_map, self.capture_map,
self.last_use_map); self.last_use_map,
self.cur_item);
debug!("creating fn_maps: %x", ptr::addr_of(&(*fn_maps)) as uint); debug!("creating fn_maps: %x", ptr::addr_of(&(*fn_maps)) as uint);
@ -692,17 +707,19 @@ pub impl Liveness {
} }
} }
fn pat_bindings(&self, pat: @pat, f: &fn(LiveNode, Variable, span)) { fn pat_bindings(&self, pat: @pat,
f: &fn(LiveNode, Variable, span, node_id)) {
let def_map = self.tcx.def_map; let def_map = self.tcx.def_map;
do pat_util::pat_bindings(def_map, pat) |_bm, p_id, sp, _n| { do pat_util::pat_bindings(def_map, pat) |_bm, p_id, sp, _n| {
let ln = self.live_node(p_id, sp); let ln = self.live_node(p_id, sp);
let var = self.variable(p_id, sp); let var = self.variable(p_id, sp);
f(ln, var, sp); f(ln, var, sp, p_id);
} }
} }
fn arm_pats_bindings(&self, fn arm_pats_bindings(&self,
pats: &[@pat], f: &fn(LiveNode, Variable, span)) { pats: &[@pat],
f: &fn(LiveNode, Variable, span, node_id)) {
// only consider the first pattern; any later patterns must have // only consider the first pattern; any later patterns must have
// the same bindings, and we also consider the first pattern to be // the same bindings, and we also consider the first pattern to be
// the "authoratative" set of ids // the "authoratative" set of ids
@ -718,7 +735,7 @@ pub impl Liveness {
fn define_bindings_in_arm_pats(&self, pats: &[@pat], fn define_bindings_in_arm_pats(&self, pats: &[@pat],
succ: LiveNode) -> LiveNode { succ: LiveNode) -> LiveNode {
let mut succ = succ; let mut succ = succ;
do self.arm_pats_bindings(pats) |ln, var, _sp| { do self.arm_pats_bindings(pats) |ln, var, _sp, _id| {
self.init_from_succ(ln, succ); self.init_from_succ(ln, succ);
self.define(ln, var); self.define(ln, var);
succ = ln; succ = ln;
@ -1509,8 +1526,8 @@ fn check_local(local: @local, &&self: @Liveness, vt: vt<@Liveness>) {
// should not be live at this point. // should not be live at this point.
debug!("check_local() with no initializer"); debug!("check_local() with no initializer");
do self.pat_bindings(local.node.pat) |ln, var, sp| { do self.pat_bindings(local.node.pat) |ln, var, sp, id| {
if !self.warn_about_unused(sp, ln, var) { if !self.warn_about_unused(sp, id, ln, var) {
match self.live_on_exit(ln, var) { match self.live_on_exit(ln, var) {
None => { /* not live: good */ } None => { /* not live: good */ }
Some(lnk) => { Some(lnk) => {
@ -1528,8 +1545,8 @@ fn check_local(local: @local, &&self: @Liveness, vt: vt<@Liveness>) {
} }
fn check_arm(arm: &arm, &&self: @Liveness, vt: vt<@Liveness>) { fn check_arm(arm: &arm, &&self: @Liveness, vt: vt<@Liveness>) {
do self.arm_pats_bindings(arm.pats) |ln, var, sp| { do self.arm_pats_bindings(arm.pats) |ln, var, sp, id| {
self.warn_about_unused(sp, ln, var); self.warn_about_unused(sp, id, ln, var);
} }
visit::visit_arm(arm, self, vt); visit::visit_arm(arm, self, vt);
} }
@ -1691,14 +1708,14 @@ pub impl Liveness {
let ln = self.live_node(expr.id, expr.span); let ln = self.live_node(expr.id, expr.span);
let var = self.variable(nid, expr.span); let var = self.variable(nid, expr.span);
self.check_for_reassignment(ln, var, expr.span); self.check_for_reassignment(ln, var, expr.span);
self.warn_about_dead_assign(expr.span, ln, var); self.warn_about_dead_assign(expr.span, expr.id, ln, var);
} }
def => { def => {
match relevant_def(def) { match relevant_def(def) {
Some(nid) => { Some(nid) => {
let ln = self.live_node(expr.id, expr.span); let ln = self.live_node(expr.id, expr.span);
let var = self.variable(nid, expr.span); let var = self.variable(nid, expr.span);
self.warn_about_dead_assign(expr.span, ln, var); self.warn_about_dead_assign(expr.span, expr.id, ln, var);
} }
None => {} None => {}
} }
@ -1715,7 +1732,7 @@ pub impl Liveness {
} }
fn check_for_reassignments_in_pat(@self, pat: @pat) { fn check_for_reassignments_in_pat(@self, pat: @pat) {
do self.pat_bindings(pat) |ln, var, sp| { do self.pat_bindings(pat) |ln, var, sp, _id| {
self.check_for_reassignment(ln, var, sp); self.check_for_reassignment(ln, var, sp);
} }
} }
@ -1861,21 +1878,21 @@ pub impl Liveness {
do pat_util::pat_bindings(self.tcx.def_map, arg.pat) do pat_util::pat_bindings(self.tcx.def_map, arg.pat)
|_bm, p_id, sp, _n| { |_bm, p_id, sp, _n| {
let var = self.variable(p_id, sp); let var = self.variable(p_id, sp);
self.warn_about_unused(sp, entry_ln, var); self.warn_about_unused(sp, p_id, entry_ln, var);
} }
} }
} }
fn warn_about_unused_or_dead_vars_in_pat(@self, pat: @pat) { fn warn_about_unused_or_dead_vars_in_pat(@self, pat: @pat) {
do self.pat_bindings(pat) |ln, var, sp| { do self.pat_bindings(pat) |ln, var, sp, id| {
if !self.warn_about_unused(sp, ln, var) { if !self.warn_about_unused(sp, id, ln, var) {
self.warn_about_dead_assign(sp, ln, var); self.warn_about_dead_assign(sp, id, ln, var);
} }
} }
} }
fn warn_about_unused(@self, sp: span, ln: LiveNode, var: Variable) fn warn_about_unused(@self, sp: span, id: node_id,
-> bool { ln: LiveNode, var: Variable) -> bool {
if !self.used_on_entry(ln, var) { if !self.used_on_entry(ln, var) {
for self.should_warn(var).each |name| { for self.should_warn(var).each |name| {
@ -1889,14 +1906,14 @@ pub impl Liveness {
}; };
if is_assigned { if is_assigned {
// FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_lint(unused_variable, id,
self.tcx.sess.span_warn( self.ir.cur_item, sp,
sp, fmt!("variable `%s` is assigned to, \ fmt!("variable `%s` is assigned to, \
but never used", **name)); but never used", **name));
} else { } else {
// FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_lint(unused_variable, id,
self.tcx.sess.span_warn( self.ir.cur_item, sp,
sp, fmt!("unused variable: `%s`", **name)); fmt!("unused variable: `%s`", **name));
} }
} }
return true; return true;
@ -1904,12 +1921,12 @@ pub impl Liveness {
return false; return false;
} }
fn warn_about_dead_assign(@self, sp: span, ln: LiveNode, var: Variable) { fn warn_about_dead_assign(@self, sp: span, id: node_id,
ln: LiveNode, var: Variable) {
if self.live_on_exit(ln, var).is_none() { if self.live_on_exit(ln, var).is_none() {
for self.should_warn(var).each |name| { for self.should_warn(var).each |name| {
// FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_lint(dead_assignment, id,
self.tcx.sess.span_warn( self.ir.cur_item, sp,
sp,
fmt!("value assigned to `%s` is never read", **name)); fmt!("value assigned to `%s` is never read", **name));
} }
} }

View File

@ -8,38 +8,57 @@
// option. This file may not be copied, modified, or distributed // option. This file may not be copied, modified, or distributed
// except according to those terms. // except according to those terms.
#[deny(unused_variable)];
#[deny(dead_assignment)];
fn f1(x: int) { fn f1(x: int) {
//~^ WARNING unused variable: `x` //~^ ERROR unused variable: `x`
} }
fn f1b(x: &mut int) { fn f1b(x: &mut int) {
//~^ WARNING unused variable: `x` //~^ ERROR unused variable: `x`
} }
#[allow(unused_variable)]
fn f1c(x: int) {}
fn f2() { fn f2() {
let x = 3; let x = 3;
//~^ WARNING unused variable: `x` //~^ ERROR unused variable: `x`
} }
fn f3() { fn f3() {
let mut x = 3; let mut x = 3;
//~^ WARNING variable `x` is assigned to, but never used //~^ ERROR variable `x` is assigned to, but never used
x += 4; x += 4;
//~^ WARNING value assigned to `x` is never read //~^ ERROR value assigned to `x` is never read
} }
fn f3b() { fn f3b() {
let mut z = 3; let mut z = 3;
//~^ WARNING variable `z` is assigned to, but never used //~^ ERROR variable `z` is assigned to, but never used
loop { loop {
z += 4; z += 4;
} }
} }
#[allow(unused_variable)]
fn f3c() {
let mut z = 3;
loop { z += 4; }
}
#[allow(unused_variable)]
#[allow(dead_assignment)]
fn f3d() {
let mut x = 3;
x += 4;
}
fn f4() { fn f4() {
match Some(3) { match Some(3) {
Some(i) => { Some(i) => {
//~^ WARNING unused variable: `i` //~^ ERROR unused variable: `i`
} }
None => {} None => {}
} }
@ -57,16 +76,5 @@ fn f4b() -> int {
} }
} }
// leave this in here just to trigger compile-fail:
struct r {
x: (),
}
impl Drop for r {
fn finalize(&self) {}
}
fn main() { fn main() {
let x = r { x: () };
|| { copy x; }; //~ ERROR copying a value of non-copyable type
} }