From 1938904fcd581e87c2be63d537fd402805627a0e Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 20:12:55 -0700 Subject: [PATCH] Change large_enum_variant to lint against size differences rather than size --- clippy_lints/src/large_enum_variant.rs | 90 +++++++++++++++----------- log | 1 - tests/ui/large_enum_variant.rs | 53 +++++++++------ 3 files changed, 87 insertions(+), 57 deletions(-) delete mode 100644 log diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index aa7db1f77e2..1363183cb81 100644 --- 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::>(); + + 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 index 0cbafc97889..00000000000 --- 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 5bbcb93910b..26ba883b1ba 100644 --- 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 { + A(i32), + B([T; 8000]), } -enum GenericEnum { +enum GenericEnum2 { 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 { 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 +} +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() { } -- 2.44.0