From 030c666cc1bc4830eac2f845b114f7c514f6e201 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 30 Apr 2013 01:15:17 -0400
Subject: [PATCH 1/3] Re-implement lint with less emphasis on item ids
This way it's much easier to add lints throughout compilation correctly, and
functions on impls can alter the way lints are emitted.
---
src/libcore/hashmap.rs | 1 -
src/librustc/back/link.rs | 1 -
src/librustc/driver/driver.rs | 7 +-
src/librustc/driver/session.rs | 19 +-
src/librustc/middle/lint.rs | 728 +++++++++---------
src/librustc/middle/liveness.rs | 29 +-
src/librustc/middle/resolve.rs | 247 ++----
src/librustc/middle/resolve_stage0.rs | 12 +-
src/libsyntax/ast_util.rs | 7 +-
src/libsyntax/ext/build.rs | 3 +-
src/libsyntax/ext/quote.rs | 10 +-
src/test/compile-fail/lint-impl-fn.rs | 37 +
...imports-warn.rs => lint-unused-imports.rs} | 0
...iables.rs => lint-unused-mut-variables.rs} | 0
...unused-unsafe.rs => lint-unused-unsafe.rs} | 0
src/test/compile-fail/liveness-dead.rs | 11 +-
16 files changed, 507 insertions(+), 605 deletions(-)
create mode 100644 src/test/compile-fail/lint-impl-fn.rs
rename src/test/compile-fail/{unused-imports-warn.rs => lint-unused-imports.rs} (100%)
rename src/test/compile-fail/{unused-mut-variables.rs => lint-unused-mut-variables.rs} (100%)
rename src/test/compile-fail/{unused-unsafe.rs => lint-unused-unsafe.rs} (100%)
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..790c24369df 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<ast::node_id, ~[(lint::lint, codemap::span, ~str)]>,
}
pub type Session = @Session_;
@@ -230,15 +232,12 @@ pub impl Session_ {
}
}
}
- 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);
+ 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 => {}
+ }
+ 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..5c8ea75440c 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_
@@ -86,7 +85,13 @@ 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),
+}
static lint_table: &'static [(&'static str, LintSpec)] = &[
("ctypes",
@@ -225,7 +230,7 @@ pub fn get_lint_dict() -> LintDict {
for lint_table.each|&(k, v)| {
map.insert(k.to_str(), v);
}
- return @map;
+ return map;
}
pub fn get_lint_name(lint_mode: lint) -> ~str {
@@ -237,61 +242,31 @@ pub fn get_lint_name(lint_mode: lint) -> ~str {
fail!();
}
// This is a highly not-optimal set of data structure decisions.
-type LintModes = @mut SmallIntMap<level>;
-type LintModeMap = @mut HashMap<ast::node_id, LintModes>;
-
-// 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)
-}
+type LintModes = SmallIntMap<level>;
+type LintModeMap = HashMap<ast::node_id, LintModes>;
struct Context {
- dict: LintDict,
+ // All known lint modes (string versions)
+ dict: @LintDict,
+ // Current levels of each lint warning
curr: LintModes,
- is_default: bool,
- sess: Session
+ // 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)],
}
-pub impl Context {
+impl Context {
fn get_level(&self, lint: lint) -> level {
- get_lint_level(self.curr, lint)
+ match self.curr.find(&(lint as uint)) {
+ Some(&c) => c,
+ None => allow
+ }
}
- fn set_level(&self, lint: lint, level: level) {
+ fn set_level(&mut self, lint: lint, level: level) {
if level == allow {
self.curr.remove(&(lint as uint));
} else {
@@ -299,8 +274,8 @@ pub impl Context {
}
}
- fn span_lint(&self, level: level, span: span, msg: ~str) {
- self.sess.span_lint_level(level, span, msg);
+ fn span_lint(&self, lint: lint, span: span, msg: &str) {
+ self.tcx.sess.span_lint_level(self.get_level(lint), span, msg);
}
/**
@@ -308,41 +283,13 @@ 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");
- }
- }
- }
- }
-
+ 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 triples = extract_lints(self.tcx.sess, attrs);
+ let mut pushed = 0u;
for triples.each |triple| {
// FIXME(#3874): it would be nicer to write this...
// let (meta, level, lintname) = /*bad*/copy *pair;
@@ -350,147 +297,144 @@ pub impl Context {
(ref meta, level, lintname) => (meta, level, lintname)
};
- match self.dict.find(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
+ }
+
+ self.lint_stack.push((lint, now));
+ pushed += 1;
+ self.set_level(lint, level);
+ }
+
+ f();
+
+ // rollback
+ for pushed.times {
+ let (lint, level) = self.lint_stack.pop();
+ self.set_level(lint, level);
+ }
+ }
+
+ fn process(&self, n: AttributedNode, v: @visit::SimpleVisitor) {
+ self.process_visitor(n, visit::mk_simple_visitor(v));
+ }
+
+ fn process_visitor(&self, n: AttributedNode, v: visit::vt<()>) {
+ let v = item_stopping_visitor(v);
+ match n {
+ Item(it) => visit::visit_item(it, (), v),
+ Crate(c) => 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) => visit::visit_fn(&visit::fk_method(copy m.ident,
+ &m.generics,
+ m),
+ &m.decl,
+ &m.body,
+ m.span,
+ m.id,
+ (),
+ v)
+ }
+ }
+}
+
+pub fn extract_lints(sess: session::Session,
+ attrs: &[ast::attribute])
+ -> ~[(@ast::meta_item, level, @~str)]
+{
+ 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(lintname) => {
+ triples.push((*meta,
+ level,
+ lintname));
+ }
+ _ => {
+ sess.span_err(meta.span, ~"malformed lint attribute");
+ }
+ }
}
-
- // 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);
+ }
+ _ => {
+ sess.span_err(meta.span, ~"malformed lint attribute");
}
}
}
- f(new_ctxt);
}
-}
-
-
-fn build_settings_item(i: @ast::item, cx: Context, v: visit::vt<Context>) {
- 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);
- }
- 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);
- }
-
- 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);
- }
-
- 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);
+ return triples;
}
// 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<E>(v: visit::vt<E>) -> visit::vt<E> {
- visit::mk_vt(@visit::Visitor {visit_item: |_i, _e, _v| { },
- .. **(ty_stopping_visitor(v))})
+fn item_stopping_visitor<E: Copy>(v: visit::vt<E>) -> visit::vt<E> {
+ 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<E>(v: visit::vt<E>) -> visit::vt<E> {
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 check_item_while_true(cx: @mut Context, n: AttributedNode) {
+ cx.process(n, @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 check_item_type_limits(cx: @mut Context, n: AttributedNode) {
fn is_valid<T:cmp::Ord>(binop: ast::binop, v: T,
min: T, max: T) -> bool {
match binop {
@@ -534,7 +478,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 +487,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 +536,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);
+ cx.process(n, @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 +567,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 +598,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,106 +609,96 @@ fn check_item_ctypes(cx: ty::ctxt, it: @ast::item) {
}
}
-fn check_item_heap(cx: ty::ctxt, it: @ast::item) {
+fn check_item_heap(cx: @mut Context, n: AttributedNode) {
- fn check_type_for_lint(cx: ty::ctxt, lint: lint,
- node: ast::node_id,
- item: ast::node_id,
- span: span, ty: ty::t) {
+ fn check_type_for_lint(cx: @mut Context, lint: lint, span: span, ty: ty::t) {
+ if cx.get_level(lint) == allow { return }
- 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
- });
+ 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 (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_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);
+ }
- 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_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: 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);
+ 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);
+ }
+ }
+
+ match n {
+ Item(it) => {
+ match it.node {
+ ast::item_fn(*) |
+ ast::item_ty(*) |
+ ast::item_enum(*) |
+ ast::item_struct(*) => check_type(cx, it.span,
+ ty::node_id_to_type(cx.tcx,
+ it.id)),
+ _ => ()
}
- }
- 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)),
- _ => ()
- }
-
- // If it's a struct, we also have to check the fields' types
- 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));
+ // If it's a struct, we also have to check the fields' types
+ match it.node {
+ ast::item_struct(struct_def, _) => {
+ for struct_def.fields.each |struct_field| {
+ 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);
+ cx.process(n, @visit::SimpleVisitor {
+ visit_expr: |e| {
+ let ty = ty::expr_ty(cx.tcx, e);
+ check_type(cx, e.span, ty);
+ },
+ .. *visit::default_simple_visitor()
+ });
}
-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 check_item_path_statement(cx: @mut Context, n: AttributedNode) {
+ cx.process(n, @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 +722,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 check_item_unused_unsafe(cx: @mut Context, n: AttributedNode) {
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);
+ cx.process(n, @visit::SimpleVisitor {
+ visit_expr: visit_expr,
+ .. *visit::default_simple_visitor()
+ });
}
-fn check_item_unused_mut(tcx: ty::ctxt, it: @ast::item) {
+fn check_item_unused_mut(cx: @mut Context, n: AttributedNode) {
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 +778,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 +790,113 @@ 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);
+ cx.process(n, @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 check_item_session_lints(cx: @mut Context, n: AttributedNode) {
+ cx.process_visitor(n, ast_util::id_visitor(|id| {
+ match cx.tcx.sess.lints.pop(&id) {
+ None => {},
+ Some(l) => {
+ info!("id %?", id);
+ do vec::consume(l) |_, (lint, span, msg)| {
+ cx.span_lint(lint, span, msg)
+ }
+ }
+ }
+ }));
+}
+
+fn check_attributed_node(cx: @mut Context, n: AttributedNode) {
+ check_item_while_true(cx, n);
+ check_item_path_statement(cx, n);
+ check_item_heap(cx, n);
+ check_item_type_limits(cx, n);
+ check_item_unused_unsafe(cx, n);
+ check_item_unused_mut(cx, n);
+ check_item_session_lints(cx, n);
}
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: ~[],
+ };
+ // Install defaults.
+ for cx.dict.each_value |spec| {
+ cx.set_level(spec.lint, spec.default);
+ }
+
+ // Install command-line options, overriding defaults.
+ for tcx.sess.opts.lint_opts.each |&(lint, level)| {
+ cx.set_level(lint, level);
+ }
+
+ // 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);
+
+ check_attributed_node(cx, Item(it));
+ visit::visit_item(it, cx, vt);
+ }
+ };
+
+ do cx.with_lint_attrs(crate.node.attrs) {
+ check_item_session_lints(cx, 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) {
+ check_attributed_node(cx, 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()
+ }));
+ }
+
+ 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<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,
- 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<Target>,
/// The type that this `use` directive names, if there is one.
type_target: Option<Target>,
-
- /// 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<node_id>,
}
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..0c3ec9797da 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 {
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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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() {}
From 1daaf785ab26f801d5003ad057f590d0886a5203 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 7 May 2013 02:07:00 -0400
Subject: [PATCH 2/3] Optimize lint passes to perform far fewer allocations
Achieves ~3x speedup on lint passes for libcore
---
src/librustc/middle/lint.rs | 336 ++++++++++++++++++++----------------
1 file changed, 190 insertions(+), 146 deletions(-)
diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs
index 5c8ea75440c..2aa9a8c7001 100644
--- a/src/librustc/middle/lint.rs
+++ b/src/librustc/middle/lint.rs
@@ -27,18 +27,25 @@ use syntax::{ast, visit, ast_util};
* 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 for the particular attribute 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 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. Once the lint state has been altered, all of the known
+ * lint passes are run on the node of the ast.
+ *
+ * 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.
*/
#[deriving(Eq)]
@@ -256,6 +263,12 @@ struct Context {
// levels, this stack keeps track of the previous lint levels of whatever
// was modified.
lint_stack: ~[(lint, level)],
+ // 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<()>],
}
impl Context {
@@ -288,15 +301,8 @@ impl Context {
// 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 triples = extract_lints(self.tcx.sess, attrs);
let mut pushed = 0u;
- 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)
- };
-
+ for each_lint(self.tcx.sess, attrs) |meta, level, lintname| {
let lint = match self.dict.find(lintname) {
None => {
self.span_lint(
@@ -315,12 +321,14 @@ impl Context {
fmt!("%s(%s) overruled by outer forbid(%s)",
level_to_str(level),
*lintname, *lintname));
- loop
+ loop;
}
- self.lint_stack.push((lint, now));
- pushed += 1;
- self.set_level(lint, level);
+ if now != level {
+ self.lint_stack.push((lint, now));
+ pushed += 1;
+ self.set_level(lint, level);
+ }
}
f();
@@ -332,63 +340,100 @@ impl Context {
}
}
- fn process(&self, n: AttributedNode, v: @visit::SimpleVisitor) {
- self.process_visitor(n, visit::mk_simple_visitor(v));
+ fn add_lint(&mut self, v: visit::vt<()>) {
+ self.visitors.push(item_stopping_visitor(v));
}
- fn process_visitor(&self, n: AttributedNode, v: visit::vt<()>) {
- let v = item_stopping_visitor(v);
+ fn process(&self, n: AttributedNode) {
match n {
- Item(it) => visit::visit_item(it, (), v),
- Crate(c) => visit::visit_crate(c, (), v),
+ 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) => visit::visit_fn(&visit::fk_method(copy m.ident,
- &m.generics,
- m),
- &m.decl,
- &m.body,
- m.span,
- m.id,
- (),
- v)
+ 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);
+ }
+ }
}
}
}
-pub fn extract_lints(sess: session::Session,
- attrs: &[ast::attribute])
- -> ~[(@ast::meta_item, level, @~str)]
+#[cfg(stage0)]
+pub fn each_lint(sess: session::Session,
+ attrs: &[ast::attribute],
+ f: &fn(@ast::meta_item, level, &~str) -> bool)
{
- 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(lintname) => {
- triples.push((*meta,
- level,
- lintname));
- }
- _ => {
+ 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");
- }
}
}
- }
- _ => {
- sess.span_err(meta.span, ~"malformed lint attribute");
- }
}
}
}
- return triples;
+}
+#[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.
@@ -411,8 +456,8 @@ fn ty_stopping_visitor<E>(v: visit::vt<E>) -> visit::vt<E> {
visit::mk_vt(@visit::Visitor {visit_ty: |_t, _e, _v| { },.. **v})
}
-fn check_item_while_true(cx: @mut Context, n: AttributedNode) {
- cx.process(n, @visit::SimpleVisitor {
+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, _) => {
@@ -431,10 +476,10 @@ fn check_item_while_true(cx: @mut Context, n: AttributedNode) {
}
},
.. *visit::default_simple_visitor()
- });
+ })
}
-fn check_item_type_limits(cx: @mut Context, n: AttributedNode) {
+fn lint_type_limits(cx: @mut Context) -> visit::vt<()> {
fn is_valid<T:cmp::Ord>(binop: ast::binop, v: T,
min: T, max: T) -> bool {
match binop {
@@ -544,10 +589,10 @@ fn check_item_type_limits(cx: @mut Context, n: AttributedNode) {
}
};
- cx.process(n, @visit::SimpleVisitor {
+ visit::mk_simple_visitor(@visit::SimpleVisitor {
visit_expr: visit_expr,
.. *visit::default_simple_visitor()
- });
+ })
}
fn check_item_default_methods(cx: @mut Context, item: @ast::item) {
@@ -609,79 +654,75 @@ fn check_item_ctypes(cx: @mut Context, it: @ast::item) {
}
}
-fn check_item_heap(cx: @mut Context, n: AttributedNode) {
+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: @mut Context, lint: lint, span: span, ty: ty::t) {
- if cx.get_level(lint) == allow { return }
+ 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
+ });
- 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 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);
- }
-
- 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);
- }
+ 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: @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);
- }
+ 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.span,
+ ty::node_id_to_type(cx.tcx,
+ it.id)),
+ _ => ()
}
- match n {
- Item(it) => {
- match it.node {
- ast::item_fn(*) |
- ast::item_ty(*) |
- ast::item_enum(*) |
- ast::item_struct(*) => check_type(cx, it.span,
- ty::node_id_to_type(cx.tcx,
- it.id)),
- _ => ()
- }
-
- // If it's a struct, we also have to check the fields' types
- match it.node {
- ast::item_struct(struct_def, _) => {
- for struct_def.fields.each |struct_field| {
- check_type(cx, struct_field.span,
- ty::node_id_to_type(cx.tcx,
- struct_field.node.id));
- }
- }
- _ => ()
+ // If it's a struct, we also have to check the fields' types
+ match it.node {
+ ast::item_struct(struct_def, _) => {
+ for struct_def.fields.each |struct_field| {
+ check_type(cx, struct_field.span,
+ ty::node_id_to_type(cx.tcx,
+ struct_field.node.id));
}
}
_ => ()
}
+}
- cx.process(n, @visit::SimpleVisitor {
+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 check_item_path_statement(cx: @mut Context, n: AttributedNode) {
- cx.process(n, @visit::SimpleVisitor {
+fn lint_path_statement(cx: @mut Context) -> visit::vt<()> {
+ visit::mk_simple_visitor(@visit::SimpleVisitor {
visit_stmt: |s| {
match s.node {
ast::stmt_semi(
@@ -695,7 +736,7 @@ fn check_item_path_statement(cx: @mut Context, n: AttributedNode) {
}
},
.. *visit::default_simple_visitor()
- });
+ })
}
fn check_item_non_camel_case_types(cx: @mut Context, it: @ast::item) {
@@ -745,7 +786,7 @@ fn check_item_non_camel_case_types(cx: @mut Context, it: @ast::item) {
}
}
-fn check_item_unused_unsafe(cx: @mut Context, n: AttributedNode) {
+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 => {
@@ -758,13 +799,13 @@ fn check_item_unused_unsafe(cx: @mut Context, n: AttributedNode) {
}
};
- cx.process(n, @visit::SimpleVisitor {
+ visit::mk_simple_visitor(@visit::SimpleVisitor {
visit_expr: visit_expr,
.. *visit::default_simple_visitor()
- });
+ })
}
-fn check_item_unused_mut(cx: @mut Context, n: AttributedNode) {
+fn lint_unused_mut(cx: @mut Context) -> visit::vt<()> {
let check_pat: @fn(@ast::pat) = |p| {
let mut used = false;
let mut bindings = 0;
@@ -790,7 +831,7 @@ fn check_item_unused_mut(cx: @mut Context, n: AttributedNode) {
}
};
- cx.process(n, @visit::SimpleVisitor {
+ visit::mk_simple_visitor(@visit::SimpleVisitor {
visit_local: |l| {
if l.node.is_mutbl {
check_pat(l.node.pat);
@@ -806,31 +847,20 @@ fn check_item_unused_mut(cx: @mut Context, n: AttributedNode) {
}
},
.. *visit::default_simple_visitor()
- });
+ })
}
-fn check_item_session_lints(cx: @mut Context, n: AttributedNode) {
- cx.process_visitor(n, ast_util::id_visitor(|id| {
+fn lint_session(cx: @mut Context) -> visit::vt<()> {
+ ast_util::id_visitor(|id| {
match cx.tcx.sess.lints.pop(&id) {
None => {},
Some(l) => {
- info!("id %?", id);
do vec::consume(l) |_, (lint, span, msg)| {
cx.span_lint(lint, span, msg)
}
}
}
- }));
-}
-
-fn check_attributed_node(cx: @mut Context, n: AttributedNode) {
- check_item_while_true(cx, n);
- check_item_path_statement(cx, n);
- check_item_heap(cx, n);
- check_item_type_limits(cx, n);
- check_item_unused_unsafe(cx, n);
- check_item_unused_mut(cx, n);
- check_item_session_lints(cx, n);
+ })
}
pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
@@ -839,6 +869,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
curr: SmallIntMap::new(),
tcx: tcx,
lint_stack: ~[],
+ visitors: ~[],
};
// Install defaults.
@@ -851,6 +882,15 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
cx.set_level(lint, level);
}
+ // 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:
@@ -862,13 +902,14 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
check_item_default_methods(cx, it);
check_item_heap(cx, it);
- check_attributed_node(cx, Item(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) {
- check_item_session_lints(cx, Crate(crate));
+ cx.process(Crate(crate));
visit::visit_crate(crate, cx, visit::mk_vt(@visit::Visitor {
visit_item: visit_item,
@@ -876,7 +917,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
match *fk {
visit::fk_method(_, _, m) => {
do cx.with_lint_attrs(m.attrs) {
- check_attributed_node(cx, Method(m));
+ cx.process(Method(m));
visit::visit_fn(fk, decl, body, span, id, cx, vt);
}
}
@@ -889,6 +930,8 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
}));
}
+ // 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 {
@@ -898,5 +941,6 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
}
}
}
+
tcx.sess.abort_if_errors();
}
From 4d44abd52aa03b5c896849d4785d387538fbb51e Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Thu, 9 May 2013 13:22:26 -0400
Subject: [PATCH 3/3] Change lints to indicate exactly where the level was set
---
src/librustc/driver/session.rs | 9 --
src/librustc/middle/lint.rs | 127 ++++++++++++++++++--------
src/librustc/middle/resolve_stage0.rs | 7 +-
3 files changed, 89 insertions(+), 54 deletions(-)
diff --git a/src/librustc/driver/session.rs b/src/librustc/driver/session.rs
index 790c24369df..6fba5ec8d3a 100644
--- a/src/librustc/driver/session.rs
+++ b/src/librustc/driver/session.rs
@@ -223,15 +223,6 @@ 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; }
diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs
index 2aa9a8c7001..aecbcb626e9 100644
--- a/src/librustc/middle/lint.rs
+++ b/src/librustc/middle/lint.rs
@@ -32,20 +32,29 @@ use syntax::{ast, visit, ast_util};
* 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 for the particular attribute is.
+ * level at that location is.
*
- * To actually emit lint warnings/errors, 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. Once the lint state has been altered, all of the known
- * lint passes are run on the node of the ast.
+ * 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.
*
- * 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.
+ * 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)]
@@ -100,6 +109,13 @@ enum AttributedNode<'self> {
Crate(@ast::crate),
}
+#[deriving(Eq)]
+enum LintSource {
+ Node(span),
+ Default,
+ CommandLine
+}
+
static lint_table: &'static [(&'static str, LintSpec)] = &[
("ctypes",
LintSpec {
@@ -240,29 +256,17 @@ pub fn get_lint_dict() -> LintDict {
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 = SmallIntMap<level>;
-type LintModeMap = HashMap<ast::node_id, LintModes>;
-
struct Context {
// All known lint modes (string versions)
dict: @LintDict,
// Current levels of each lint warning
- curr: LintModes,
+ 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)],
+ 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,
@@ -274,21 +278,65 @@ struct Context {
impl Context {
fn get_level(&self, lint: lint) -> level {
match self.curr.find(&(lint as uint)) {
- Some(&c) => c,
+ Some(&(lvl, _)) => lvl,
None => allow
}
}
- fn set_level(&mut self, lint: lint, level: level) {
- if level == allow {
- self.curr.remove(&(lint as uint));
- } else {
- self.curr.insert(lint as uint, level);
+ 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) {
- self.tcx.sess.span_lint_level(self.get_level(lint), span, msg);
+ 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");
+ }
}
/**
@@ -325,9 +373,10 @@ impl Context {
}
if now != level {
- self.lint_stack.push((lint, now));
+ let src = self.get_source(lint);
+ self.lint_stack.push((lint, now, src));
pushed += 1;
- self.set_level(lint, level);
+ self.set_level(lint, level, Node(meta.span));
}
}
@@ -335,8 +384,8 @@ impl Context {
// rollback
for pushed.times {
- let (lint, level) = self.lint_stack.pop();
- self.set_level(lint, level);
+ let (lint, lvl, src) = self.lint_stack.pop();
+ self.set_level(lint, lvl, src);
}
}
@@ -874,12 +923,12 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
// Install defaults.
for cx.dict.each_value |spec| {
- cx.set_level(spec.lint, spec.default);
+ 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);
+ cx.set_level(lint, level, CommandLine);
}
// Register each of the lint passes with the context
diff --git a/src/librustc/middle/resolve_stage0.rs b/src/librustc/middle/resolve_stage0.rs
index 0c3ec9797da..713132b12fc 100644
--- a/src/librustc/middle/resolve_stage0.rs
+++ b/src/librustc/middle/resolve_stage0.rs
@@ -5229,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)!
}
}
}