---

yaml
---
r: 152037
b: refs/heads/try2
c: c327080
h: refs/heads/master
i:
  152035: 44bbc88
v: v3

Author: emberian

[refs]

@@ -5,7 +5,7 @@ refs/heads/snap-stage3: 78a7676898d9f80ab540c6df5d4c9ce35bb50463
 refs/heads/try: 519addf6277dbafccbb4159db4b710c37eaa2ec5
 refs/tags/release-0.1: 1f5c5126e96c79d22cb7862f75304136e204f105
 refs/heads/ndm: f3868061cd7988080c30d6d5bf352a5a5fe2460b
-refs/heads/try2: f122ad08a57d3e250f0c8c5036327eaa320dd4eb
+refs/heads/try2: c327080ee04e641a34f30ae71b713a91106680b1
 refs/heads/dist-snap: ba4081a5a8573875fed17545846f6f6902c8ba8d
 refs/tags/release-0.2: c870d2dffb391e14efb05aa27898f1f6333a9596
 refs/tags/release-0.3: b5f0d0f648d9a6153664837026ba1be43d3e2503

branches/try2/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<ast::NodeId, (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);
     }
 
     /**
@@ -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;
 }

branches/try2/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<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;
@@ -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) => {

branches/try2/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<dependency_format::Dependencies>,
+
+    pub enum_lint_levels: RefCell<HashMap<ast::NodeId, (lint::Level, lint::LintSource)>>,
 }
 
 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()),
     }
 }
 

branches/try2/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 <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() { }