Change large_enum_variant to lint against size differences rather than size

diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs
index aa7db1f77e2c4d614e892922aec731227d40eedd..1363183cb810d0807973da0eee41be067a12a80e 100644 (file)
--- a/clippy_lints/src/large_enum_variant.rs
+++ b/clippy_lints/src/large_enum_variant.rs
@@ -1,4 +1,4 @@
-//! lint when there are large variants on an enum
+//! lint when there is a large size difference between variants on an enum
 
 use rustc::lint::*;
 use rustc::hir::*;
@@ -7,7 +7,7 @@
 use rustc::ty::TypeFoldable;
 use rustc::traits::Reveal;
 
-/// **What it does:** Checks for large variants on `enum`s.
+/// **What it does:** Checks for large size differences between variants on `enum`s.
 ///
 /// **Why is this bad?** Enum size is bounded by the largest variant. Having a large variant
 /// can penalize the memory layout of that enum.
@@ -24,17 +24,17 @@
 declare_lint! {
     pub LARGE_ENUM_VARIANT,
     Warn,
-    "large variants on an enum"
+    "large size difference between variants on an enum"
 }
 
 #[derive(Copy,Clone)]
 pub struct LargeEnumVariant {
-    maximum_variant_size_allowed: u64,
+    maximum_size_difference_allowed: u64,
 }
 
 impl LargeEnumVariant {
-    pub fn new(maximum_variant_size_allowed: u64) -> Self {
-        LargeEnumVariant { maximum_variant_size_allowed: maximum_variant_size_allowed }
+    pub fn new(maximum_size_difference_allowed: u64) -> Self {
+        LargeEnumVariant { maximum_size_difference_allowed: maximum_size_difference_allowed }
     }
 }
 
@@ -50,7 +50,11 @@ fn check_item(&mut self, cx: &LateContext, item: &Item) {
         if let ItemEnum(ref def, _) = item.node {
             let ty = cx.tcx.item_type(did);
             let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
-            for (i, variant) in adt.variants.iter().enumerate() {
+
+            let mut sizes = Vec::new();
+            let mut variants = Vec::new();
+
+            for variant in &adt.variants {
                 let data_layout = TargetDataLayout::parse(cx.sess());
                 cx.tcx.infer_ctxt((), Reveal::All).enter(|infcx| {
                     let size: u64 = variant.fields
@@ -68,39 +72,49 @@ fn check_item(&mut self, cx: &LateContext, item: &Item) {
                         })
                         .sum();
 
-                    use std::io::Write;
-                    let mut f = ::std::fs::File::create("log").unwrap();
-
-                    writeln!(f, "size, max size: {}, {}", size, self.maximum_variant_size_allowed).unwrap();
-                    if size > self.maximum_variant_size_allowed {
-                        writeln!(f, "size > max").unwrap();
-                        // panic!("foo");
-
-                        span_lint_and_then(cx,
-                                           LARGE_ENUM_VARIANT,
-                                           def.variants[i].span,
-                                           "large enum variant found",
-                                           |db| {
-                            if variant.fields.len() == 1 {
-                                let span = match def.variants[i].node.data {
-                                    VariantData::Struct(ref fields, _) |
-                                    VariantData::Tuple(ref fields, _) => fields[0].ty.span,
-                                    VariantData::Unit(_) => unreachable!(),
-                                };
-                                if let Some(snip) = snippet_opt(cx, span) {
-                                    db.span_suggestion(span,
-                                                       "consider boxing the large fields to reduce the total size of \
-                                                        the enum",
-                                                       format!("Box<{}>", snip));
-                                    return;
-                                }
-                            }
-                            db.span_help(def.variants[i].span,
-                                         "consider boxing the large fields to reduce the total size of the enum");
-                        });
-                    }
+                    sizes.push(size);
+                    variants.push(variant);
                 });
             }
+
+            let mut grouped = sizes.into_iter().zip(variants.into_iter().enumerate()).collect::<Vec<_>>();
+
+            grouped.sort_by_key(|g| g.0);
+
+            let smallest_variant = grouped.first();
+            let largest_variant = grouped.last();
+
+            if let (Some(smallest), Some(largest)) = (smallest_variant, largest_variant) {
+                let difference = largest.0 - smallest.0;
+
+                if difference > self.maximum_size_difference_allowed {
+                    let (i, variant) = largest.1;
+
+                    span_lint_and_then(cx,
+                                       LARGE_ENUM_VARIANT,
+                                       def.variants[i].span,
+                                       "large size difference between variants",
+                                       |db| {
+                        if variant.fields.len() == 1 {
+                            let span = match def.variants[i].node.data {
+                                VariantData::Struct(ref fields, _) |
+                                VariantData::Tuple(ref fields, _) => fields[0].ty.span,
+                                VariantData::Unit(_) => unreachable!(),
+                            };
+                            if let Some(snip) = snippet_opt(cx, span) {
+                                db.span_suggestion(span,
+                                                   "consider boxing the large fields to reduce the total size of the \
+                                                    enum",
+                                                   format!("Box<{}>", snip));
+                                return;
+                            }
+                        }
+                        db.span_help(def.variants[i].span,
+                                     "consider boxing the large fields to reduce the total size of the enum");
+                    });
+                }
+            }
+
         }
     }
 }

diff --git a/log b/log
deleted file mode 100644 (file)
index 0cbafc9..0000000
--- a/log
+++ /dev/null
@@ -1 +0,0 @@
-size, max size: 0, 200

diff --git a/tests/ui/large_enum_variant.rs b/tests/ui/large_enum_variant.rs
index 5bbcb93910b9e8c6a9a70c45e4f3d1abec1ffaab..26ba883b1baa6051c801d3af7ddb3ca1ea06725f 100644 (file)
--- a/tests/ui/large_enum_variant.rs
+++ b/tests/ui/large_enum_variant.rs
@@ -7,19 +7,21 @@
 
 enum LargeEnum {
     A(i32),
-    B([i32; 8000]),
-
+    B([i32; 8000]), //~ ERROR large size difference between variants
+    //~^ HELP consider boxing the large fields to reduce the total size of the enum
+    //~| SUGGESTION Box<[i32; 8000]>
+}
 
+enum GenericEnumOk<T> {
+    A(i32),
+    B([T; 8000]),
 }
 
-enum GenericEnum<T> {
+enum GenericEnum2<T> {
     A(i32),
     B([i32; 8000]),
-
-
-    C([T; 8000]),
-    D(T, [i32; 8000]),
-
+    C(T, [i32; 8000]), //~ ERROR large size difference between variants
+    //~^ HELP consider boxing the large fields to reduce the total size of the enum
 }
 
 trait SomeTrait {
@@ -30,24 +32,39 @@ enum LargeEnumGeneric<A: SomeTrait> {
     Var(A::Item), // regression test, this used to ICE
 }
 
-enum AnotherLargeEnum {
+enum LargeEnum2 {
     VariantOk(i32, u32),
-    ContainingLargeEnum(LargeEnum),
-
-
-    ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
-
+    ContainingLargeEnum(LargeEnum), //~ ERROR large size difference between variants
+    //~^ HELP consider boxing the large fields to reduce the total size of the enum
+    //~| SUGGESTION Box<LargeEnum>
+}
+enum LargeEnum3 {
+    ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]), //~ ERROR large size difference between variants
+    //~^ HELP consider boxing the large fields to reduce the total size of the enum
     VoidVariant,
     StructLikeLittle { x: i32, y: i32 },
-    StructLikeLarge { x: [i32; 8000], y: i32 },
+}
 
-    StructLikeLarge2 {
-        x:
-        [i32; 8000]
+enum LargeEnum4 {
+    VariantOk(i32, u32),
+    StructLikeLarge { x: [i32; 8000], y: i32 }, //~ ERROR large size difference between variants
+    //~^ HELP consider boxing the large fields to reduce the total size of the enum
+}
 
+enum LargeEnum5 {
+    VariantOk(i32, u32),
+    StructLikeLarge2 { //~ ERROR large size difference between variants
+        x:
+        [i32; 8000] //~ SUGGESTION Box<[i32; 8000]>
+        //~^ HELP consider boxing the large fields to reduce the total size of the enum
     },
 }
 
+enum LargeEnumOk {
+    LargeA([i32; 8000]),
+    LargeB([i32; 8001]),
+}
+
 fn main() {
 
 }