Skip to content

Commit

Permalink
auto merge of #14696 : jakub-/rust/dead-struct-fields, r=alexcrichton
Browse files Browse the repository at this point in the history
This uncovered some dead code, most notably in middle/liveness.rs, which I think suggests there must be something fishy with that part of the code.

The #[allow(dead_code)] annotations on some of the fields I am not super happy about but as I understand, marker type may disappear at some point.
  • Loading branch information
bors committed Jun 10, 2014
2 parents 0ee6a8e + 8e34f64 commit 9bb8f88
Show file tree
Hide file tree
Showing 40 changed files with 223 additions and 148 deletions.
3 changes: 0 additions & 3 deletions src/libglob/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ use std::string::String;
* pattern - see the `glob` function for more details.
*/
pub struct Paths {
root: Path,
dir_patterns: Vec<Pattern>,
require_dir: bool,
options: MatchOptions,
Expand Down Expand Up @@ -108,7 +107,6 @@ pub fn glob_with(pattern: &str, options: MatchOptions) -> Paths {
// FIXME: How do we want to handle verbatim paths? I'm inclined to return nothing,
// since we can't very well find all UNC shares with a 1-letter server name.
return Paths {
root: root,
dir_patterns: Vec::new(),
require_dir: false,
options: options,
Expand All @@ -134,7 +132,6 @@ pub fn glob_with(pattern: &str, options: MatchOptions) -> Paths {
}

Paths {
root: root,
dir_patterns: dir_patterns,
require_dir: require_dir,
options: options,
Expand Down
1 change: 1 addition & 0 deletions src/libgreen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ extern {
// stacks are disabled.

#[cfg(target_arch = "x86")]
#[repr(C)]
struct Registers {
eax: u32, ebx: u32, ecx: u32, edx: u32,
ebp: u32, esi: u32, edi: u32, esp: u32,
Expand Down
2 changes: 2 additions & 0 deletions src/libnative/io/c_win32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub static FIONBIO: libc::c_long = 0x8004667e;
static FD_SETSIZE: uint = 64;
pub static MSG_DONTWAIT: libc::c_int = 0;

#[repr(C)]
pub struct WSADATA {
pub wVersion: libc::WORD,
pub wHighVersion: libc::WORD,
Expand All @@ -32,6 +33,7 @@ pub struct WSADATA {

pub type LPWSADATA = *mut WSADATA;

#[repr(C)]
pub struct fd_set {
fd_count: libc::c_uint,
fd_array: [libc::SOCKET, ..FD_SETSIZE],
Expand Down
4 changes: 2 additions & 2 deletions src/libnative/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ fn keep_going(data: &[u8], f: |*u8, uint| -> i64) -> i64 {
/// Implementation of rt::rtio's IoFactory trait to generate handles to the
/// native I/O functionality.
pub struct IoFactory {
cannot_construct_outside_of_this_module: ()
_cannot_construct_outside_of_this_module: ()
}

impl IoFactory {
pub fn new() -> IoFactory {
net::init();
IoFactory { cannot_construct_outside_of_this_module: () }
IoFactory { _cannot_construct_outside_of_this_module: () }
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/libnative/io/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,10 @@ pub struct TcpStream {

struct Inner {
fd: sock_t,
lock: mutex::NativeMutex,

// Unused on Linux, where this lock is not necessary.
#[allow(dead_code)]
lock: mutex::NativeMutex
}

pub struct Guard<'a> {
Expand Down
5 changes: 4 additions & 1 deletion src/libnative/io/pipe_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ fn addr_to_sockaddr_un(addr: &CString) -> IoResult<(libc::sockaddr_storage, uint

struct Inner {
fd: fd_t,
lock: mutex::NativeMutex,

// Unused on Linux, where this lock is not necessary.
#[allow(dead_code)]
lock: mutex::NativeMutex
}

impl Inner {
Expand Down
2 changes: 0 additions & 2 deletions src/librand/distributions/gamma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ struct GammaSmallShape {
/// See `Gamma` for sampling from a Gamma distribution with general
/// shape parameters.
struct GammaLargeShape {
shape: f64,
scale: f64,
c: f64,
d: f64
Expand Down Expand Up @@ -118,7 +117,6 @@ impl GammaLargeShape {
fn new_raw(shape: f64, scale: f64) -> GammaLargeShape {
let d = shape - 1. / 3.;
GammaLargeShape {
shape: shape,
scale: scale,
c: 1. / (9. * d).sqrt(),
d: d
Expand Down
10 changes: 3 additions & 7 deletions src/librustc/front/std_inject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,7 @@ fn inject_crates_ref(sess: &Session, krate: ast::Crate) -> ast::Crate {
fold.fold_crate(krate)
}

struct PreludeInjector<'a> {
sess: &'a Session,
}
struct PreludeInjector<'a>;


impl<'a> fold::Folder for PreludeInjector<'a> {
Expand Down Expand Up @@ -223,9 +221,7 @@ impl<'a> fold::Folder for PreludeInjector<'a> {
}
}

fn inject_prelude(sess: &Session, krate: ast::Crate) -> ast::Crate {
let mut fold = PreludeInjector {
sess: sess,
};
fn inject_prelude(_: &Session, krate: ast::Crate) -> ast::Crate {
let mut fold = PreludeInjector;
fold.fold_crate(krate)
}
4 changes: 2 additions & 2 deletions src/librustc/metadata/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub struct Library {
}

pub struct ArchiveMetadata {
archive: ArchiveRO,
_archive: ArchiveRO,
// See comments in ArchiveMetadata::new for why this is static
data: &'static [u8],
}
Expand Down Expand Up @@ -487,7 +487,7 @@ impl ArchiveMetadata {
unsafe { mem::transmute(data) }
};
Some(ArchiveMetadata {
archive: ar,
_archive: ar,
data: data,
})
}
Expand Down
4 changes: 0 additions & 4 deletions src/librustc/metadata/tyencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ pub struct ctxt<'a> {
// Extra parameters are for converting to/from def_ids in the string rep.
// Whatever format you choose should not contain pipe characters.
pub struct ty_abbrev {
pos: uint,
len: uint,
s: String
}

Expand All @@ -68,8 +66,6 @@ pub fn enc_ty(w: &mut MemWriter, cx: &ctxt, t: ty::t) {
if abbrev_len < len {
// I.e. it's actually an abbreviation.
cx.abbrevs.borrow_mut().insert(t, ty_abbrev {
pos: pos as uint,
len: len as uint,
s: format!("\\#{:x}:{:x}\\#", pos, len)
});
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/middle/borrowck/gather_loans/lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn guarantee_lifetime(bccx: &BorrowckCtxt,
cause: euv::LoanCause,
cmt: mc::cmt,
loan_region: ty::Region,
loan_kind: ty::BorrowKind)
_: ty::BorrowKind)
-> Result<(),()> {
debug!("guarantee_lifetime(cmt={}, loan_region={})",
cmt.repr(bccx.tcx), loan_region.repr(bccx.tcx));
Expand All @@ -38,7 +38,6 @@ pub fn guarantee_lifetime(bccx: &BorrowckCtxt,
span: span,
cause: cause,
loan_region: loan_region,
loan_kind: loan_kind,
cmt_original: cmt.clone()};
ctxt.check(&cmt, None)
}
Expand All @@ -55,7 +54,6 @@ struct GuaranteeLifetimeContext<'a> {
span: Span,
cause: euv::LoanCause,
loan_region: ty::Region,
loan_kind: ty::BorrowKind,
cmt_original: mc::cmt
}

Expand Down
7 changes: 2 additions & 5 deletions src/librustc/middle/borrowck/gather_loans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ impl<'a> GatherLoanCtxt<'a> {
Loan {
index: self.all_loans.len(),
loan_path: loan_path,
cmt: cmt,
kind: req_kind,
gen_scope: gen_scope,
kill_scope: kill_scope,
Expand Down Expand Up @@ -481,8 +480,7 @@ impl<'a> GatherLoanCtxt<'a> {
/// This visitor walks static initializer's expressions and makes
/// sure the loans being taken are sound.
struct StaticInitializerCtxt<'a> {
bccx: &'a BorrowckCtxt<'a>,
item_ub: ast::NodeId,
bccx: &'a BorrowckCtxt<'a>
}

impl<'a> visit::Visitor<()> for StaticInitializerCtxt<'a> {
Expand All @@ -509,8 +507,7 @@ pub fn gather_loans_in_static_initializer(bccx: &mut BorrowckCtxt, expr: &ast::E
debug!("gather_loans_in_static_initializer(expr={})", expr.repr(bccx.tcx));

let mut sicx = StaticInitializerCtxt {
bccx: bccx,
item_ub: expr.id,
bccx: bccx
};

sicx.visit_expr(expr, ());
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/middle/borrowck/gather_loans/restrictions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub fn compute_restrictions(bccx: &BorrowckCtxt,
bccx: bccx,
span: span,
cause: cause,
cmt_original: cmt.clone(),
loan_region: loan_region,
};

Expand All @@ -49,7 +48,6 @@ pub fn compute_restrictions(bccx: &BorrowckCtxt,
struct RestrictionsContext<'a> {
bccx: &'a BorrowckCtxt<'a>,
span: Span,
cmt_original: mc::cmt,
loan_region: ty::Region,
cause: euv::LoanCause,
}
Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ pub enum PartialTotal {
pub struct Loan {
index: uint,
loan_path: Rc<LoanPath>,
cmt: mc::cmt,
kind: ty::BorrowKind,
restrictions: Vec<Restriction>,
gen_scope: ast::NodeId,
Expand Down
81 changes: 79 additions & 2 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,32 @@ impl<'a> MarkSymbolVisitor<'a> {
}
}

fn handle_field_access(&mut self, lhs: &ast::Expr, name: &ast::Ident) {
match ty::get(ty::expr_ty_adjusted(self.tcx, lhs)).sty {
ty::ty_struct(id, _) => {
let fields = ty::lookup_struct_fields(self.tcx, id);
let field_id = fields.iter()
.find(|field| field.name == name.name).unwrap().id;
self.live_symbols.insert(field_id.node);
},
_ => ()
}
}

fn handle_field_pattern_match(&mut self, lhs: &ast::Pat, pats: &[ast::FieldPat]) {
match self.tcx.def_map.borrow().get(&lhs.id) {
&def::DefStruct(id) | &def::DefVariant(_, id, _) => {
let fields = ty::lookup_struct_fields(self.tcx, id);
for pat in pats.iter() {
let field_id = fields.iter()
.find(|field| field.name == pat.ident.name).unwrap().id;
self.live_symbols.insert(field_id.node);
}
}
_ => ()
}
}

fn mark_live_symbols(&mut self) {
let mut scanned = HashSet::new();
while self.worklist.len() > 0 {
Expand All @@ -147,10 +173,22 @@ impl<'a> MarkSymbolVisitor<'a> {
match *node {
ast_map::NodeItem(item) => {
match item.node {
ast::ItemStruct(struct_def, _) => {
let has_extern_repr = item.attrs.iter().fold(attr::ReprAny, |acc, attr| {
attr::find_repr_attr(self.tcx.sess.diagnostic(), attr, acc)
}) == attr::ReprExtern;
let live_fields = struct_def.fields.iter().filter(|f| {
has_extern_repr || match f.node.kind {
ast::NamedField(_, ast::Public) => true,
_ => false
}
});
self.live_symbols.extend(live_fields.map(|f| f.node.id));
visit::walk_item(self, item, ());
}
ast::ItemFn(..)
| ast::ItemTy(..)
| ast::ItemEnum(..)
| ast::ItemStruct(..)
| ast::ItemStatic(..) => {
visit::walk_item(self, item, ());
}
Expand Down Expand Up @@ -178,18 +216,32 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> {
ast::ExprMethodCall(..) => {
self.lookup_and_handle_method(expr.id, expr.span);
}
ast::ExprField(ref lhs, ref ident, _) => {
self.handle_field_access(*lhs, ident);
}
_ => ()
}

visit::walk_expr(self, expr, ())
}

fn visit_pat(&mut self, pat: &ast::Pat, _: ()) {
match pat.node {
ast::PatStruct(_, ref fields, _) => {
self.handle_field_pattern_match(pat, fields.as_slice());
}
_ => ()
}

visit::walk_pat(self, pat, ())
}

fn visit_path(&mut self, path: &ast::Path, id: ast::NodeId, _: ()) {
self.lookup_and_handle_definition(&id);
visit::walk_path(self, path, ());
}

fn visit_item(&mut self, _item: &ast::Item, _: ()) {
fn visit_item(&mut self, _: &ast::Item, _: ()) {
// Do not recurse into items. These items will be added to the
// worklist and recursed into manually if necessary.
}
Expand Down Expand Up @@ -317,6 +369,23 @@ struct DeadVisitor<'a> {
}

impl<'a> DeadVisitor<'a> {
fn should_warn_about_field(&mut self, node: &ast::StructField_) -> bool {
let (is_named, has_leading_underscore) = match node.ident() {
Some(ref ident) => (true, token::get_ident(*ident).get()[0] == ('_' as u8)),
_ => (false, false)
};
let field_type = ty::node_id_to_type(self.tcx, node.id);
let is_marker_field = match ty::ty_to_def_id(field_type) {
Some(def_id) => self.tcx.lang_items.items().any(|(_, item)| *item == Some(def_id)),
_ => false
};
is_named
&& !self.symbol_is_live(node.id, None)
&& !has_leading_underscore
&& !is_marker_field
&& !has_allow_dead_code_or_lang_attr(node.attrs.as_slice())
}

// id := node id of an item's definition.
// ctor_id := `Some` if the item is a struct_ctor (tuple struct),
// `None` otherwise.
Expand Down Expand Up @@ -399,6 +468,14 @@ impl<'a> Visitor<()> for DeadVisitor<'a> {
visit::walk_block(self, block, ());
}

fn visit_struct_field(&mut self, field: &ast::StructField, _: ()) {
if self.should_warn_about_field(&field.node) {
self.warn_dead_code(field.node.id, field.span, field.node.ident().unwrap());
}

visit::walk_struct_field(self, field, ());
}

// Overwrite so that we don't warn the trait method itself.
fn visit_trait_method(&mut self, trait_method: &ast::TraitMethod, _: ()) {
match *trait_method {
Expand Down
Loading

0 comments on commit 9bb8f88

Please sign in to comment.