Skip to content

Allow lint modes to be used on unused variables and dead assignments #5804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions src/librustc/middle/lint.rs
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
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
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
}