Skip to content

Commit

Permalink
auto merge of #5804 : alexcrichton/rust/issue-3266, r=graydon
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@bors
bors committed Apr 10, 2013
2 parents 80b6bbd + 3e67085 commit 6100bb5
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 60 deletions.
19 changes: 8 additions & 11 deletions src/librustc/middle/lint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ pub enum lint {

legacy_modes,

// FIXME(#3266)--make liveness warnings lintable
// unused_variable,
// dead_assignment
unused_variable,
dead_assignment,
}

pub fn level_to_str(lv: level) -> &'static str {
Expand Down Expand Up @@ -257,21 +256,19 @@ pub fn get_lint_dict() -> LintDict {
default: deny
}),

/* FIXME(#3266)--make liveness warnings lintable
(@~"unused_variable",
@LintSpec {
(~"unused_variable",
LintSpec {
lint: unused_variable,
desc: "detect variables which are not used in any way",
default: warn
}),
}),

(@~"dead_assignment",
@LintSpec {
(~"dead_assignment",
LintSpec {
lint: dead_assignment,
desc: "detect assignments that will never be read",
default: warn
}),
*/
}),
];
let mut map = HashMap::new();
do vec::consume(v) |_, (k, v)| {
Expand Down
79 changes: 48 additions & 31 deletions src/librustc/middle/liveness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@

use core::prelude::*;

use middle::lint::{unused_variable, dead_assignment};
use middle::pat_util;
use middle::ty;
use middle::typeck;
Expand All @@ -118,6 +119,7 @@ use core::ptr;
use core::to_str;
use core::uint;
use core::vec;
use core::util::with;
use syntax::ast::*;
use syntax::codemap::span;
use syntax::parse::token::special_idents;
Expand Down Expand Up @@ -169,6 +171,7 @@ pub fn check_crate(tcx: ty::ctxt,
visit_local: visit_local,
visit_expr: visit_expr,
visit_arm: visit_arm,
visit_item: visit_item,
.. *visit::default_visitor()
});

Expand All @@ -177,7 +180,8 @@ pub fn check_crate(tcx: ty::ctxt,
method_map,
variable_moves_map,
capture_map,
last_use_map);
last_use_map,
0);
visit::visit_crate(*crate, initial_maps, visitor);
tcx.sess.abort_if_errors();
return last_use_map;
Expand Down Expand Up @@ -269,13 +273,16 @@ struct IrMaps {
capture_info_map: HashMap<node_id, @~[CaptureInfo]>,
var_kinds: ~[VarKind],
lnks: ~[LiveNodeKind],

cur_item: node_id,
}

fn IrMaps(tcx: ty::ctxt,
method_map: typeck::method_map,
variable_moves_map: moves::VariableMovesMap,
capture_map: moves::CaptureMap,
last_use_map: last_use_map)
last_use_map: last_use_map,
cur_item: node_id)
-> IrMaps {
IrMaps {
tcx: tcx,
Expand All @@ -289,7 +296,8 @@ fn IrMaps(tcx: ty::ctxt,
variable_map: HashMap::new(),
capture_info_map: HashMap::new(),
var_kinds: ~[],
lnks: ~[]
lnks: ~[],
cur_item: cur_item,
}
}

Expand Down Expand Up @@ -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,
decl: &fn_decl,
body: &blk,
Expand All @@ -409,7 +423,8 @@ fn visit_fn(fk: &visit::fn_kind,
self.method_map,
self.variable_moves_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);

Expand Down Expand Up @@ -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;
do pat_util::pat_bindings(def_map, pat) |_bm, p_id, sp, _n| {
let ln = self.live_node(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,
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
// the same bindings, and we also consider the first pattern to be
// the "authoratative" set of ids
Expand All @@ -718,7 +735,7 @@ pub impl Liveness {
fn define_bindings_in_arm_pats(&self, pats: &[@pat],
succ: LiveNode) -> LiveNode {
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.define(ln, var);
succ = ln;
Expand Down Expand Up @@ -1509,8 +1526,8 @@ fn check_local(local: @local, &&self: @Liveness, vt: vt<@Liveness>) {
// should not be live at this point.

debug!("check_local() with no initializer");
do self.pat_bindings(local.node.pat) |ln, var, sp| {
if !self.warn_about_unused(sp, ln, var) {
do self.pat_bindings(local.node.pat) |ln, var, sp, id| {
if !self.warn_about_unused(sp, id, ln, var) {
match self.live_on_exit(ln, var) {
None => { /* not live: good */ }
Some(lnk) => {
Expand All @@ -1528,8 +1545,8 @@ fn check_local(local: @local, &&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| {
self.warn_about_unused(sp, ln, var);
do self.arm_pats_bindings(arm.pats) |ln, var, sp, id| {
self.warn_about_unused(sp, id, ln, var);
}
visit::visit_arm(arm, self, vt);
}
Expand Down Expand Up @@ -1691,14 +1708,14 @@ pub impl Liveness {
let ln = self.live_node(expr.id, expr.span);
let var = self.variable(nid, 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 => {
match relevant_def(def) {
Some(nid) => {
let ln = self.live_node(expr.id, 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 => {}
}
Expand All @@ -1715,7 +1732,7 @@ pub impl Liveness {
}

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);
}
}
Expand Down Expand Up @@ -1861,21 +1878,21 @@ pub impl Liveness {
do pat_util::pat_bindings(self.tcx.def_map, arg.pat)
|_bm, p_id, sp, _n| {
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) {
do self.pat_bindings(pat) |ln, var, sp| {
if !self.warn_about_unused(sp, ln, var) {
self.warn_about_dead_assign(sp, ln, var);
do self.pat_bindings(pat) |ln, var, sp, id| {
if !self.warn_about_unused(sp, id, ln, var) {
self.warn_about_dead_assign(sp, id, ln, var);
}
}
}

fn warn_about_unused(@self, sp: span, ln: LiveNode, var: Variable)
-> bool {
fn warn_about_unused(@self, sp: span, id: node_id,
ln: LiveNode, var: Variable) -> bool {
if !self.used_on_entry(ln, var) {
for self.should_warn(var).each |name| {

Expand All @@ -1889,27 +1906,27 @@ pub impl Liveness {
};

if is_assigned {
// FIXME(#3266)--make liveness warnings lintable
self.tcx.sess.span_warn(
sp, fmt!("variable `%s` is assigned to, \
self.tcx.sess.span_lint(unused_variable, id,
self.ir.cur_item, sp,
fmt!("variable `%s` is assigned to, \
but never used", **name));
} else {
// FIXME(#3266)--make liveness warnings lintable
self.tcx.sess.span_warn(
sp, fmt!("unused variable: `%s`", **name));
self.tcx.sess.span_lint(unused_variable, id,
self.ir.cur_item, sp,
fmt!("unused variable: `%s`", **name));
}
}
return true;
}
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() {
for self.should_warn(var).each |name| {
// FIXME(#3266)--make liveness warnings lintable
self.tcx.sess.span_warn(
sp,
self.tcx.sess.span_lint(dead_assignment, id,
self.ir.cur_item, sp,
fmt!("value assigned to `%s` is never read", **name));
}
}
Expand Down
44 changes: 26 additions & 18 deletions src/test/compile-fail/liveness-unused.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,57 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[deny(unused_variable)];
#[deny(dead_assignment)];

fn f1(x: int) {
//~^ WARNING unused variable: `x`
//~^ ERROR unused variable: `x`
}

fn f1b(x: &mut int) {
//~^ WARNING unused variable: `x`
//~^ ERROR unused variable: `x`
}

#[allow(unused_variable)]
fn f1c(x: int) {}

fn f2() {
let x = 3;
//~^ WARNING unused variable: `x`
//~^ ERROR unused variable: `x`
}

fn f3() {
let mut x = 3;
//~^ WARNING variable `x` is assigned to, but never used
//~^ ERROR variable `x` is assigned to, but never used
x += 4;
//~^ WARNING value assigned to `x` is never read
//~^ ERROR value assigned to `x` is never read
}

fn f3b() {
let mut z = 3;
//~^ WARNING variable `z` is assigned to, but never used
//~^ ERROR variable `z` is assigned to, but never used
loop {
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() {
match Some(3) {
Some(i) => {
//~^ WARNING unused variable: `i`
//~^ ERROR unused variable: `i`
}
None => {}
}
Expand All @@ -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() {
let x = r { x: () };
|| { copy x; }; //~ ERROR copying a value of non-copyable type
}

0 comments on commit 6100bb5

Please sign in to comment.