Skip to content

Commit

Permalink
Auto merge of #5446 - rust-lang:gimme-a-second, r=flip1995
Browse files Browse the repository at this point in the history
compare with the second largest instead of the smallest variant

This should make the lint less noisy for now. See [my comment](#5418 (comment)) to issue #5418.

---

changelog: none
  • Loading branch information
bors committed Apr 10, 2020
2 parents ba7076f + 89f6012 commit 0353f21
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 46 deletions.
34 changes: 13 additions & 21 deletions clippy_lints/src/large_enum_variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ declare_clippy_lint! {
/// `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.
/// large variant can penalize the memory layout of that enum.
///
/// **Known problems:** None.
/// **Known problems:** This lint obviously cannot take the distribution of
/// variants in your running program into account. It is possible that the
/// smaller variants make up less than 1% of all instances, in which case
/// the overhead is negligible and the boxing is counter-productive. Always
/// measure the change this lint suggests.
///
/// **Example:**
/// ```rust
Expand Down Expand Up @@ -52,8 +55,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
let ty = cx.tcx.type_of(did);
let adt = ty.ty_adt_def().expect("already checked whether this is an enum");

let mut smallest_variant: Option<(_, _)> = None;
let mut largest_variant: Option<(_, _)> = None;
let mut second_variant: Option<(_, _)> = None;

for (i, variant) in adt.variants.iter().enumerate() {
let size: u64 = variant
Expand All @@ -69,12 +72,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {

let grouped = (size, (i, variant));

update_if(&mut smallest_variant, grouped, |a, b| b.0 <= a.0);
update_if(&mut largest_variant, grouped, |a, b| b.0 >= a.0);
if grouped.0 >= largest_variant.map_or(0, |x| x.0) {
second_variant = largest_variant;
largest_variant = Some(grouped);
}
}

if let (Some(smallest), Some(largest)) = (smallest_variant, largest_variant) {
let difference = largest.0 - smallest.0;
if let (Some(largest), Some(second)) = (largest_variant, second_variant) {
let difference = largest.0 - second.0;

if difference > self.maximum_size_difference_allowed {
let (i, variant) = largest.1;
Expand Down Expand Up @@ -114,16 +119,3 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
}
}
}

fn update_if<T, F>(old: &mut Option<T>, new: T, f: F)
where
F: Fn(&T, &T) -> bool,
{
if let Some(ref mut val) = *old {
if f(val, &new) {
*val = new;
}
} else {
*old = Some(new);
}
}
26 changes: 1 addition & 25 deletions tests/ui/large_enum_variant.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,6 @@ help: consider boxing the large fields to reduce the total size of the enum
LL | B(Box<[i32; 8000]>),
| ^^^^^^^^^^^^^^^^

error: large size difference between variants
--> $DIR/large_enum_variant.rs:18:5
|
LL | C(T, [i32; 8000]),
| ^^^^^^^^^^^^^^^^^
|
help: consider boxing the large fields to reduce the total size of the enum
--> $DIR/large_enum_variant.rs:18:5
|
LL | C(T, [i32; 8000]),
| ^^^^^^^^^^^^^^^^^

error: large size difference between variants
--> $DIR/large_enum_variant.rs:31:5
|
Expand All @@ -33,18 +21,6 @@ help: consider boxing the large fields to reduce the total size of the enum
LL | ContainingLargeEnum(Box<LargeEnum>),
| ^^^^^^^^^^^^^^

error: large size difference between variants
--> $DIR/large_enum_variant.rs:34:5
|
LL | ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider boxing the large fields to reduce the total size of the enum
--> $DIR/large_enum_variant.rs:34:5
|
LL | ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: large size difference between variants
--> $DIR/large_enum_variant.rs:41:5
|
Expand All @@ -68,5 +44,5 @@ help: consider boxing the large fields to reduce the total size of the enum
LL | StructLikeLarge2 { x: Box<[i32; 8000]> },
| ^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors
error: aborting due to 4 previous errors

0 comments on commit 0353f21

Please sign in to comment.