Skip to content

Convert lints to use a trait-based infrastructure #14804

New issue

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

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

Already on GitHub? Sign in to your account

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
395d205
Move lint.rs out of middle
kmcallister Jun 1, 2014
8a50032
Move lint infrastructure and individual lints into separate files
kmcallister Jun 1, 2014
3d58010
Rename lint::Lint to lint::LintId
kmcallister Jun 1, 2014
d79e624
Convert lints to a trait-based infrastructure
kmcallister Jun 2, 2014
4c50f8b
Replace enum LintId with an extensible alternative
kmcallister Jun 4, 2014
1545869
Store the registered lints in the Session
kmcallister Jun 10, 2014
129aa8b
Clean up and document the public lint API
kmcallister Jun 6, 2014
0260fd2
Stop using Default for initializing builtin lints
kmcallister Jun 13, 2014
8a99dbb
Remove big indent in enum_variant_size_lint
kmcallister Jun 13, 2014
9b90a7d
Use default PartialEq::ne for LintId
kmcallister Jun 13, 2014
6f77351
describe_lints: Address review comments
kmcallister Jun 13, 2014
bd73a4b
Run lint passes using the Option dance instead of RefCells
kmcallister Jun 13, 2014
4f026f1
Use names in Lint structs in an ASCII-case-insensitive way
kmcallister Jun 13, 2014
90a33b7
Convert builtin lints to uppercase names for style consistency
kmcallister Jun 13, 2014
b8a2a0f
Rework lint attr parsing and use it in middle::dead
kmcallister Jun 13, 2014
c45b9a7
Make lint::context private
kmcallister Jun 13, 2014
e8f4229
Remove a HeapMemory lint FIXME
kmcallister Jun 13, 2014
18b834c
Make lint::Context conform to rustc API conventions
kmcallister Jun 13, 2014
3b846ef
fixup! Rework lint attr parsing and use it in middle::dead
kmcallister Jun 14, 2014
c306d20
fixup! Make lint::Context conform to rustc API conventions
kmcallister Jun 17, 2014
6512035
Make the crate and its exported items available in the lint context
kmcallister Jun 17, 2014
5d1872e
Drop the ExportedItems argument from LintPass::check_crate
kmcallister Jun 18, 2014
d98af22
Reorder arguments to lint::check_crate for consistency
kmcallister Jun 18, 2014
b0597c6
Reindent function call continuations, and other style fixes
kmcallister Jun 18, 2014
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 14 additions & 26 deletions src/librustc/driver/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use back::link;
use back::target_strs;
use back::{arm, x86, x86_64, mips};
use metadata;
use middle::lint;
use lint;

use syntax::abi;
use syntax::ast;
Expand Down Expand Up @@ -70,7 +70,8 @@ pub struct Options {
pub gc: bool,
pub optimize: OptLevel,
pub debuginfo: DebugInfoLevel,
pub lint_opts: Vec<(lint::Lint, lint::Level)> ,
pub lint_opts: Vec<(String, lint::Level)>,
pub describe_lints: bool,
pub output_types: Vec<back::link::OutputType> ,
// This was mutable for rustpkg, which updates search paths based on the
// parsed code. It remains mutable in case its replacements wants to use
Expand Down Expand Up @@ -104,6 +105,7 @@ pub fn basic_options() -> Options {
optimize: No,
debuginfo: NoDebugInfo,
lint_opts: Vec::new(),
describe_lints: false,
output_types: Vec::new(),
addl_lib_search_paths: RefCell::new(HashSet::new()),
maybe_sysroot: None,
Expand Down Expand Up @@ -588,30 +590,15 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let no_trans = matches.opt_present("no-trans");
let no_analysis = matches.opt_present("no-analysis");

let lint_levels = [lint::Allow, lint::Warn,
lint::Deny, lint::Forbid];
let mut lint_opts = Vec::new();
let lint_dict = lint::get_lint_dict();
for level in lint_levels.iter() {
let level_name = lint::level_to_str(*level);

let level_short = level_name.slice_chars(0, 1);
let level_short = level_short.to_ascii().to_upper().into_str();
let flags = matches.opt_strs(level_short.as_slice())
.move_iter()
.collect::<Vec<_>>()
.append(matches.opt_strs(level_name).as_slice());
for lint_name in flags.iter() {
let lint_name = lint_name.replace("-", "_").into_string();
match lint_dict.find_equiv(&lint_name) {
None => {
early_error(format!("unknown {} flag: {}",
level_name,
lint_name).as_slice());
}
Some(lint) => {
lint_opts.push((lint.lint, *level));
}
let mut lint_opts = vec!();
let mut describe_lints = false;

for &level in [lint::Allow, lint::Warn, lint::Deny, lint::Forbid].iter() {
for lint_name in matches.opt_strs(level.as_str()).move_iter() {
if lint_name.as_slice() == "help" {
describe_lints = true;
} else {
lint_opts.push((lint_name.replace("-", "_").into_string(), level));
}
}
}
Expand Down Expand Up @@ -755,6 +742,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
optimize: opt_level,
debuginfo: debuginfo,
lint_opts: lint_opts,
describe_lints: describe_lints,
output_types: output_types,
addl_lib_search_paths: RefCell::new(addl_lib_search_paths),
maybe_sysroot: sysroot_opt,
Expand Down
42 changes: 31 additions & 11 deletions src/librustc/driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ use metadata::common::LinkMeta;
use metadata::creader;
use middle::cfg;
use middle::cfg::graphviz::LabelledCFG;
use middle::{trans, freevars, kind, ty, typeck, lint, reachable};
use middle::{trans, freevars, kind, ty, typeck, reachable};
use middle::dependency_format;
use middle;
use plugin::load::Plugins;
use plugin::registry::Registry;
use plugin;
use lint;
use util::common::time;
use util::ppaux;
use util::nodemap::{NodeSet};
Expand Down Expand Up @@ -78,8 +79,12 @@ pub fn compile_input(sess: Session,
&sess);
let id = link::find_crate_id(krate.attrs.as_slice(),
outputs.out_filestem.as_slice());
let (expanded_crate, ast_map) =
phase_2_configure_and_expand(&sess, krate, &id);
let (expanded_crate, ast_map)
= match phase_2_configure_and_expand(&sess, krate, &id) {
None => return,
Some(p) => p,
};

(outputs, expanded_crate, ast_map)
};
write_out_deps(&sess, input, &outputs, &expanded_crate);
Expand Down Expand Up @@ -171,10 +176,12 @@ pub fn phase_1_parse_input(sess: &Session, cfg: ast::CrateConfig, input: &Input)
/// syntax expansion, secondary `cfg` expansion, synthesis of a test
/// harness if one is to be provided and injection of a dependency on the
/// standard library and prelude.
///
/// Returns `None` if we're aborting after handling -W help.
pub fn phase_2_configure_and_expand(sess: &Session,
mut krate: ast::Crate,
crate_id: &CrateId)
-> (ast::Crate, syntax::ast_map::Map) {
-> Option<(ast::Crate, syntax::ast_map::Map)> {
let time_passes = sess.time_passes();

*sess.crate_types.borrow_mut() = collect_crate_types(sess, krate.attrs.as_slice());
Expand Down Expand Up @@ -210,6 +217,17 @@ pub fn phase_2_configure_and_expand(sess: &Session,

let Registry { syntax_exts, .. } = registry;

// Process command line flags for lints.
// Do this here because we will have lint plugins eventually.
if sess.opts.describe_lints {
super::describe_lints(&*sess.lint_store.borrow());
return None;
}
sess.lint_store.borrow_mut().process_command_line(sess);

// Abort if there are errors from lint processing or a plugin registrar.
sess.abort_if_errors();

krate = time(time_passes, "expansion", (krate, macros, syntax_exts),
|(krate, macros, syntax_exts)| {
// Windows dlls do not have rpaths, so they don't know how to find their
Expand Down Expand Up @@ -252,7 +270,7 @@ pub fn phase_2_configure_and_expand(sess: &Session,
krate.encode(&mut json).unwrap();
}

(krate, map)
Some((krate, map))
}

pub struct CrateAnalysis {
Expand Down Expand Up @@ -359,7 +377,7 @@ pub fn phase_3_run_analysis_passes(sess: Session,
});

time(time_passes, "lint checking", (), |_|
lint::check_crate(&ty_cx, &exported_items, krate));
lint::check_crate(&ty_cx, krate, &exported_items));

CrateAnalysis {
exp_map2: exp_map2,
Expand Down Expand Up @@ -612,9 +630,11 @@ pub fn pretty_print_input(sess: Session,

let (krate, ast_map, is_expanded) = match ppm {
PpmExpanded | PpmExpandedIdentified | PpmTyped | PpmFlowGraph(_) => {
let (krate, ast_map) = phase_2_configure_and_expand(&sess,
krate,
&id);
let (krate, ast_map)
= match phase_2_configure_and_expand(&sess, krate, &id) {
None => return,
Some(p) => p,
};
(krate, Some(ast_map), true)
}
_ => (krate, None, false)
Expand Down Expand Up @@ -748,15 +768,15 @@ pub fn collect_crate_types(session: &Session,
}
Some(ref n) if n.equiv(&("bin")) => Some(config::CrateTypeExecutable),
Some(_) => {
session.add_lint(lint::UnknownCrateType,
session.add_lint(lint::builtin::UNKNOWN_CRATE_TYPE,
ast::CRATE_NODE_ID,
a.span,
"invalid `crate_type` \
value".to_string());
None
}
_ => {
session.add_lint(lint::UnknownCrateType,
session.add_lint(lint::builtin::UNKNOWN_CRATE_TYPE,
ast::CRATE_NODE_ID,
a.span,
"`crate_type` requires a \
Expand Down
91 changes: 55 additions & 36 deletions src/librustc/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ pub use syntax::diagnostic;
use back::link;
use driver::driver::{Input, FileInput, StrInput};
use driver::session::{Session, build_session};
use middle::lint;
use lint::Lint;
use lint;
use metadata;

use std::any::AnyRefExt;
use std::cmp;
use std::io;
use std::os;
use std::str;
Expand Down Expand Up @@ -49,9 +49,18 @@ fn run_compiler(args: &[String]) {
Some(matches) => matches,
None => return
};
let sopts = config::build_session_options(&matches);

let (input, input_file_path) = match matches.free.len() {
0u => early_error("no input filename given"),
0u => {
if sopts.describe_lints {
let mut ls = lint::LintStore::new();
ls.register_builtin(None);
describe_lints(&ls);
return;
}
early_error("no input filename given");
}
1u => {
let ifile = matches.free.get(0).as_slice();
if ifile == "-" {
Expand All @@ -66,7 +75,6 @@ fn run_compiler(args: &[String]) {
_ => early_error("multiple input filenames provided")
};

let sopts = config::build_session_options(&matches);
let sess = build_session(sopts, input_file_path);
let cfg = config::build_configuration(&sess);
let odir = matches.opt_str("out-dir").map(|o| Path::new(o));
Expand Down Expand Up @@ -124,41 +132,57 @@ Additional help:
config::optgroups().as_slice()));
}

fn describe_warnings() {
fn describe_lints(lint_store: &lint::LintStore) {
println!("
Available lint options:
-W <foo> Warn about <foo>
-A <foo> Allow <foo>
-D <foo> Deny <foo>
-F <foo> Forbid <foo> (deny, and deny all overrides)
");

let lint_dict = lint::get_lint_dict();
let mut lint_dict = lint_dict.move_iter()
.map(|(k, v)| (v, k))
.collect::<Vec<(lint::LintSpec, &'static str)> >();
lint_dict.as_mut_slice().sort();
");

let mut max_key = 0;
for &(_, name) in lint_dict.iter() {
max_key = cmp::max(name.len(), max_key);
}
fn padded(max: uint, s: &str) -> String {
format!("{}{}", " ".repeat(max - s.len()), s)
}
println!("\nAvailable lint checks:\n");
println!(" {} {:7.7s} {}",
padded(max_key, "name"), "default", "meaning");
println!(" {} {:7.7s} {}\n",
padded(max_key, "----"), "-------", "-------");
for (spec, name) in lint_dict.move_iter() {
let name = name.replace("_", "-");
println!(" {} {:7.7s} {}",
padded(max_key, name.as_slice()),
lint::level_to_str(spec.default),
spec.desc);
fn sort_lints(lints: Vec<(&'static Lint, bool)>) -> Vec<&'static Lint> {
let mut lints: Vec<_> = lints.move_iter().map(|(x, _)| x).collect();
lints.sort_by(|x: &&Lint, y: &&Lint| {
match x.default_level.cmp(&y.default_level) {
// The sort doesn't case-fold but it's doubtful we care.
Equal => x.name.cmp(&y.name),
r => r,
}
});
lints
}
println!("");

let (_plugin, builtin) = lint_store.get_lints().partitioned(|&(_, p)| p);
// let plugin = sort_lints(plugin);
let builtin = sort_lints(builtin);

// FIXME (#7043): We should use the width in character cells rather than
// the number of codepoints.
let max_name_len = builtin.iter()
.map(|&s| s.name.char_len())
.max().unwrap_or(0);
let padded = |x: &str| {
" ".repeat(max_name_len - x.char_len()).append(x)
};

println!("Lint checks provided by rustc:\n");
println!(" {} {:7.7s} {}", padded("name"), "default", "meaning");
println!(" {} {:7.7s} {}", padded("----"), "-------", "-------");

let print_lints = |lints: Vec<&Lint>| {
for lint in lints.move_iter() {
let name = lint.name_lower().replace("_", "-");
println!(" {} {:7.7s} {}",
padded(name.as_slice()), lint.default_level.as_str(), lint.desc);
}
println!("\n");
};

print_lints(builtin);

// Describe lint plugins here once they exist.
}

fn describe_debug_flags() {
Expand Down Expand Up @@ -214,12 +238,7 @@ pub fn handle_options(mut args: Vec<String>) -> Option<getopts::Matches> {
return None;
}

let lint_flags = matches.opt_strs("W").move_iter().collect::<Vec<_>>().append(
matches.opt_strs("warn").as_slice());
if lint_flags.iter().any(|x| x.as_slice() == "help") {
describe_warnings();
return None;
}
// Don't handle -W help here, because we might first load plugins.

let r = matches.opt_strs("Z");
if r.iter().any(|x| x.as_slice() == "help") {
Expand Down
20 changes: 13 additions & 7 deletions src/librustc/driver/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use driver::driver;
use front;
use metadata::cstore::CStore;
use metadata::filesearch;
use middle::lint;
use lint;
use util::nodemap::NodeMap;

use syntax::ast::NodeId;
Expand Down Expand Up @@ -42,7 +42,8 @@ pub struct Session {
// expected to be absolute. `None` means that there is no source file.
pub local_crate_source_file: Option<Path>,
pub working_dir: Path,
pub lints: RefCell<NodeMap<Vec<(lint::Lint, codemap::Span, String)>>>,
pub lint_store: RefCell<lint::LintStore>,
pub lints: RefCell<NodeMap<Vec<(lint::LintId, codemap::Span, String)>>>,
pub node_id: Cell<ast::NodeId>,
pub crate_types: RefCell<Vec<config::CrateType>>,
pub features: front::feature_gate::Features,
Expand Down Expand Up @@ -105,16 +106,17 @@ impl Session {
self.diagnostic().handler().unimpl(msg)
}
pub fn add_lint(&self,
lint: lint::Lint,
lint: &'static lint::Lint,
id: ast::NodeId,
sp: Span,
msg: String) {
let lint_id = lint::LintId::of(lint);
let mut lints = self.lints.borrow_mut();
match lints.find_mut(&id) {
Some(arr) => { arr.push((lint, sp, msg)); return; }
Some(arr) => { arr.push((lint_id, sp, msg)); return; }
None => {}
}
lints.insert(id, vec!((lint, sp, msg)));
lints.insert(id, vec!((lint_id, sp, msg)));
}
pub fn next_node_id(&self) -> ast::NodeId {
self.reserve_node_ids(1)
Expand Down Expand Up @@ -224,7 +226,7 @@ pub fn build_session_(sopts: config::Options,
}
);

Session {
let sess = Session {
targ_cfg: target_cfg,
opts: sopts,
cstore: CStore::new(token::get_ident_interner()),
Expand All @@ -236,12 +238,16 @@ pub fn build_session_(sopts: config::Options,
default_sysroot: default_sysroot,
local_crate_source_file: local_crate_source_file,
working_dir: os::getcwd(),
lint_store: RefCell::new(lint::LintStore::new()),
lints: RefCell::new(NodeMap::new()),
node_id: Cell::new(1),
crate_types: RefCell::new(Vec::new()),
features: front::feature_gate::Features::new(),
recursion_limit: Cell::new(64),
}
};

sess.lint_store.borrow_mut().register_builtin(Some(&sess));
sess
}

// Seems out of place, but it uses session, so I'm putting it here
Expand Down
Loading