diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 8feacba6e006d..e577102c4fe3e 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -72,7 +72,7 @@ use syntax::parse::token; 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, @@ -93,6 +93,7 @@ pub enum Lint { UnknownFeatures, UnknownCrateType, UnsignedNegate, + VariantSizeDifference, ManagedHeapMemory, OwnedHeapMemory, @@ -146,8 +147,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 @@ -399,6 +401,13 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[ default: Warn }), + ("variant_size_difference", + LintSpec { + lint: VariantSizeDifference, + desc: "detects enums with widely varying variant sizes", + default: Allow, + }), + ("unused_must_use", LintSpec { lint: UnusedMustUse, @@ -437,30 +446,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> { @@ -492,7 +549,7 @@ impl<'a> Context<'a> { return *k; } } - fail!("unregistered lint {:?}", lint); + fail!("unregistered lint {}", lint); } fn span_lint(&self, lint: Lint, span: Span, msg: &str) { @@ -501,37 +558,8 @@ impl<'a> Context<'a> { 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); } /** @@ -610,8 +638,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. +/// 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) @@ -645,8 +673,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) @@ -1685,9 +1713,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); @@ -1878,6 +1921,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 @@ -1913,13 +1957,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; } diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index e208097e99b33..5f708b2b8bfc2 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -36,6 +36,7 @@ use lib::llvm::{ModuleRef, ValueRef, BasicBlockRef}; 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::foreign; 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; @@ -1539,7 +1540,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], + sp: Span, id: ast::NodeId, vi: &[Rc], i: &mut uint) { for &variant in enum_definition.variants.iter() { let disr_val = vi[*i].disr_val; @@ -1559,6 +1560,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> { @@ -1605,7 +1657,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) => { diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 64619b636a33a..3a7a94cdbcede 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -14,6 +14,7 @@ use back::svh::Svh; 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>>, pub next_id: Cell, 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>, - // Maps from a method to the method "descriptor" + /// Maps from a method to the method "descriptor" pub methods: RefCell>>, - // 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>>>, - // A cache for the trait_methods() routine + /// A cache for the trait_methods() routine pub trait_methods_cache: RefCell>>>>, pub impl_trait_cache: RefCell>>>, @@ -287,64 +288,64 @@ pub struct ctxt { pub adjustments: RefCell>, pub normalized_cache: RefCell>, 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>, pub supertraits: RefCell>>>>, pub superstructs: RefCell>>, pub struct_fields: RefCell>>>, - // 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>>, - // 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>, - // 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, - // 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>>>>, - // 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>>>>, - // 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>>, - // 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, - // 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, - // 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, - // 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, - // Borrows + /// Borrows pub upvar_borrow_map: RefCell, - // 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>>, pub extern_const_variants: RefCell>>, @@ -352,6 +353,9 @@ pub struct ctxt { pub vtable_map: typeck::vtable_map, pub dependency_formats: RefCell, + + pub node_lint_levels: RefCell>, } 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()), } } diff --git a/src/test/run-pass/enum-size-variance.rs b/src/test/run-pass/enum-size-variance.rs new file mode 100644 index 0000000000000..39ab8316958a6 --- /dev/null +++ b/src/test/run-pass/enum-size-variance.rs @@ -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 or the MIT license +// , 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 { + O(T), + P(U), + Q(int) +} + +#[allow(enum_size_variance)] +enum Enum7 { + R(int, int, int, int), + S(int), + T +} +pub fn main() { }