languages/rust
languages/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust synced 2024-10-16 05:23:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

auto merge of #14300 : cmr/rust/enum-size-lint, r=kballard

Browse source 
See commits for details.
This commit is contained in:
bors 2014-05-26 00:16:27 -07:00
parent a7ab73344d d8467e23e7
commit ba77c60270
4 changed files with 239 additions and 98 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

150
src/librustc/middle/lint.rs
View file

@ -72,7 +72,7 @@
use syntax::visit::Visitor;
use syntax::{ast, ast_util, visit};
#[deriving(Clone, Eq, Ord, TotalEq, TotalOrd)]
#[deriving(Clone, Show, Eq, Ord, TotalEq, TotalOrd, Hash)]
pub enum Lint {
CTypes,
UnusedImports,
@ -94,6 +94,7 @@ pub enum Lint {
UnknownFeatures,
UnknownCrateType,
UnsignedNegate,
VariantSizeDifference,
ManagedHeapMemory,
OwnedHeapMemory,
@ -147,8 +148,9 @@ pub struct LintSpec {
pub type LintDict = HashMap<&'static str, LintSpec>;
// this is public for the lints that run in trans
#[deriving(Eq)]
enum LintSource {
pub enum LintSource {
Node(Span),
Default,
CommandLine
@ -407,6 +409,13 @@ enum LintSource {
default: Warn
}),
("variant_size_difference",
LintSpec {
lint: VariantSizeDifference,
desc: "detects enums with widely varying variant sizes",
default: Allow,
}),
("unused_must_use",
LintSpec {
lint: UnusedMustUse,
@ -445,30 +454,78 @@ pub fn get_lint_dict() -> LintDict {
}
struct Context<'a> {
// All known lint modes (string versions)
/// All known lint modes (string versions)
dict: LintDict,
// Current levels of each lint warning
/// Current levels of each lint warning
cur: SmallIntMap<(Level, LintSource)>,
// context we're checking in (used to access fields like sess)
/// Context we're checking in (used to access fields like sess)
tcx: &'a ty::ctxt,
// Items exported by the crate; used by the missing_doc lint.
/// Items exported by the crate; used by the missing_doc lint.
exported_items: &'a privacy::ExportedItems,
// The id of the current `ast::StructDef` being walked.
/// The id of the current `ast::StructDef` being walked.
cur_struct_def_id: ast::NodeId,
// Whether some ancestor of the current node was marked
// #[doc(hidden)].
/// Whether some ancestor of the current node was marked
/// #[doc(hidden)].
is_doc_hidden: bool,
// 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.
/// 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: Vec<(Lint, Level, LintSource)>,
// id of the last visited negated expression
/// Id of the last visited negated expression
negated_expr_id: ast::NodeId,
// ids of structs/enums which have been checked for raw_pointer_deriving
/// Ids of structs/enums which have been checked for raw_pointer_deriving
checked_raw_pointers: NodeSet,
/// Level of lints for certain NodeIds, stored here because the body of
/// the lint needs to run in trans.
node_levels: HashMap<(ast::NodeId, Lint), (Level, LintSource)>,
}
pub fn emit_lint(level: Level, src: LintSource, msg: &str, span: Span,
lint_str: &str, tcx: &ty::ctxt) {
if level == Allow { return }
let mut note = None;
let msg = match src {
Default => {
format!("{}, \\#[{}({})] on by default", msg,
level_to_str(level), lint_str)
},
CommandLine => {
format!("{} [-{} {}]", msg,
match level {
Warn => 'W', Deny => 'D', Forbid => 'F',
Allow => fail!()
}, lint_str.replace("_", "-"))
},
Node(src) => {
note = Some(src);
msg.to_str()
}
};
match level {
Warn => { tcx.sess.span_warn(span, msg.as_slice()); }
Deny | Forbid => { tcx.sess.span_err(span, msg.as_slice()); }
Allow => fail!(),
}
for &span in note.iter() {
tcx.sess.span_note(span, "lint level defined here");
}
}
pub fn lint_to_str(lint: Lint) -> &'static str {
for &(name, lspec) in lint_table.iter() {
if lspec.lint == lint {
return name;
}
}
fail!("unrecognized lint: {}", lint);
}
impl<'a> Context<'a> {
@ -500,7 +557,7 @@ fn lint_to_str(&self, lint: Lint) -> &'static str {
return *k;
}
}
fail!("unregistered lint {:?}", lint);
fail!("unregistered lint {}", lint);
}
fn span_lint(&self, lint: Lint, span: Span, msg: &str) {
@ -509,37 +566,8 @@ fn span_lint(&self, lint: Lint, span: Span, msg: &str) {
Some(&(Warn, src)) => (self.get_level(Warnings), src),
Some(&pair) => pair,
};
if level == Allow { return }
let mut note = None;
let msg = match src {
Default => {
format_strbuf!("{}, \\#[{}({})] on by default",
msg,
level_to_str(level),
self.lint_to_str(lint))
},
CommandLine => {
format!("{} [-{} {}]", msg,
match level {
Warn => 'W', Deny => 'D', Forbid => 'F',
Allow => fail!()
}, self.lint_to_str(lint).replace("_", "-"))
},
Node(src) => {
note = Some(src);
msg.to_str()
}
};
match level {
Warn => self.tcx.sess.span_warn(span, msg.as_slice()),
Deny | Forbid => self.tcx.sess.span_err(span, msg.as_slice()),
Allow => fail!(),
}
for &span in note.iter() {
self.tcx.sess.span_note(span, "lint level defined here");
}
emit_lint(level, src, msg, span, self.lint_to_str(lint), self.tcx);
}
/**
@ -618,8 +646,8 @@ fn visit_ids(&self, f: |&mut ast_util::IdVisitor<Context>|) {
}
}
// Check that every lint from the list of attributes satisfies `f`.
// Return true if that's the case. Otherwise return false.
/// 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, InternedString| -> bool)
@ -653,8 +681,8 @@ 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)]).
/// 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)
@ -1739,9 +1767,24 @@ fn check_stability(cx: &Context, e: &ast::Expr) {
cx.span_lint(lint, e.span, msg.as_slice());
}
fn check_enum_variant_sizes(cx: &mut Context, it: &ast::Item) {
match it.node {
ast::ItemEnum(..) => {
match cx.cur.find(&(VariantSizeDifference as uint)) {
Some(&(lvl, src)) if lvl != Allow => {
cx.node_levels.insert((it.id, VariantSizeDifference), (lvl, src));
},
_ => { }
}
},
_ => { }
}
}
impl<'a> Visitor<()> for Context<'a> {
fn visit_item(&mut self, it: &ast::Item, _: ()) {
self.with_lint_attrs(it.attrs.as_slice(), |cx| {
check_enum_variant_sizes(cx, it);
check_item_ctypes(cx, it);
check_item_non_camel_case_types(cx, it);
check_item_non_uppercase_statics(cx, it);
@ -1933,6 +1976,7 @@ pub fn check_crate(tcx: &ty::ctxt,
lint_stack: Vec::new(),
negated_expr_id: -1,
checked_raw_pointers: NodeSet::new(),
node_levels: HashMap::new(),
};
// Install default lint levels, followed by the command line levels, and
@ -1969,13 +2013,11 @@ pub fn check_crate(tcx: &ty::ctxt,
// in the iteration code.
for (id, v) in tcx.sess.lints.borrow().iter() {
for &(lint, span, ref msg) in v.iter() {
tcx.sess.span_bug(span,
format!("unprocessed lint {:?} at {}: {}",
lint,
tcx.map.node_to_str(*id),
*msg).as_slice())
tcx.sess.span_bug(span, format!("unprocessed lint {} at {}: {}",
lint, tcx.map.node_to_str(*id), *msg).as_slice())
}
}
tcx.sess.abort_if_errors();
*tcx.node_lint_levels.borrow_mut() = cx.node_levels;
}

58
src/librustc/middle/trans/base.rs
View file

@ -36,6 +36,7 @@
use lib::llvm::{llvm, Vector};
use lib;
use metadata::{csearch, encoder};
use middle::lint;
use middle::astencode;
use middle::lang_items::{LangItem, ExchangeMallocFnLangItem, StartFnLangItem};
use middle::weak_lang_items;
@ -57,7 +58,7 @@
use middle::trans::glue;
use middle::trans::inline;
use middle::trans::machine;
use middle::trans::machine::{llalign_of_min, llsize_of};
use middle::trans::machine::{llalign_of_min, llsize_of, llsize_of_real};
use middle::trans::meth;
use middle::trans::monomorphize;
use middle::trans::tvec;
@ -1489,7 +1490,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,
}
fn trans_enum_def(ccx: &CrateContext, enum_definition: &ast::EnumDef,
id: ast::NodeId, vi: &[Rc<ty::VariantInfo>],
sp: Span, id: ast::NodeId, vi: &[Rc<ty::VariantInfo>],
i: &mut uint) {
for &variant in enum_definition.variants.iter() {
let disr_val = vi[*i].disr_val;
@ -1509,6 +1510,57 @@ fn trans_enum_def(ccx: &CrateContext, enum_definition: &ast::EnumDef,
}
}
}
enum_variant_size_lint(ccx, enum_definition, sp, id);
}
fn enum_variant_size_lint(ccx: &CrateContext, enum_def: &ast::EnumDef, sp: Span, id: ast::NodeId) {
let mut sizes = Vec::new(); // does no allocation if no pushes, thankfully
let (lvl, src) = ccx.tcx.node_lint_levels.borrow()
.find(&(id, lint::VariantSizeDifference))
.map_or((lint::Allow, lint::Default), |&(lvl,src)| (lvl, src));
if lvl != lint::Allow {
let avar = adt::represent_type(ccx, ty::node_id_to_type(ccx.tcx(), id));
match *avar {
adt::General(_, ref variants) => {
for var in variants.iter() {
let mut size = 0;
for field in var.fields.iter().skip(1) {
// skip the dicriminant
size += llsize_of_real(ccx, sizing_type_of(ccx, *field));
}
sizes.push(size);
}
},
_ => { /* its size is either constant or unimportant */ }
}
let (largest, slargest, largest_index) = sizes.iter().enumerate().fold((0, 0, 0),
|(l, s, li), (idx, &size)|
if size > l {
(size, l, idx)
} else if size > s {
(l, size, li)
} else {
(l, s, li)
}
);
// we only warn if the largest variant is at least thrice as large as
// the second-largest.
if largest > slargest * 3 && slargest > 0 {
lint::emit_lint(lvl, src,
format!("enum variant is more than three times larger \
({} bytes) than the next largest (ignoring padding)",
largest).as_slice(),
sp, lint::lint_to_str(lint::VariantSizeDifference), ccx.tcx());
ccx.sess().span_note(enum_def.variants.get(largest_index).span,
"this variant is the largest");
}
}
}
pub struct TransItemVisitor<'a> {
@ -1555,7 +1607,7 @@ pub fn trans_item(ccx: &CrateContext, item: &ast::Item) {
if !generics.is_type_parameterized() {
let vi = ty::enum_variants(ccx.tcx(), local_def(item.id));
let mut i = 0;
trans_enum_def(ccx, enum_definition, item.id, vi.as_slice(), &mut i);
trans_enum_def(ccx, enum_definition, item.span, item.id, vi.as_slice(), &mut i);
}
}
ast::ItemStatic(_, m, expr) => {

87
src/librustc/middle/ty.rs
View file

@ -14,6 +14,7 @@
use driver::session::Session;
use metadata::csearch;
use mc = middle::mem_categorization;
use middle::lint;
use middle::const_eval;
use middle::dependency_format;
use middle::lang_items::{ExchangeHeapLangItem, OpaqueStructLangItem};
@ -237,8 +238,8 @@ pub enum AutoRef {
/// generates so that so that it can be reused and doesn't have to be redone
/// later on.
pub struct ctxt {
// Specifically use a speedy hash algorithm for this hash map, it's used
// quite often.
/// Specifically use a speedy hash algorithm for this hash map, it's used
/// quite often.
pub interner: RefCell<FnvHashMap<intern_key, Box<t_box_>>>,
pub next_id: Cell<uint>,
pub sess: Session,
@ -248,24 +249,24 @@ pub struct ctxt {
pub region_maps: middle::region::RegionMaps,
// Stores the types for various nodes in the AST. Note that this table
// is not guaranteed to be populated until after typeck. See
// typeck::check::fn_ctxt for details.
/// Stores the types for various nodes in the AST. Note that this table
/// is not guaranteed to be populated until after typeck. See
/// typeck::check::fn_ctxt for details.
pub node_types: node_type_table,
// Stores the type parameters which were substituted to obtain the type
// of this node. This only applies to nodes that refer to entities
// param<eterized by type parameters, such as generic fns, types, or
// other items.
/// Stores the type parameters which were substituted to obtain the type
/// of this node. This only applies to nodes that refer to entities
/// param<eterized by type parameters, such as generic fns, types, or
/// other items.
pub item_substs: RefCell<NodeMap<ItemSubsts>>,
// Maps from a method to the method "descriptor"
/// Maps from a method to the method "descriptor"
pub methods: RefCell<DefIdMap<Rc<Method>>>,
// Maps from a trait def-id to a list of the def-ids of its methods
/// Maps from a trait def-id to a list of the def-ids of its methods
pub trait_method_def_ids: RefCell<DefIdMap<Rc<Vec<DefId>>>>,
// A cache for the trait_methods() routine
/// A cache for the trait_methods() routine
pub trait_methods_cache: RefCell<DefIdMap<Rc<Vec<Rc<Method>>>>>,
pub impl_trait_cache: RefCell<DefIdMap<Option<Rc<ty::TraitRef>>>>,
@ -287,64 +288,64 @@ pub struct ctxt {
pub adjustments: RefCell<NodeMap<AutoAdjustment>>,
pub normalized_cache: RefCell<HashMap<t, t>>,
pub lang_items: middle::lang_items::LanguageItems,
// A mapping of fake provided method def_ids to the default implementation
/// A mapping of fake provided method def_ids to the default implementation
pub provided_method_sources: RefCell<DefIdMap<ast::DefId>>,
pub supertraits: RefCell<DefIdMap<Rc<Vec<Rc<TraitRef>>>>>,
pub superstructs: RefCell<DefIdMap<Option<ast::DefId>>>,
pub struct_fields: RefCell<DefIdMap<Rc<Vec<field_ty>>>>,
// Maps from def-id of a type or region parameter to its
// (inferred) variance.
/// Maps from def-id of a type or region parameter to its
/// (inferred) variance.
pub item_variance_map: RefCell<DefIdMap<Rc<ItemVariances>>>,
// A mapping from the def ID of an enum or struct type to the def ID
// of the method that implements its destructor. If the type is not
// present in this map, it does not have a destructor. This map is
// populated during the coherence phase of typechecking.
/// A mapping from the def ID of an enum or struct type to the def ID
/// of the method that implements its destructor. If the type is not
/// present in this map, it does not have a destructor. This map is
/// populated during the coherence phase of typechecking.
pub destructor_for_type: RefCell<DefIdMap<ast::DefId>>,
// A method will be in this list if and only if it is a destructor.
/// A method will be in this list if and only if it is a destructor.
pub destructors: RefCell<DefIdSet>,
// Maps a trait onto a list of impls of that trait.
/// Maps a trait onto a list of impls of that trait.
pub trait_impls: RefCell<DefIdMap<Rc<RefCell<Vec<ast::DefId>>>>>,
// Maps a DefId of a type to a list of its inherent impls.
// Contains implementations of methods that are inherent to a type.
// Methods in these implementations don't need to be exported.
/// Maps a DefId of a type to a list of its inherent impls.
/// Contains implementations of methods that are inherent to a type.
/// Methods in these implementations don't need to be exported.
pub inherent_impls: RefCell<DefIdMap<Rc<RefCell<Vec<ast::DefId>>>>>,
// Maps a DefId of an impl to a list of its methods.
// Note that this contains all of the impls that we know about,
// including ones in other crates. It's not clear that this is the best
// way to do it.
/// Maps a DefId of an impl to a list of its methods.
/// Note that this contains all of the impls that we know about,
/// including ones in other crates. It's not clear that this is the best
/// way to do it.
pub impl_methods: RefCell<DefIdMap<Vec<ast::DefId>>>,
// Set of used unsafe nodes (functions or blocks). Unsafe nodes not
// present in this set can be warned about.
/// Set of used unsafe nodes (functions or blocks). Unsafe nodes not
/// present in this set can be warned about.
pub used_unsafe: RefCell<NodeSet>,
// Set of nodes which mark locals as mutable which end up getting used at
// some point. Local variable definitions not in this set can be warned
// about.
/// Set of nodes which mark locals as mutable which end up getting used at
/// some point. Local variable definitions not in this set can be warned
/// about.
pub used_mut_nodes: RefCell<NodeSet>,
// vtable resolution information for impl declarations
/// vtable resolution information for impl declarations
pub impl_vtables: typeck::impl_vtable_map,
// The set of external nominal types whose implementations have been read.
// This is used for lazy resolution of methods.
/// The set of external nominal types whose implementations have been read.
/// This is used for lazy resolution of methods.
pub populated_external_types: RefCell<DefIdSet>,
// The set of external traits whose implementations have been read. This
// is used for lazy resolution of traits.
/// The set of external traits whose implementations have been read. This
/// is used for lazy resolution of traits.
pub populated_external_traits: RefCell<DefIdSet>,
// Borrows
/// Borrows
pub upvar_borrow_map: RefCell<UpvarBorrowMap>,
// These two caches are used by const_eval when decoding external statics
// and variants that are found.
/// These two caches are used by const_eval when decoding external statics
/// and variants that are found.
pub extern_const_statics: RefCell<DefIdMap<Option<@ast::Expr>>>,
pub extern_const_variants: RefCell<DefIdMap<Option<@ast::Expr>>>,
@ -352,6 +353,9 @@ pub struct ctxt {
pub vtable_map: typeck::vtable_map,
pub dependency_formats: RefCell<dependency_format::Dependencies>,
pub node_lint_levels: RefCell<HashMap<(ast::NodeId, lint::Lint),
(lint::Level, lint::LintSource)>>,
}
pub enum tbox_flag {
@ -1134,6 +1138,7 @@ pub fn mk_ctxt(s: Session,
method_map: RefCell::new(FnvHashMap::new()),
vtable_map: RefCell::new(FnvHashMap::new()),
dependency_formats: RefCell::new(HashMap::new()),
node_lint_levels: RefCell::new(HashMap::new()),
}
}

42
src/test/run-pass/enum-size-variance.rs Normal file
View file

@ -0,0 +1,42 @@
// Copyright 2014 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.
//
// ignore-pretty
#![deny(enum_size_variance)]
#![allow(dead_code)]
enum Enum1 { }
enum Enum2 { A, B, C }
enum Enum3 { D(int), E, F }
enum Enum4 { H(int), I(int), J }
enum Enum5 { //~ ERROR three times larger
L(int, int, int, int), //~ NOTE this variant is the largest
M(int),
N
}
enum Enum6<T, U> {
O(T),
P(U),
Q(int)
}
#[allow(enum_size_variance)]
enum Enum7 {
R(int, int, int, int),
S(int),
T
}
pub fn main() { }