diff --git a/src/libcore/hashmap.rs b/src/libcore/hashmap.rs index 7418239f765..d9912813cf9 100644 --- a/src/libcore/hashmap.rs +++ b/src/libcore/hashmap.rs @@ -17,7 +17,6 @@ use container::{Container, Mutable, Map, Set}; use cmp::{Eq, Equiv}; use hash::Hash; use old_iter::BaseIter; -use hash::Hash; use old_iter; use option::{None, Option, Some}; use rand::RngUtil; diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index 0e2739e40a9..92d3a451559 100644 --- a/src/librustc/back/link.rs +++ b/src/librustc/back/link.rs @@ -171,7 +171,6 @@ pub mod write { use back::link::{output_type_assembly, output_type_bitcode}; use back::link::{output_type_exe, output_type_llvm_assembly}; use back::link::{output_type_object}; - use back::link::output_type; use driver::session::Session; use driver::session; use lib::llvm::llvm; diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 0e37653e5c4..99ffa8cc94a 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -22,6 +22,7 @@ use middle; use util::common::time; use util::ppaux; +use core::hashmap::HashMap; use core::int; use core::io; use core::os; @@ -200,9 +201,6 @@ pub fn compile_rest(sess: Session, crate = time(time_passes, ~"core injection", || front::core_inject::maybe_inject_libcore_ref(sess, crate)); - time(time_passes, ~"building lint settings table", || - lint::build_settings_crate(sess, crate)); - let ast_map = time(time_passes, ~"ast indexing", || syntax::ast_map::map_crate(sess.diagnostic(), crate)); @@ -709,7 +707,6 @@ pub fn build_session_(sopts: @session::options, &sopts.maybe_sysroot, sopts.target_triple, /*bad*/copy sopts.addl_lib_search_paths); - let lint_settings = lint::mk_lint_settings(); @Session_ { targ_cfg: target_cfg, opts: sopts, @@ -723,7 +720,7 @@ pub fn build_session_(sopts: @session::options, filesearch: filesearch, building_library: @mut false, working_dir: os::getcwd(), - lint_settings: lint_settings + lints: @mut HashMap::new(), } } diff --git a/src/librustc/driver/session.rs b/src/librustc/driver/session.rs index 16eec0b10de..6fba5ec8d3a 100644 --- a/src/librustc/driver/session.rs +++ b/src/librustc/driver/session.rs @@ -26,6 +26,8 @@ use syntax::{ast, codemap}; use syntax::abi; use syntax; +use core::hashmap::HashMap; + #[deriving(Eq)] pub enum os { os_win32, os_macos, os_linux, os_android, os_freebsd, } @@ -170,7 +172,7 @@ pub struct Session_ { filesearch: @filesearch::FileSearch, building_library: @mut bool, working_dir: Path, - lint_settings: lint::LintSettings + lints: @mut HashMap, } pub type Session = @Session_; @@ -221,24 +223,12 @@ pub impl Session_ { fn unimpl(@self, msg: &str) -> ! { self.span_diagnostic.handler().unimpl(msg) } - fn span_lint_level(@self, level: lint::level, sp: span, msg: &str) { - match level { - lint::allow => { }, - lint::warn => self.span_warn(sp, msg), - lint::deny | lint::forbid => { - self.span_err(sp, msg); - } + fn add_lint(@self, lint: lint::lint, id: ast::node_id, sp: span, msg: ~str) { + match self.lints.find_mut(&id) { + Some(arr) => { arr.push((lint, sp, msg)); return; } + None => {} } - } - fn span_lint(@self, lint_mode: lint::lint, - expr_id: ast::node_id, - item_id: ast::node_id, - span: span, - msg: &str) { - let level = lint::get_lint_settings_level( - self.lint_settings, lint_mode, expr_id, item_id); - let msg = fmt!("%s [-W %s]", msg, lint::get_lint_name(lint_mode)); - self.span_lint_level(level, span, msg); + self.lints.insert(id, ~[(lint, sp, msg)]); } fn next_node_id(@self) -> ast::node_id { return syntax::parse::next_node_id(self.parse_sess); diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 7ea0840880c..aecbcb626e9 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use driver::session::Session; use driver::session; use middle::ty; use middle::pat_util; @@ -19,7 +18,7 @@ use std::smallintmap::SmallIntMap; use syntax::attr; use syntax::codemap::span; use syntax::codemap; -use syntax::{ast, visit}; +use syntax::{ast, visit, ast_util}; /** * A 'lint' check is a kind of miscellaneous constraint that a user _might_ @@ -28,18 +27,34 @@ use syntax::{ast, visit}; * other phases of the compiler, which are generally required to hold in order * to compile the program at all. * - * We also build up a table containing information about lint settings, in - * order to allow other passes to take advantage of the lint attribute - * infrastructure. To save space, the table is keyed by the id of /items/, not - * of every expression. When an item has the default settings, the entry will - * be omitted. If we start allowing lint attributes on expressions, we will - * start having entries for expressions that do not share their enclosing - * items settings. + * The lint checking is all consolidated into one pass which runs just before + * translation to LLVM bytecode. Throughout compilation, lint warnings can be + * added via the `add_lint` method on the Session structure. This requires a + * span and an id of the node that the lint is being added to. The lint isn't + * actually emitted at that time because it is unknown what the actual lint + * level at that location is. * - * This module then, exports two passes: one that populates the lint - * settings table in the session and is run early in the compile process, and - * one that does a variety of lint checks, and is run late in the compile - * process. + * To actually emit lint warnings/errors, a separate pass is used just before + * translation. A context keeps track of the current state of all lint levels. + * Upon entering a node of the ast which can modify the lint settings, the + * previous lint state is pushed onto a stack and the ast is then recursed upon. + * As the ast is traversed, this keeps track of the current lint level for all + * lint attributes. + * + * At each node of the ast which can modify lint attributes, all known lint + * passes are also applied. Each lint pass is a visit::vt<()> structure. These + * visitors are constructed via the lint_*() functions below. There are also + * some lint checks which operate directly on ast nodes (such as @ast::item), + * and those are organized as check_item_*(). Each visitor added to the lint + * context is modified to stop once it reaches a node which could alter the lint + * levels. This means that everything is looked at once and only once by every + * lint pass. + * + * With this all in place, to add a new lint warning, all you need to do is to + * either invoke `add_lint` on the session at the appropriate time, or write a + * lint pass in this module which is just an ast visitor. The context used when + * traversing the ast has a `span_lint` method which only needs the span of the + * item that's being warned about. */ #[deriving(Eq)] @@ -86,7 +101,20 @@ struct LintSpec { default: level } -pub type LintDict = @HashMap<~str, LintSpec>; +pub type LintDict = HashMap<~str, LintSpec>; + +enum AttributedNode<'self> { + Item(@ast::item), + Method(&'self ast::method), + Crate(@ast::crate), +} + +#[deriving(Eq)] +enum LintSource { + Node(span), + Default, + CommandLine +} static lint_table: &'static [(&'static str, LintSpec)] = &[ ("ctypes", @@ -225,82 +253,90 @@ pub fn get_lint_dict() -> LintDict { for lint_table.each|&(k, v)| { map.insert(k.to_str(), v); } - return @map; -} - -pub fn get_lint_name(lint_mode: lint) -> ~str { - for lint_table.each |&(name, spec)| { - if spec.lint == lint_mode { - return name.to_str(); - } - } - fail!(); -} -// This is a highly not-optimal set of data structure decisions. -type LintModes = @mut SmallIntMap; -type LintModeMap = @mut HashMap; - -// settings_map maps node ids of items with non-default lint settings -// to their settings; default_settings contains the settings for everything -// not in the map. -pub struct LintSettings { - default_settings: LintModes, - settings_map: LintModeMap -} - -pub fn mk_lint_settings() -> LintSettings { - LintSettings { - default_settings: @mut SmallIntMap::new(), - settings_map: @mut HashMap::new() - } -} - -pub fn get_lint_level(modes: LintModes, lint: lint) -> level { - match modes.find(&(lint as uint)) { - Some(&c) => c, - None => allow - } -} - -pub fn get_lint_settings_level(settings: LintSettings, - lint_mode: lint, - _expr_id: ast::node_id, - item_id: ast::node_id) - -> level { - match settings.settings_map.find(&item_id) { - Some(&modes) => get_lint_level(modes, lint_mode), - None => get_lint_level(settings.default_settings, lint_mode) - } -} - -// This is kind of unfortunate. It should be somewhere else, or we should use -// a persistent data structure... -fn clone_lint_modes(modes: LintModes) -> LintModes { - @mut (copy *modes) + return map; } struct Context { - dict: LintDict, - curr: LintModes, - is_default: bool, - sess: Session + // All known lint modes (string versions) + dict: @LintDict, + // Current levels of each lint warning + curr: SmallIntMap<(level, LintSource)>, + // context we're checking in (used to access fields like sess) + tcx: ty::ctxt, + // When recursing into an attributed node of the ast which modifies lint + // levels, this stack keeps track of the previous lint levels of whatever + // was modified. + lint_stack: ~[(lint, level, LintSource)], + // Each of these visitors represents a lint pass. A number of the lint + // attributes are registered by adding a visitor to iterate over the ast. + // Others operate directly on @ast::item structures (or similar). Finally, + // others still are added to the Session object via `add_lint`, and these + // are all passed with the lint_session visitor. + visitors: ~[visit::vt<()>], } -pub impl Context { +impl Context { fn get_level(&self, lint: lint) -> level { - get_lint_level(self.curr, lint) - } - - fn set_level(&self, lint: lint, level: level) { - if level == allow { - self.curr.remove(&(lint as uint)); - } else { - self.curr.insert(lint as uint, level); + match self.curr.find(&(lint as uint)) { + Some(&(lvl, _)) => lvl, + None => allow } } - fn span_lint(&self, level: level, span: span, msg: ~str) { - self.sess.span_lint_level(level, span, msg); + fn get_source(&self, lint: lint) -> LintSource { + match self.curr.find(&(lint as uint)) { + Some(&(_, src)) => src, + None => Default + } + } + + fn set_level(&mut self, lint: lint, level: level, src: LintSource) { + if level == allow { + self.curr.remove(&(lint as uint)); + } else { + self.curr.insert(lint as uint, (level, src)); + } + } + + fn lint_to_str(&self, lint: lint) -> ~str { + for self.dict.each |k, v| { + if v.lint == lint { + return copy *k; + } + } + fail!("unregistered lint %?", lint); + } + + fn span_lint(&self, lint: lint, span: span, msg: &str) { + let (level, src) = match self.curr.find(&(lint as uint)) { + Some(&pair) => pair, + None => { return; } + }; + if level == allow { return; } + + let mut note = None; + let msg = match src { + Default | CommandLine => { + fmt!("%s [-%c %s%s]", msg, match level { + warn => 'W', deny => 'D', forbid => 'F', + allow => fail!() + }, str::replace(self.lint_to_str(lint), "_", "-"), + if src == Default { " (default)" } else { "" }) + }, + Node(src) => { + note = Some(src); + msg.to_str() + } + }; + match level { + warn => { self.tcx.sess.span_warn(span, msg); } + deny | forbid => { self.tcx.sess.span_err(span, msg); } + allow => fail!(), + } + + for note.each |&span| { + self.tcx.sess.span_note(span, "lint level defined here"); + } } /** @@ -308,189 +344,191 @@ pub impl Context { * current lint context, call the provided function, then reset the * lints in effect to their previous state. */ - fn with_lint_attrs(&self, attrs: ~[ast::attribute], f: &fn(Context)) { - - let mut new_ctxt = *self; - let mut triples = ~[]; - - for [allow, warn, deny, forbid].each |level| { - let level_name = level_to_str(*level); - let metas = - attr::attr_metas(attr::find_attrs_by_name(attrs, level_name)); - for metas.each |meta| { - match meta.node { - ast::meta_list(_, ref metas) => { - for metas.each |meta| { - match meta.node { - ast::meta_word(ref lintname) => { - triples.push((*meta, - *level, - /*bad*/copy *lintname)); - } - _ => { - self.sess.span_err( - meta.span, - "malformed lint attribute"); - } - } - } - } - _ => { - self.sess.span_err(meta.span, - "malformed lint attribute"); - } - } - } - } - - for triples.each |triple| { - // FIXME(#3874): it would be nicer to write this... - // let (meta, level, lintname) = /*bad*/copy *pair; - let (meta, level, lintname) = match *triple { - (ref meta, level, lintname) => (meta, level, lintname) - }; - - match self.dict.find(lintname) { + fn with_lint_attrs(@mut self, attrs: &[ast::attribute], f: &fn()) { + // Parse all of the lint attributes, and then add them all to the + // current dictionary of lint information. Along the way, keep a history + // of what we changed so we can roll everything back after invoking the + // specified closure + let mut pushed = 0u; + for each_lint(self.tcx.sess, attrs) |meta, level, lintname| { + let lint = match self.dict.find(lintname) { None => { self.span_lint( - new_ctxt.get_level(unrecognized_lint), + unrecognized_lint, meta.span, fmt!("unknown `%s` attribute: `%s`", level_to_str(level), *lintname)); + loop } - Some(lint) => { + Some(lint) => { lint.lint } + }; - if new_ctxt.get_level(lint.lint) == forbid && - level != forbid { - self.span_lint( - forbid, - meta.span, - fmt!("%s(%s) overruled by outer forbid(%s)", - level_to_str(level), - *lintname, *lintname)); - } + let now = self.get_level(lint); + if now == forbid && level != forbid { + self.tcx.sess.span_err(meta.span, + fmt!("%s(%s) overruled by outer forbid(%s)", + level_to_str(level), + *lintname, *lintname)); + loop; + } - // we do multiple unneeded copies of the - // map if many attributes are set, but - // this shouldn't actually be a problem... - - let c = clone_lint_modes(new_ctxt.curr); - new_ctxt = Context { - is_default: false, - curr: c, - .. new_ctxt - }; - new_ctxt.set_level(lint.lint, level); - } + if now != level { + let src = self.get_source(lint); + self.lint_stack.push((lint, now, src)); + pushed += 1; + self.set_level(lint, level, Node(meta.span)); } } - f(new_ctxt); - } -} + f(); -fn build_settings_item(i: @ast::item, cx: Context, v: visit::vt) { - do cx.with_lint_attrs(/*bad*/copy i.attrs) |cx| { - if !cx.is_default { - cx.sess.lint_settings.settings_map.insert(i.id, cx.curr); + // rollback + for pushed.times { + let (lint, lvl, src) = self.lint_stack.pop(); + self.set_level(lint, lvl, src); } - visit::visit_item(i, cx, v); - } -} - -pub fn build_settings_crate(sess: session::Session, crate: @ast::crate) { - let cx = Context { - dict: get_lint_dict(), - curr: @mut SmallIntMap::new(), - is_default: true, - sess: sess - }; - - // Install defaults. - for cx.dict.each_value |&spec| { - cx.set_level(spec.lint, spec.default); } - // Install command-line options, overriding defaults. - for sess.opts.lint_opts.each |pair| { - let (lint,level) = *pair; - cx.set_level(lint, level); + fn add_lint(&mut self, v: visit::vt<()>) { + self.visitors.push(item_stopping_visitor(v)); } - do cx.with_lint_attrs(/*bad*/copy crate.node.attrs) |cx| { - // Copy out the default settings - for cx.curr.each |&k, &v| { - sess.lint_settings.default_settings.insert(k, v); + fn process(&self, n: AttributedNode) { + match n { + Item(it) => { + for self.visitors.each |v| { + visit::visit_item(it, (), *v); + } + } + Crate(c) => { + for self.visitors.each |v| { + visit::visit_crate(c, (), *v); + } + } + // Can't use visit::visit_method_helper because the + // item_stopping_visitor has overridden visit_fn(&fk_method(... )) + // to be a no-op, so manually invoke visit_fn. + Method(m) => { + let fk = visit::fk_method(copy m.ident, &m.generics, m); + for self.visitors.each |v| { + visit::visit_fn(&fk, &m.decl, &m.body, m.span, m.id, + (), *v); + } + } } - - let cx = Context { - is_default: true, - .. cx - }; - - let visit = visit::mk_vt(@visit::Visitor { - visit_item: build_settings_item, - .. *visit::default_visitor() - }); - visit::visit_crate(crate, cx, visit); } - - sess.abort_if_errors(); } -fn check_item(i: @ast::item, cx: ty::ctxt) { - check_item_ctypes(cx, i); - check_item_while_true(cx, i); - check_item_path_statement(cx, i); - check_item_non_camel_case_types(cx, i); - check_item_heap(cx, i); - check_item_type_limits(cx, i); - check_item_default_methods(cx, i); - check_item_unused_unsafe(cx, i); - check_item_unused_mut(cx, i); +#[cfg(stage0)] +pub fn each_lint(sess: session::Session, + attrs: &[ast::attribute], + f: &fn(@ast::meta_item, level, &~str) -> bool) +{ + for [allow, warn, deny, forbid].each |&level| { + let level_name = level_to_str(level); + let attrs = attr::find_attrs_by_name(attrs, level_name); + for attrs.each |attr| { + let meta = attr.node.value; + let metas = match meta.node { + ast::meta_list(_, ref metas) => metas, + _ => { + sess.span_err(meta.span, ~"malformed lint attribute"); + loop; + } + }; + for metas.each |meta| { + match meta.node { + ast::meta_word(lintname) => { + if !f(*meta, level, lintname) { + return; + } + } + _ => { + sess.span_err(meta.span, ~"malformed lint attribute"); + } + } + } + } + } +} +#[cfg(not(stage0))] +pub fn each_lint(sess: session::Session, + attrs: &[ast::attribute], + f: &fn(@ast::meta_item, level, &~str) -> bool) -> bool +{ + for [allow, warn, deny, forbid].each |&level| { + let level_name = level_to_str(level); + let attrs = attr::find_attrs_by_name(attrs, level_name); + for attrs.each |attr| { + let meta = attr.node.value; + let metas = match meta.node { + ast::meta_list(_, ref metas) => metas, + _ => { + sess.span_err(meta.span, ~"malformed lint attribute"); + loop; + } + }; + for metas.each |meta| { + match meta.node { + ast::meta_word(lintname) => { + if !f(*meta, level, lintname) { + return false; + } + } + _ => { + sess.span_err(meta.span, ~"malformed lint attribute"); + } + } + } + } + } + return true; } // Take a visitor, and modify it so that it will not proceed past subitems. // This is used to make the simple visitors used for the lint passes // not traverse into subitems, since that is handled by the outer // lint visitor. -fn item_stopping_visitor(v: visit::vt) -> visit::vt { - visit::mk_vt(@visit::Visitor {visit_item: |_i, _e, _v| { }, - .. **(ty_stopping_visitor(v))}) +fn item_stopping_visitor(v: visit::vt) -> visit::vt { + visit::mk_vt(@visit::Visitor { + visit_item: |_i, _e, _v| { }, + visit_fn: |fk, fd, b, s, id, e, v| { + match *fk { + visit::fk_method(*) => {} + _ => visit::visit_fn(fk, fd, b, s, id, e, v) + } + }, + .. **(ty_stopping_visitor(v))}) } fn ty_stopping_visitor(v: visit::vt) -> visit::vt { visit::mk_vt(@visit::Visitor {visit_ty: |_t, _e, _v| { },.. **v}) } -fn check_item_while_true(cx: ty::ctxt, it: @ast::item) { - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_expr: |e: @ast::expr| { - match e.node { - ast::expr_while(cond, _) => { - match cond.node { - ast::expr_lit(@codemap::spanned { - node: ast::lit_bool(true), _}) => - { - cx.sess.span_lint( - while_true, e.id, it.id, - e.span, - "denote infinite loops \ - with loop { ... }"); - } - _ => () +fn lint_while_true(cx: @mut Context) -> visit::vt<()> { + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_expr: |e: @ast::expr| { + match e.node { + ast::expr_while(cond, _) => { + match cond.node { + ast::expr_lit(@codemap::spanned { + node: ast::lit_bool(true), _}) => + { + cx.span_lint(while_true, e.span, + "denote infinite loops with \ + loop { ... }"); } + _ => () } - _ => () } - }, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); + _ => () + } + }, + .. *visit::default_simple_visitor() + }) } -fn check_item_type_limits(cx: ty::ctxt, it: @ast::item) { +fn lint_type_limits(cx: @mut Context) -> visit::vt<()> { fn is_valid(binop: ast::binop, v: T, min: T, max: T) -> bool { match binop { @@ -534,7 +572,7 @@ fn check_item_type_limits(cx: ty::ctxt, it: @ast::item) { } } - fn check_limits(cx: ty::ctxt, binop: ast::binop, l: &ast::expr, + fn check_limits(cx: @mut Context, binop: ast::binop, l: &ast::expr, r: &ast::expr) -> bool { let (lit, expr, swap) = match (&l.node, &r.node) { (&ast::expr_lit(_), _) => (l, r, true), @@ -543,12 +581,12 @@ fn check_item_type_limits(cx: ty::ctxt, it: @ast::item) { }; // Normalize the binop so that the literal is always on the RHS in // the comparison - let norm_binop = if (swap) { + let norm_binop = if swap { rev_binop(binop) } else { binop }; - match ty::get(ty::expr_ty(cx, @/*bad*/copy *expr)).sty { + match ty::get(ty::expr_ty(cx.tcx, @/*bad*/copy *expr)).sty { ty::ty_int(int_ty) => { let (min, max) = int_ty_range(int_ty); let lit_val: i64 = match lit.node { @@ -592,36 +630,29 @@ fn check_item_type_limits(cx: ty::ctxt, it: @ast::item) { ast::expr_binary(ref binop, @ref l, @ref r) => { if is_comparison(*binop) && !check_limits(cx, *binop, l, r) { - cx.sess.span_lint( - type_limits, e.id, it.id, e.span, - "comparison is useless due to type limits"); + cx.span_lint(type_limits, e.span, + "comparison is useless due to type limits"); } } _ => () } }; - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_expr: visit_expr, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_expr: visit_expr, + .. *visit::default_simple_visitor() + }) } -fn check_item_default_methods(cx: ty::ctxt, item: @ast::item) { +fn check_item_default_methods(cx: @mut Context, item: @ast::item) { match item.node { ast::item_trait(_, _, ref methods) => { for methods.each |method| { match *method { ast::required(*) => {} ast::provided(*) => { - cx.sess.span_lint( - default_methods, - item.id, - item.id, - item.span, - "default methods are experimental"); + cx.span_lint(default_methods, item.span, + "default methods are experimental"); } } } @@ -630,25 +661,21 @@ fn check_item_default_methods(cx: ty::ctxt, item: @ast::item) { } } -fn check_item_ctypes(cx: ty::ctxt, it: @ast::item) { - fn check_foreign_fn(cx: ty::ctxt, fn_id: ast::node_id, - decl: &ast::fn_decl) { +fn check_item_ctypes(cx: @mut Context, it: @ast::item) { + + fn check_foreign_fn(cx: @mut Context, decl: &ast::fn_decl) { let tys = vec::map(decl.inputs, |a| a.ty ); for vec::each(vec::append_one(tys, decl.output)) |ty| { match ty.node { ast::ty_path(_, id) => { - match cx.def_map.get_copy(&id) { + match cx.tcx.def_map.get_copy(&id) { ast::def_prim_ty(ast::ty_int(ast::ty_i)) => { - cx.sess.span_lint( - ctypes, id, fn_id, - ty.span, + cx.span_lint(ctypes, ty.span, "found rust type `int` in foreign module, while \ libc::c_int or libc::c_long should be used"); } ast::def_prim_ty(ast::ty_uint(ast::ty_u)) => { - cx.sess.span_lint( - ctypes, id, fn_id, - ty.span, + cx.span_lint(ctypes, ty.span, "found rust type `uint` in foreign module, while \ libc::c_uint or libc::c_ulong should be used"); } @@ -665,7 +692,7 @@ fn check_item_ctypes(cx: ty::ctxt, it: @ast::item) { for nmod.items.each |ni| { match ni.node { ast::foreign_item_fn(ref decl, _, _) => { - check_foreign_fn(cx, it.id, decl); + check_foreign_fn(cx, decl); } // FIXME #4622: Not implemented. ast::foreign_item_const(*) => {} @@ -676,57 +703,47 @@ fn check_item_ctypes(cx: ty::ctxt, it: @ast::item) { } } -fn check_item_heap(cx: ty::ctxt, it: @ast::item) { +fn check_type_for_lint(cx: @mut Context, lint: lint, span: span, ty: ty::t) { + if cx.get_level(lint) == allow { return } - fn check_type_for_lint(cx: ty::ctxt, lint: lint, - node: ast::node_id, - item: ast::node_id, - span: span, ty: ty::t) { + let mut n_box = 0; + let mut n_uniq = 0; + ty::fold_ty(cx.tcx, ty, |t| { + match ty::get(t).sty { + ty::ty_box(_) => n_box += 1, + ty::ty_uniq(_) => n_uniq += 1, + _ => () + }; + t + }); - if get_lint_settings_level(cx.sess.lint_settings, - lint, node, item) != allow { - let mut n_box = 0; - let mut n_uniq = 0; - ty::fold_ty(cx, ty, |t| { - match ty::get(t).sty { - ty::ty_box(_) => n_box += 1, - ty::ty_uniq(_) => n_uniq += 1, - _ => () - }; - t - }); - - if (n_uniq > 0 && lint != managed_heap_memory) { - let s = ty_to_str(cx, ty); - let m = ~"type uses owned (~ type) pointers: " + s; - cx.sess.span_lint(lint, node, item, span, m); - } - - if (n_box > 0 && lint != owned_heap_memory) { - let s = ty_to_str(cx, ty); - let m = ~"type uses managed (@ type) pointers: " + s; - cx.sess.span_lint(lint, node, item, span, m); - } - } + if n_uniq > 0 && lint != managed_heap_memory { + let s = ty_to_str(cx.tcx, ty); + let m = ~"type uses owned (~ type) pointers: " + s; + cx.span_lint(lint, span, m); } - fn check_type(cx: ty::ctxt, - node: ast::node_id, - item: ast::node_id, - span: span, ty: ty::t) { - for [managed_heap_memory, - owned_heap_memory, - heap_memory].each |lint| { - check_type_for_lint(cx, *lint, node, item, span, ty); - } + if n_box > 0 && lint != owned_heap_memory { + let s = ty_to_str(cx.tcx, ty); + let m = ~"type uses managed (@ type) pointers: " + s; + cx.span_lint(lint, span, m); } +} +fn check_type(cx: @mut Context, span: span, ty: ty::t) { + for [managed_heap_memory, owned_heap_memory, heap_memory].each |lint| { + check_type_for_lint(cx, *lint, span, ty); + } +} + +fn check_item_heap(cx: @mut Context, it: @ast::item) { match it.node { ast::item_fn(*) | ast::item_ty(*) | ast::item_enum(*) | - ast::item_struct(*) => check_type(cx, it.id, it.id, it.span, - ty::node_id_to_type(cx, it.id)), + ast::item_struct(*) => check_type(cx, it.span, + ty::node_id_to_type(cx.tcx, + it.id)), _ => () } @@ -734,48 +751,44 @@ fn check_item_heap(cx: ty::ctxt, it: @ast::item) { match it.node { ast::item_struct(struct_def, _) => { for struct_def.fields.each |struct_field| { - check_type(cx, struct_field.node.id, it.id, - struct_field.span, - ty::node_id_to_type(cx, struct_field.node.id)); + check_type(cx, struct_field.span, + ty::node_id_to_type(cx.tcx, + struct_field.node.id)); } } _ => () } - - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_expr: |e: @ast::expr| { - let ty = ty::expr_ty(cx, e); - check_type(cx, e.id, it.id, e.span, ty); - }, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); } -fn check_item_path_statement(cx: ty::ctxt, it: @ast::item) { - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_stmt: |s: @ast::stmt| { - match s.node { - ast::stmt_semi( - @ast::expr { id: id, node: ast::expr_path(_), _ }, - _ - ) => { - cx.sess.span_lint( - path_statement, id, it.id, - s.span, - "path statement with no effect"); - } - _ => () +fn lint_heap(cx: @mut Context) -> visit::vt<()> { + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_expr: |e| { + let ty = ty::expr_ty(cx.tcx, e); + check_type(cx, e.span, ty); + }, + .. *visit::default_simple_visitor() + }) +} + +fn lint_path_statement(cx: @mut Context) -> visit::vt<()> { + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_stmt: |s| { + match s.node { + ast::stmt_semi( + @ast::expr { node: ast::expr_path(_), _ }, + _ + ) => { + cx.span_lint(path_statement, s.span, + "path statement with no effect"); } - }, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); + _ => () + } + }, + .. *visit::default_simple_visitor() + }) } -fn check_item_non_camel_case_types(cx: ty::ctxt, it: @ast::item) { +fn check_item_non_camel_case_types(cx: @mut Context, it: @ast::item) { fn is_camel_case(cx: ty::ctxt, ident: ast::ident) -> bool { let ident = cx.sess.str_of(ident); assert!(!ident.is_empty()); @@ -799,61 +812,54 @@ fn check_item_non_camel_case_types(cx: ty::ctxt, it: @ast::item) { } } - fn check_case(cx: ty::ctxt, ident: ast::ident, - expr_id: ast::node_id, item_id: ast::node_id, - span: span) { - if !is_camel_case(cx, ident) { - cx.sess.span_lint( - non_camel_case_types, expr_id, item_id, span, - "type, variant, or trait should have \ - a camel case identifier"); + fn check_case(cx: @mut Context, ident: ast::ident, span: span) { + if !is_camel_case(cx.tcx, ident) { + cx.span_lint(non_camel_case_types, span, + "type, variant, or trait should have \ + a camel case identifier"); } } match it.node { ast::item_ty(*) | ast::item_struct(*) | ast::item_trait(*) => { - check_case(cx, it.ident, it.id, it.id, it.span) + check_case(cx, it.ident, it.span) } ast::item_enum(ref enum_definition, _) => { - check_case(cx, it.ident, it.id, it.id, it.span); + check_case(cx, it.ident, it.span); for enum_definition.variants.each |variant| { - check_case(cx, variant.node.name, - variant.node.id, it.id, variant.span); + check_case(cx, variant.node.name, variant.span); } } _ => () } } -fn check_item_unused_unsafe(cx: ty::ctxt, it: @ast::item) { +fn lint_unused_unsafe(cx: @mut Context) -> visit::vt<()> { let visit_expr: @fn(@ast::expr) = |e| { match e.node { ast::expr_block(ref blk) if blk.node.rules == ast::unsafe_blk => { - if !cx.used_unsafe.contains(&blk.node.id) { - cx.sess.span_lint(unused_unsafe, blk.node.id, it.id, - blk.span, - "unnecessary `unsafe` block"); + if !cx.tcx.used_unsafe.contains(&blk.node.id) { + cx.span_lint(unused_unsafe, blk.span, + "unnecessary `unsafe` block"); } } _ => () } }; - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_expr: visit_expr, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_expr: visit_expr, + .. *visit::default_simple_visitor() + }) } -fn check_item_unused_mut(tcx: ty::ctxt, it: @ast::item) { +fn lint_unused_mut(cx: @mut Context) -> visit::vt<()> { let check_pat: @fn(@ast::pat) = |p| { let mut used = false; let mut bindings = 0; - do pat_util::pat_bindings(tcx.def_map, p) |_, id, _, _| { - used = used || tcx.used_mut_nodes.contains(&id); + do pat_util::pat_bindings(cx.tcx.def_map, p) |_, id, _, _| { + used = used || cx.tcx.used_mut_nodes.contains(&id); bindings += 1; } if !used { @@ -862,7 +868,7 @@ fn check_item_unused_mut(tcx: ty::ctxt, it: @ast::item) { } else { "variables do not need to be mutable" }; - tcx.sess.span_lint(unused_mut, p.id, it.id, p.span, msg); + cx.span_lint(unused_mut, p.span, msg); } }; @@ -874,45 +880,116 @@ fn check_item_unused_mut(tcx: ty::ctxt, it: @ast::item) { } }; - let visit = item_stopping_visitor( - visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_local: |l| { - if l.node.is_mutbl { - check_pat(l.node.pat); - } - }, - visit_fn: |_, fd, _, _, _| visit_fn_decl(fd), - visit_ty_method: |tm| visit_fn_decl(&tm.decl), - visit_struct_method: |sm| visit_fn_decl(&sm.decl), - visit_trait_method: |tm| { - match *tm { - ast::required(ref tm) => visit_fn_decl(&tm.decl), - ast::provided(m) => visit_fn_decl(&m.decl), - } - }, - .. *visit::default_simple_visitor() - })); - visit::visit_item(it, (), visit); + visit::mk_simple_visitor(@visit::SimpleVisitor { + visit_local: |l| { + if l.node.is_mutbl { + check_pat(l.node.pat); + } + }, + visit_fn: |_, fd, _, _, _| visit_fn_decl(fd), + visit_ty_method: |tm| visit_fn_decl(&tm.decl), + visit_struct_method: |sm| visit_fn_decl(&sm.decl), + visit_trait_method: |tm| { + match *tm { + ast::required(ref tm) => visit_fn_decl(&tm.decl), + ast::provided(m) => visit_fn_decl(&m.decl), + } + }, + .. *visit::default_simple_visitor() + }) } -fn check_fn(_: ty::ctxt, - fk: &visit::fn_kind, - _: &ast::fn_decl, - _: &ast::blk, - _: span, - id: ast::node_id) { - debug!("lint check_fn fk=%? id=%?", fk, id); +fn lint_session(cx: @mut Context) -> visit::vt<()> { + ast_util::id_visitor(|id| { + match cx.tcx.sess.lints.pop(&id) { + None => {}, + Some(l) => { + do vec::consume(l) |_, (lint, span, msg)| { + cx.span_lint(lint, span, msg) + } + } + } + }) } pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) { - let v = visit::mk_simple_visitor(@visit::SimpleVisitor { - visit_item: |it| - check_item(it, tcx), - visit_fn: |fk, decl, body, span, id| - check_fn(tcx, fk, decl, body, span, id), - .. *visit::default_simple_visitor() - }); - visit::visit_crate(crate, (), v); + let cx = @mut Context { + dict: @get_lint_dict(), + curr: SmallIntMap::new(), + tcx: tcx, + lint_stack: ~[], + visitors: ~[], + }; + + // Install defaults. + for cx.dict.each_value |spec| { + cx.set_level(spec.lint, spec.default, Default); + } + + // Install command-line options, overriding defaults. + for tcx.sess.opts.lint_opts.each |&(lint, level)| { + cx.set_level(lint, level, CommandLine); + } + + // Register each of the lint passes with the context + cx.add_lint(lint_while_true(cx)); + cx.add_lint(lint_path_statement(cx)); + cx.add_lint(lint_heap(cx)); + cx.add_lint(lint_type_limits(cx)); + cx.add_lint(lint_unused_unsafe(cx)); + cx.add_lint(lint_unused_mut(cx)); + cx.add_lint(lint_session(cx)); + + // type inference doesn't like this being declared below, we need to tell it + // what the type of this first function is... + let visit_item: + @fn(@ast::item, @mut Context, visit::vt<@mut Context>) = + |it, cx, vt| { + do cx.with_lint_attrs(it.attrs) { + check_item_ctypes(cx, it); + check_item_non_camel_case_types(cx, it); + check_item_default_methods(cx, it); + check_item_heap(cx, it); + + cx.process(Item(it)); + visit::visit_item(it, cx, vt); + } + }; + + // Actually perform the lint checks (iterating the ast) + do cx.with_lint_attrs(crate.node.attrs) { + cx.process(Crate(crate)); + + visit::visit_crate(crate, cx, visit::mk_vt(@visit::Visitor { + visit_item: visit_item, + visit_fn: |fk, decl, body, span, id, cx, vt| { + match *fk { + visit::fk_method(_, _, m) => { + do cx.with_lint_attrs(m.attrs) { + cx.process(Method(m)); + visit::visit_fn(fk, decl, body, span, id, cx, vt); + } + } + _ => { + visit::visit_fn(fk, decl, body, span, id, cx, vt); + } + } + }, + .. *visit::default_visitor() + })); + } + + // If we missed any lints added to the session, then there's a bug somewhere + // in the iteration code. + for tcx.sess.lints.each |_, v| { + for v.each |t| { + match *t { + (lint, span, ref msg) => + tcx.sess.span_bug(span, fmt!("unprocessed lint %?: %s", + lint, *msg)) + } + } + } tcx.sess.abort_if_errors(); } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 9995de24e8d..711e3915277 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -153,15 +153,13 @@ 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() }); let initial_maps = @mut IrMaps(tcx, method_map, variable_moves_map, - capture_map, - 0); + capture_map); visit::visit_crate(crate, initial_maps, visitor); tcx.sess.abort_if_errors(); } @@ -240,15 +238,12 @@ struct IrMaps { capture_info_map: HashMap, 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, - cur_item: node_id) + capture_map: moves::CaptureMap) -> IrMaps { IrMaps { tcx: tcx, @@ -262,7 +257,6 @@ fn IrMaps(tcx: ty::ctxt, capture_info_map: HashMap::new(), var_kinds: ~[], lnks: ~[], - cur_item: cur_item, } } @@ -341,13 +335,6 @@ pub impl IrMaps { } } -fn visit_item(item: @item, this: @mut IrMaps, v: vt<@mut IrMaps>) { - let old_cur_item = this.cur_item; - this.cur_item = item.id; - visit::visit_item(item, this, v); - this.cur_item = old_cur_item; -} - fn visit_fn(fk: &visit::fn_kind, decl: &fn_decl, body: &blk, @@ -362,8 +349,7 @@ fn visit_fn(fk: &visit::fn_kind, let fn_maps = @mut IrMaps(this.tcx, this.method_map, this.variable_moves_map, - this.capture_map, - this.cur_item); + this.capture_map); unsafe { debug!("creating fn_maps: %x", transmute(&*fn_maps)); @@ -1802,13 +1788,11 @@ pub impl Liveness { }; if is_assigned { - self.tcx.sess.span_lint(unused_variable, id, - self.ir.cur_item, sp, + self.tcx.sess.add_lint(unused_variable, id, sp, fmt!("variable `%s` is assigned to, \ but never used", **name)); } else { - self.tcx.sess.span_lint(unused_variable, id, - self.ir.cur_item, sp, + self.tcx.sess.add_lint(unused_variable, id, sp, fmt!("unused variable: `%s`", **name)); } } @@ -1821,8 +1805,7 @@ pub impl Liveness { ln: LiveNode, var: Variable) { if self.live_on_exit(ln, var).is_none() { for self.should_warn(var).each |name| { - self.tcx.sess.span_lint(dead_assignment, id, - self.ir.cur_item, sp, + self.tcx.sess.add_lint(dead_assignment, id, sp, fmt!("value assigned to `%s` is never read", **name)); } } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index dd5658d7600..a7e590e359c 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -16,47 +16,10 @@ use metadata::csearch::get_type_name_if_impl; use metadata::cstore::find_extern_mod_stmt_cnum; use metadata::decoder::{def_like, dl_def, dl_field, dl_impl}; use middle::lang_items::LanguageItems; -use middle::lint::{allow, level, unused_imports}; -use middle::lint::{get_lint_level, get_lint_settings_level}; +use middle::lint::unused_imports; use middle::pat_util::pat_bindings; -use syntax::ast::{TyParamBound, ty_closure}; -use syntax::ast::{RegionTyParamBound, TraitTyParamBound, _mod, add, arm}; -use syntax::ast::{binding_mode, bitand, bitor, bitxor, blk}; -use syntax::ast::{bind_infer, bind_by_ref, bind_by_copy}; -use syntax::ast::{crate, decl_item, def, def_arg, def_binding}; -use syntax::ast::{def_const, def_foreign_mod, def_fn, def_id, def_label}; -use syntax::ast::{def_local, def_mod, def_prim_ty, def_region, def_self}; -use syntax::ast::{def_self_ty, def_static_method, def_struct, def_ty}; -use syntax::ast::{def_ty_param, def_typaram_binder, def_trait}; -use syntax::ast::{def_upvar, def_use, def_variant, explicit_self_, expr, expr_assign_op}; -use syntax::ast::{expr_binary, expr_break, expr_field}; -use syntax::ast::{expr_fn_block, expr_index, expr_method_call, expr_path}; -use syntax::ast::{def_prim_ty, def_region, def_self, def_ty, def_ty_param}; -use syntax::ast::{def_upvar, def_use, def_variant, div, eq}; -use syntax::ast::{expr, expr_again, expr_assign_op}; -use syntax::ast::{expr_index, expr_loop}; -use syntax::ast::{expr_path, expr_self, expr_struct, expr_unary, fn_decl}; -use syntax::ast::{foreign_item, foreign_item_const, foreign_item_fn, ge}; -use syntax::ast::Generics; -use syntax::ast::{gt, ident, inherited, item, item_struct}; -use syntax::ast::{item_const, item_enum, item_fn, item_foreign_mod}; -use syntax::ast::{item_impl, item_mac, item_mod, item_trait, item_ty, le}; -use syntax::ast::{local, local_crate, lt, method, mul}; -use syntax::ast::{named_field, ne, neg, node_id, pat, pat_enum, pat_ident}; -use syntax::ast::{Path, pat_lit, pat_range, pat_struct}; -use syntax::ast::{prim_ty, private, provided}; -use syntax::ast::{public, required, rem, shl, shr, stmt_decl}; -use syntax::ast::{struct_field, struct_variant_kind}; -use syntax::ast::{sty_static, subtract, trait_ref, tuple_variant_kind, Ty}; -use syntax::ast::{ty_bool, ty_char, ty_f, ty_f32, ty_f64, ty_float, ty_i}; -use syntax::ast::{ty_i16, ty_i32, ty_i64, ty_i8, ty_int, TyParam, ty_path}; -use syntax::ast::{ty_str, ty_u, ty_u16, ty_u32, ty_u64, ty_u8, ty_uint}; -use syntax::ast::unnamed_field; -use syntax::ast::{variant, view_item, view_item_extern_mod}; -use syntax::ast::{view_item_use, view_path_glob, view_path_list}; -use syntax::ast::{view_path_simple, anonymous, named, not}; -use syntax::ast::{unsafe_fn}; +use syntax::ast::*; use syntax::ast_util::{def_id_of_def, local_def}; use syntax::ast_util::{path_to_ident, walk_pat, trait_method_to_ty_method}; use syntax::ast_util::{Privacy, Public, Private}; @@ -66,6 +29,7 @@ use syntax::parse::token::ident_interner; use syntax::parse::token::special_idents; use syntax::print::pprust::path_to_str; use syntax::codemap::{span, dummy_sp, BytePos}; +use syntax::visit::{mk_simple_visitor, default_simple_visitor, SimpleVisitor}; use syntax::visit::{default_visitor, mk_vt, Visitor, visit_block}; use syntax::visit::{visit_crate, visit_expr, visit_expr_opt}; use syntax::visit::{visit_foreign_item, visit_item}; @@ -346,18 +310,21 @@ pub struct ImportDirective { module_path: ~[ident], subclass: @ImportDirectiveSubclass, span: span, + id: node_id, } pub fn ImportDirective(privacy: Privacy, module_path: ~[ident], subclass: @ImportDirectiveSubclass, - span: span) + span: span, + id: node_id) -> ImportDirective { ImportDirective { privacy: privacy, module_path: module_path, subclass: subclass, - span: span + span: span, + id: id } } @@ -381,7 +348,7 @@ pub struct ImportResolution { /// The privacy of this `use` directive (whether it's `use` or /// `pub use`. privacy: Privacy, - span: span, + id: node_id, // The number of outstanding references to this name. When this reaches // zero, outside modules can count on the targets being correct. Before @@ -393,21 +360,16 @@ pub struct ImportResolution { value_target: Option, /// The type that this `use` directive names, if there is one. type_target: Option, - - /// There exists one state per import statement - state: @mut ImportState, } pub fn ImportResolution(privacy: Privacy, - span: span, - state: @mut ImportState) -> ImportResolution { + id: node_id) -> ImportResolution { ImportResolution { privacy: privacy, - span: span, + id: id, outstanding_references: 0, value_target: None, type_target: None, - state: state, } } @@ -420,15 +382,6 @@ pub impl ImportResolution { } } -pub struct ImportState { - used: bool, - warned: bool -} - -pub fn ImportState() -> ImportState { - ImportState{ used: false, warned: false } -} - /// The link from a module up to its nearest parent node. pub enum ParentLink { NoParentLink, @@ -805,6 +758,7 @@ pub fn Resolver(session: Session, def_map: @mut HashMap::new(), export_map2: @mut HashMap::new(), trait_map: HashMap::new(), + used_imports: HashSet::new(), intr: session.intr() }; @@ -862,6 +816,8 @@ pub struct Resolver { def_map: DefMap, export_map2: ExportMap2, trait_map: TraitMap, + + used_imports: HashSet, } pub impl Resolver { @@ -879,7 +835,7 @@ pub impl Resolver { self.resolve_crate(); self.session.abort_if_errors(); - self.check_for_unused_imports_if_necessary(); + self.check_for_unused_imports(); } // @@ -1424,7 +1380,7 @@ pub impl Resolver { // Build up the import directives. let module_ = self.get_module_from_parent(parent); match view_path.node { - view_path_simple(binding, full_path, _) => { + view_path_simple(binding, full_path, id) => { let source_ident = *full_path.idents.last(); let subclass = @SingleImport(binding, source_ident); @@ -1432,7 +1388,8 @@ pub impl Resolver { module_, module_path, subclass, - view_path.span); + view_path.span, + id); } view_path_list(_, ref source_idents, _) => { for source_idents.each |source_ident| { @@ -1442,15 +1399,17 @@ pub impl Resolver { module_, copy module_path, subclass, - source_ident.span); + source_ident.span, + source_ident.node.id); } } - view_path_glob(_, _) => { + view_path_glob(_, id) => { self.build_import_directive(privacy, module_, module_path, @GlobImport, - view_path.span); + view_path.span, + id); } } } @@ -1575,10 +1534,7 @@ pub impl Resolver { // avoid creating cycles in the // module graph. - let resolution = - @mut ImportResolution(Public, - dummy_sp(), - @mut ImportState()); + let resolution = @mut ImportResolution(Public, 0); resolution.outstanding_references = 0; match existing_module.parent_link { @@ -1840,9 +1796,10 @@ pub impl Resolver { module_: @mut Module, module_path: ~[ident], subclass: @ImportDirectiveSubclass, - span: span) { + span: span, + id: node_id) { let directive = @ImportDirective(privacy, module_path, - subclass, span); + subclass, span, id); module_.imports.push(directive); // Bump the reference count on the name. Or, if this is a glob, set @@ -1864,16 +1821,7 @@ pub impl Resolver { } None => { debug!("(building import directive) creating new"); - let state = @mut ImportState(); - let resolution = @mut ImportResolution(privacy, - span, - state); - let name = self.idents_to_str(directive.module_path); - // Don't warn about unused intrinsics because they're - // automatically appended to all files - if name == ~"intrinsic::rusti" { - resolution.state.warned = true; - } + let resolution = @mut ImportResolution(privacy, id); resolution.outstanding_references = 1; module_.import_resolutions.insert(target, resolution); } @@ -2069,13 +2017,12 @@ pub impl Resolver { import_directive.span); } GlobImport => { - let span = import_directive.span; let privacy = import_directive.privacy; resolution_result = self.resolve_glob_import(privacy, module_, containing_module, - span); + import_directive.id); } } } @@ -2202,7 +2149,8 @@ pub impl Resolver { if import_resolution.outstanding_references == 0 => { - fn get_binding(import_resolution: + fn get_binding(this: @mut Resolver, + import_resolution: @mut ImportResolution, namespace: Namespace) -> NamespaceResult { @@ -2219,7 +2167,7 @@ pub impl Resolver { return UnboundResult; } Some(target) => { - import_resolution.state.used = true; + this.used_imports.insert(import_resolution.id); return BoundResult(target.target_module, target.bindings); } @@ -2229,11 +2177,11 @@ pub impl Resolver { // The name is an import which has been fully // resolved. We can, therefore, just follow it. if value_result.is_unknown() { - value_result = get_binding(*import_resolution, + value_result = get_binding(self, *import_resolution, ValueNS); } if type_result.is_unknown() { - type_result = get_binding(*import_resolution, + type_result = get_binding(self, *import_resolution, TypeNS); } } @@ -2358,13 +2306,12 @@ pub impl Resolver { privacy: Privacy, module_: @mut Module, containing_module: @mut Module, - span: span) + id: node_id) -> ResolveResult<()> { // This function works in a highly imperative manner; it eagerly adds // everything it can to the list of import resolutions of the module // node. debug!("(resolving glob import) resolving %? glob import", privacy); - let state = @mut ImportState(); // We must bail out if the node has unresolved imports of any kind // (including globs). @@ -2390,9 +2337,7 @@ pub impl Resolver { None if target_import_resolution.privacy == Public => { // Simple: just copy the old import resolution. let new_import_resolution = - @mut ImportResolution(privacy, - target_import_resolution.span, - state); + @mut ImportResolution(privacy, id); new_import_resolution.value_target = copy target_import_resolution.value_target; new_import_resolution.type_target = @@ -2434,9 +2379,7 @@ pub impl Resolver { match module_.import_resolutions.find(&ident) { None => { // Create a new import resolution from this child. - dest_import_resolution = @mut ImportResolution(privacy, - span, - state); + dest_import_resolution = @mut ImportResolution(privacy, id); module_.import_resolutions.insert (ident, dest_import_resolution); } @@ -2714,7 +2657,7 @@ pub impl Resolver { Some(target) => { debug!("(resolving item in lexical scope) using \ import resolution"); - import_resolution.state.used = true; + self.used_imports.insert(import_resolution.id); return Success(copy target); } } @@ -2985,7 +2928,7 @@ pub impl Resolver { import_resolution.privacy == Public => { debug!("(resolving name in module) resolved to \ import"); - import_resolution.state.used = true; + self.used_imports.insert(import_resolution.id); return Success(copy target); } Some(_) => { @@ -4466,7 +4409,7 @@ pub impl Resolver { namespace)) { (Some(def), Some(Public)) => { // Found it. - import_resolution.state.used = true; + self.used_imports.insert(import_resolution.id); return ImportNameDefinition(def); } (Some(_), _) | (None, _) => { @@ -5046,8 +4989,8 @@ pub impl Resolver { &mut found_traits, trait_def_id, name); if added { - import_resolution.state.used = - true; + self.used_imports.insert( + import_resolution.id); } } _ => { @@ -5145,86 +5088,50 @@ pub impl Resolver { // resolve data structures. // - fn unused_import_lint_level(@mut self, m: @mut Module) -> level { - let settings = self.session.lint_settings; - match m.def_id { - Some(def) => get_lint_settings_level(settings, unused_imports, - def.node, def.node), - None => get_lint_level(settings.default_settings, unused_imports) - } + fn check_for_unused_imports(@mut self) { + let vt = mk_simple_visitor(@SimpleVisitor { + visit_view_item: |vi| self.check_for_item_unused_imports(vi), + .. *default_simple_visitor() + }); + visit_crate(self.crate, (), vt); } - fn check_for_unused_imports_if_necessary(@mut self) { - if self.unused_import_lint_level(self.current_module) == allow { - return; - } + fn check_for_item_unused_imports(&mut self, vi: @view_item) { + // Ignore public import statements because there's no way to be sure + // whether they're used or not. Also ignore imports with a dummy span + // because this means that they were generated in some fashion by the + // compiler and we don't need to consider them. + if vi.vis == public { return } + if vi.span == dummy_sp() { return } - let root_module = self.graph_root.get_module(); - self.check_for_unused_imports_in_module_subtree(root_module); - } + match vi.node { + view_item_extern_mod(*) => {} // ignore + view_item_use(ref path) => { + for path.each |p| { + match p.node { + view_path_simple(_, _, id) | view_path_glob(_, id) => { + if !self.used_imports.contains(&id) { + self.session.add_lint(unused_imports, + id, vi.span, + ~"unused import"); + } + } - fn check_for_unused_imports_in_module_subtree(@mut self, - module_: @mut Module) { - // If this isn't a local crate, then bail out. We don't need to check - // for unused imports in external crates. - - match module_.def_id { - Some(def_id) if def_id.crate == local_crate => { - // OK. Continue. - } - None => { - // Check for unused imports in the root module. - } - Some(_) => { - // Bail out. - debug!("(checking for unused imports in module subtree) not \ - checking for unused imports for `%s`", - self.module_to_str(module_)); - return; - } - } - - self.check_for_unused_imports_in_module(module_); - - for module_.children.each_value |&child_name_bindings| { - match (*child_name_bindings).get_module_if_available() { - None => { - // Nothing to do. - } - Some(child_module) => { - self.check_for_unused_imports_in_module_subtree - (child_module); + view_path_list(_, ref list, _) => { + for list.each |i| { + if !self.used_imports.contains(&i.node.id) { + self.session.add_lint(unused_imports, + i.node.id, i.span, + ~"unused import"); + } + } + } + } } } } - - for module_.anonymous_children.each_value |&child_module| { - self.check_for_unused_imports_in_module_subtree(child_module); - } } - fn check_for_unused_imports_in_module(@mut self, module_: @mut Module) { - for module_.import_resolutions.each_value |&import_resolution| { - // Ignore dummy spans for things like automatically injected - // imports for the prelude, and also don't warn about the same - // import statement being unused more than once. Furthermore, if - // the import is public, then we can't be sure whether it's unused - // or not so don't warn about it. - if !import_resolution.state.used && - !import_resolution.state.warned && - import_resolution.span != dummy_sp() && - import_resolution.privacy != Public { - import_resolution.state.warned = true; - let span = import_resolution.span; - self.session.span_lint_level( - self.unused_import_lint_level(module_), - span, - ~"unused import"); - } - } - } - - // // Diagnostics // diff --git a/src/librustc/middle/resolve_stage0.rs b/src/librustc/middle/resolve_stage0.rs index a404dcf7249..713132b12fc 100644 --- a/src/librustc/middle/resolve_stage0.rs +++ b/src/librustc/middle/resolve_stage0.rs @@ -17,8 +17,7 @@ use metadata::csearch::get_type_name_if_impl; use metadata::cstore::find_extern_mod_stmt_cnum; use metadata::decoder::{def_like, dl_def, dl_field, dl_impl}; use middle::lang_items::LanguageItems; -use middle::lint::{allow, level, unused_imports}; -use middle::lint::{get_lint_level, get_lint_settings_level}; +use middle::lint::{allow, level, warn}; use middle::pat_util::pat_bindings; use syntax::ast::{RegionTyParamBound, TraitTyParamBound, _mod, add, arm}; @@ -5168,14 +5167,7 @@ pub impl Resolver { // resolve data structures. // - fn unused_import_lint_level(@mut self, m: @mut Module) -> level { - let settings = self.session.lint_settings; - match m.def_id { - Some(def) => get_lint_settings_level(settings, unused_imports, - def.node, def.node), - None => get_lint_level(settings.default_settings, unused_imports) - } - } + fn unused_import_lint_level(@mut self, _: @mut Module) -> level { warn } fn check_for_unused_imports_if_necessary(@mut self) { if self.unused_import_lint_level(self.current_module) == allow { @@ -5237,12 +5229,7 @@ pub impl Resolver { !import_resolution.state.warned && import_resolution.span != dummy_sp() && import_resolution.privacy != Public { - import_resolution.state.warned = true; - let span = import_resolution.span; - self.session.span_lint_level( - self.unused_import_lint_level(module_), - span, - ~"unused import"); + // I swear I work in not(stage0)! } } } diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index aa1ee7cd27e..d4a67d61d94 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -412,7 +412,12 @@ pub fn id_visitor(vfn: @fn(node_id)) -> visit::vt<()> { match vp.node { view_path_simple(_, _, id) => vfn(id), view_path_glob(_, id) => vfn(id), - view_path_list(_, _, id) => vfn(id) + view_path_list(_, ref paths, id) => { + vfn(id); + for paths.each |p| { + vfn(p.node.id); + } + } } } } diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 01b37a1196c..624e0495e59 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -196,6 +196,7 @@ pub fn mk_global_struct_e(cx: @ext_ctxt, } pub fn mk_glob_use(cx: @ext_ctxt, sp: span, + vis: ast::visibility, path: ~[ast::ident]) -> @ast::view_item { let glob = @codemap::spanned { node: ast::view_path_glob(mk_raw_path(sp, path), cx.next_id()), @@ -203,7 +204,7 @@ pub fn mk_glob_use(cx: @ext_ctxt, }; @ast::view_item { node: ast::view_item_use(~[glob]), attrs: ~[], - vis: ast::private, + vis: vis, span: sp } } pub fn mk_local(cx: @ext_ctxt, sp: span, mutbl: bool, diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs index f4227cd2f2c..fc673c4422f 100644 --- a/src/libsyntax/ext/quote.rs +++ b/src/libsyntax/ext/quote.rs @@ -41,7 +41,6 @@ pub mod rt { pub use parse::new_parser_from_tts; pub use codemap::{BytePos, span, dummy_spanned}; - use print::pprust; use print::pprust::{item_to_str, ty_to_str}; pub trait ToTokens { @@ -678,10 +677,11 @@ fn expand_tts(cx: @ext_ctxt, // We want to emit a block expression that does a sequence of 'use's to // import the runtime module, followed by a tt-building expression. - let uses = ~[ build::mk_glob_use(cx, sp, ids_ext(cx, ~[~"syntax", - ~"ext", - ~"quote", - ~"rt"])) ]; + let uses = ~[ build::mk_glob_use(cx, sp, ast::public, + ids_ext(cx, ~[~"syntax", + ~"ext", + ~"quote", + ~"rt"])) ]; // We also bind a single value, sp, to ext_cx.call_site() // diff --git a/src/test/compile-fail/lint-impl-fn.rs b/src/test/compile-fail/lint-impl-fn.rs new file mode 100644 index 00000000000..3cc0495206d --- /dev/null +++ b/src/test/compile-fail/lint-impl-fn.rs @@ -0,0 +1,37 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[allow(while_true)]; + +struct A(int); + +impl A { + fn foo(&self) { while true {} } + + #[deny(while_true)] + fn bar(&self) { while true {} } //~ ERROR: infinite loops +} + +#[deny(while_true)] +mod foo { + struct B(int); + + impl B { + fn foo(&self) { while true {} } //~ ERROR: infinite loops + + #[allow(while_true)] + fn bar(&self) { while true {} } + } +} + +#[deny(while_true)] +fn main() { + while true {} //~ ERROR: infinite loops +} diff --git a/src/test/compile-fail/unused-imports-warn.rs b/src/test/compile-fail/lint-unused-imports.rs similarity index 100% rename from src/test/compile-fail/unused-imports-warn.rs rename to src/test/compile-fail/lint-unused-imports.rs diff --git a/src/test/compile-fail/unused-mut-variables.rs b/src/test/compile-fail/lint-unused-mut-variables.rs similarity index 100% rename from src/test/compile-fail/unused-mut-variables.rs rename to src/test/compile-fail/lint-unused-mut-variables.rs diff --git a/src/test/compile-fail/unused-unsafe.rs b/src/test/compile-fail/lint-unused-unsafe.rs similarity index 100% rename from src/test/compile-fail/unused-unsafe.rs rename to src/test/compile-fail/lint-unused-unsafe.rs diff --git a/src/test/compile-fail/liveness-dead.rs b/src/test/compile-fail/liveness-dead.rs index 1d6a7426045..2ab3cb4568a 100644 --- a/src/test/compile-fail/liveness-dead.rs +++ b/src/test/compile-fail/liveness-dead.rs @@ -8,12 +8,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[deny(dead_assignment)]; + fn f1(x: &mut int) { *x = 1; // no error } fn f2() { - let mut x = 3; //~ WARNING value assigned to `x` is never read + let mut x = 3; //~ ERROR: value assigned to `x` is never read x = 4; copy x; } @@ -21,10 +23,7 @@ fn f2() { fn f3() { let mut x = 3; copy x; - x = 4; //~ WARNING value assigned to `x` is never read + x = 4; //~ ERROR: value assigned to `x` is never read } -fn main() { // leave this in here just to trigger compile-fail: - let x: int; - copy x; //~ ERROR use of possibly uninitialized variable: `x` -} +fn main() {}