New large_enum_variants linter

Added linter to detect large variants and suggesting to box them

Author: gnieto

CHANGELOG.md

@@ -193,6 +193,7 @@ All notable changes to this project will be documented in this file.
 [`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop
 [`enum_clike_unportable_variant`]: https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant
 [`enum_glob_use`]: https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use
+[`enum_large_variant`]: https://github.com/Manishearth/rust-clippy/wiki#enum_large_variant
 [`enum_variant_names`]: https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names
 [`eq_op`]: https://github.com/Manishearth/rust-clippy/wiki#eq_op
 [`eval_order_dependence`]: https://github.com/Manishearth/rust-clippy/wiki#eval_order_dependence

README.md

@@ -17,7 +17,7 @@ Table of contents:
 
 ## Lints
 
-There are 166 lints included in this crate:
+There are 167 lints included in this crate:
 
 name                                                                                                                 | default | triggers on
 ---------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -55,6 +55,7 @@ name
 [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop)                                             | warn    | empty `loop {}`, which should block or sleep
 [enum_clike_unportable_variant](https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant)       | warn    | C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`
 [enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use)                                       | allow   | use items that import all variants of an enum
+[enum_large_variant](https://github.com/Manishearth/rust-clippy/wiki#enum_large_variant)                             | warn    | large variants on an enum
 [enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names)                             | warn    | enums where all variants share a prefix/postfix
 [eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op)                                                       | warn    | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
 [eval_order_dependence](https://github.com/Manishearth/rust-clippy/wiki#eval_order_dependence)                       | warn    | whether a variable read occurs before a write depends on sub-expression evaluation order

clippy_lints/src/enum_large_variant.rs

@@ -0,0 +1,71 @@
+//! lint when there are large variants on an enum
+
+use rustc::lint::*;
+use rustc::hir::*;
+use utils::span_help_and_lint;
+use rustc::ty::layout::TargetDataLayout;
+
+/// **What it does:** Checks for large variants on enums.
+///
+/// **Why is this bad?** Enum size is bounded by the largest variant. Having a large variant
+/// can penalize the memory layout of that enum.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// enum Test {
+///    A(i32),
+///    B([i32; 8000]),
+/// }
+/// ```
+declare_lint! {
+    pub ENUM_LARGE_VARIANT,
+    Warn,
+    "large variants on an enum"
+}
+
+#[derive(Copy,Clone)]
+pub struct EnumLargeVariant {
+    maximum_variant_size_allowed: u64,
+}
+
+impl EnumLargeVariant {
+    pub fn new(maximum_variant_size_allowed: u64) -> Self {
+        EnumLargeVariant {
+            maximum_variant_size_allowed: maximum_variant_size_allowed,
+        }
+    }
+}
+
+impl LintPass for EnumLargeVariant {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(ENUM_LARGE_VARIANT)
+    }
+}
+
+impl LateLintPass for EnumLargeVariant {
+    fn check_variant(&mut self, cx: &LateContext, variant: &Variant, _ :&Generics) {
+        let data_layout = TargetDataLayout::parse(cx.sess());
+        let param_env = cx.tcx.empty_parameter_environment();
+        let infcx = cx.tcx.borrowck_fake_infer_ctxt(param_env);
+        let mut variant_size = 0;
+
+        for field in variant.node.data.fields() {
+            let ty = cx.tcx.node_id_to_type(field.id);
+            if let Ok(layout) = ty.layout(&infcx) {
+                variant_size += layout.size(&data_layout).bytes();
+            }
+        }
+
+        if variant_size > self.maximum_variant_size_allowed {
+            span_help_and_lint(
+                cx,
+                ENUM_LARGE_VARIANT,
+                variant.span,
+                &format!("large enum variant found on variant `{}`", variant.node.name),
+                "consider boxing the large branches to reduce the total size of the enum",
+            );
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -74,6 +74,7 @@ pub mod drop_ref;
 pub mod entry;
 pub mod enum_clike;
 pub mod enum_glob_use;
+pub mod enum_large_variant;
 pub mod enum_variants;
 pub mod eq_op;
 pub mod escape;
@@ -260,6 +261,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box assign_ops::AssignOps);
     reg.register_late_lint_pass(box let_if_seq::LetIfSeq);
     reg.register_late_lint_pass(box eval_order_dependence::EvalOrderDependence);
+    reg.register_late_lint_pass(box enum_large_variant::EnumLargeVariant::new(conf.enum_variant_size_threshold));
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -326,6 +328,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         drop_ref::DROP_REF,
         entry::MAP_ENTRY,
         enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
+        enum_large_variant::ENUM_LARGE_VARIANT,
         enum_variants::ENUM_VARIANT_NAMES,
         eq_op::EQ_OP,
         escape::BOXED_LOCAL,

clippy_lints/src/utils/conf.rs

@@ -162,6 +162,8 @@ define_Conf! {
     ("too-large-for-stack", too_large_for_stack, 200 => u64),
     /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger
     ("enum-variant-name-threshold", enum_variant_name_threshold, 3 => u64),
+    /// Lint: ENUM_LARGE_VARIANT. The maximum size of a emum's variant to avoid box suggestion
+    ("enum-variant-size-threshold", enum_variant_size_threshold, 200 => u64),
 }
 
 /// Read the `toml` configuration file. The function will ignore “File not found” errors iif

tests/compile-fail/enum_large_variant.rs

@@ -0,0 +1,24 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![allow(dead_code)]
+#![allow(unused_variables)]
+#![deny(enum_large_variant)]
+
+enum LargeEnum {
+    A(i32),
+    B([i32; 8000]), //~ ERROR large enum variant found on variant `B`
+}
+
+enum AnotherLargeEnum {
+    VariantOk(i32, u32),
+    ContainingLargeEnum(LargeEnum), //~ ERROR large enum variant found on variant `ContainingLargeEnum`
+    ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]), //~ ERROR large enum variant found on variant `ContainingMoreThanOneField`
+    VoidVariant,
+    StructLikeLittle { x: i32, y: i32 },
+    StructLikeLarge { x: [i32; 8000], y: i32 }, //~ ERROR large enum variant found on variant `StructLikeLarge`
+}
+
+fn main() {
+
+}