diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 88ded969ad5d..7dc3db0a5d1a 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)] 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, @@ -461,6 +470,54 @@ struct Context<'a> { /// Ids of structs/enums which have been checked for raw_pointer_deriving checked_raw_pointers: NodeSet, + + /// Level of EnumSizeVariance lint for each enum, stored here because the + /// body of the lint needs to run in trans. + enum_levels: HashMap, +} + +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); } /** @@ -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(), + enum_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.enum_lint_levels.borrow_mut() = cx.enum_levels; } diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index e208097e99b3..5f708b2b8bfc 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 62679fa222bf..1ce1eb0a82a6 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}; @@ -352,6 +353,8 @@ pub struct ctxt { pub vtable_map: typeck::vtable_map, pub dependency_formats: RefCell, + + pub enum_lint_levels: RefCell>, } pub enum tbox_flag { @@ -1134,6 +1137,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()), + enum_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 000000000000..39ab8316958a --- /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() { }