Skip to content

Mark allowed dead code and lang items as live #11471

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 12, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 46 additions & 12 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,23 @@
// closely. The idea is that all reachable symbols are live, codes called
// from live codes are live, and everything else is dead.

use middle::lint::{allow, contains_lint, DeadCode};
use middle::privacy;
use middle::ty;
use middle::typeck;
use middle::privacy;
use middle::lint::DeadCode;

use std::hashmap::HashSet;
use syntax::ast;
use syntax::ast_map;
use syntax::ast_util::{local_def, def_id_of_def, is_local};
use syntax::attr;
use syntax::codemap;
use syntax::parse::token;
use syntax::visit::Visitor;
use syntax::visit;

pub static DEAD_CODE_LINT_STR: &'static str = "dead_code";

// Any local node that may call something in its body block should be
// explored. For example, if it's a live NodeItem that is a
// function, then we should explore its block to check for codes that
Expand Down Expand Up @@ -196,26 +199,57 @@ impl Visitor<()> for MarkSymbolVisitor {
}
}

// This visitor is used to mark the implemented methods of a trait. Since we
// can not be sure if such methods are live or dead, we simply mark them
// as live.
struct TraitMethodSeeder {
fn has_allow_dead_code_or_lang_attr(attrs: &[ast::Attribute]) -> bool {
contains_lint(attrs, allow, DEAD_CODE_LINT_STR)
|| attr::contains_name(attrs, "lang")
}

// This visitor seeds items that
// 1) We want to explicitly consider as live:
// * Item annotated with #[allow(dead_code)]
// - This is done so that if we want to suppress warnings for a
// group of dead functions, we only have to annotate the "root".
// For example, if both `f` and `g` are dead and `f` calls `g`,
// then annotating `f` with `#[allow(dead_code)]` will suppress
// warning for both `f` and `g`.
// * Item annotated with #[lang=".."]
// - This is because lang items are always callable from elsewhere.
// or
// 2) We are not sure to be live or not
// * Implementation of a trait method
struct LifeSeeder {
worklist: ~[ast::NodeId],
}

impl Visitor<()> for TraitMethodSeeder {
impl Visitor<()> for LifeSeeder {
fn visit_item(&mut self, item: &ast::Item, _: ()) {
if has_allow_dead_code_or_lang_attr(item.attrs) {
self.worklist.push(item.id);
}
match item.node {
ast::ItemImpl(_, Some(ref _trait_ref), _, ref methods) => {
for method in methods.iter() {
self.worklist.push(method.id);
}
}
ast::ItemMod(..) | ast::ItemFn(..) => {
visit::walk_item(self, item, ());
_ => ()
}
visit::walk_item(self, item, ());
}

fn visit_fn(&mut self, fk: &visit::FnKind,
_: &ast::FnDecl, block: &ast::Block,
_: codemap::Span, id: ast::NodeId, _: ()) {
// Check for method here because methods are not ast::Item
match *fk {
visit::FkMethod(_, _, method) => {
if has_allow_dead_code_or_lang_attr(method.attrs) {
self.worklist.push(id);
}
}
_ => ()
}
visit::walk_block(self, block, ());
}
}

Expand Down Expand Up @@ -244,12 +278,12 @@ fn create_and_seed_worklist(tcx: ty::ctxt,
}

// Seed implemeneted trait methods
let mut trait_method_seeder = TraitMethodSeeder {
let mut life_seeder = LifeSeeder {
worklist: worklist
};
visit::walk_crate(&mut trait_method_seeder, crate, ());
visit::walk_crate(&mut life_seeder, crate, ());

return trait_method_seeder.worklist;
return life_seeder.worklist;
}

fn find_live(tcx: ty::ctxt,
Expand Down
24 changes: 23 additions & 1 deletion src/librustc/middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
//! Context itself, span_lint should be used instead of add_lint.

use driver::session;
use middle::dead::DEAD_CODE_LINT_STR;
use middle::privacy;
use middle::trans::adt; // for `adt::is_ffi_safe`
use middle::ty;
Expand Down Expand Up @@ -293,7 +294,7 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
default: warn
}),

("dead_code",
(DEAD_CODE_LINT_STR,
LintSpec {
lint: DeadCode,
desc: "detect piece of code that will never be used",
Expand Down Expand Up @@ -531,6 +532,8 @@ impl<'a> Context<'a> {
}
}

// Check that every lint from the list of attributes satisfies `f`.
// Return true if that's the case. Otherwise return false.
pub fn each_lint(sess: session::Session,
attrs: &[ast::Attribute],
f: |@ast::MetaItem, level, @str| -> bool)
Expand Down Expand Up @@ -564,6 +567,25 @@ pub fn each_lint(sess: session::Session,
true
}

// Check from a list of attributes if it contains the appropriate
// `#[level(lintname)]` attribute (e.g. `#[allow(dead_code)]).
pub fn contains_lint(attrs: &[ast::Attribute],
level: level, lintname: &'static str) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each_lint seems to be more like an all(..) function whereas what I want to do is more along the line of an any(..) function, so I decided that it's more straightforward to write a separate contains_lint function.

let level_name = level_to_str(level);
for attr in attrs.iter().filter(|m| level_name == m.name()) {
if attr.meta_item_list().is_none() {
continue
}
let list = attr.meta_item_list().unwrap();
for meta_item in list.iter() {
if lintname == meta_item.name() {
return true;
}
}
}
false
}

fn check_while_true_expr(cx: &Context, e: &ast::Expr) {
match e.node {
ast::ExprWhile(cond, _) => {
Expand Down
11 changes: 11 additions & 0 deletions src/test/compile-fail/lint-dead-code-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[no_std];
#[allow(unused_variable)];
#[deny(dead_code)];

Expand Down Expand Up @@ -85,3 +86,13 @@ fn foo() { //~ ERROR: code is never used
fn bar() { //~ ERROR: code is never used
foo();
}

// Code with #[allow(dead_code)] should be marked live (and thus anything it
// calls is marked live)
#[allow(dead_code)]
fn g() { h(); }
fn h() {}

// Similarly, lang items are live
#[lang="fail_"]
fn fail(_: *u8, _: *u8, _: uint) -> ! { loop {} }