Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c17ae44

Browse files
committedApr 10, 2020
Auto merge of #5446 - rust-lang:gimme-a-second, r=flip1995
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
2 parents 34763a5 + 89f6012 commit c17ae44

File tree

2 files changed

+14
-46
lines changed

2 files changed

+14
-46
lines changed
 

‎clippy_lints/src/large_enum_variant.rs

+13-21
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@ declare_clippy_lint! {
1212
/// `enum`s.
1313
///
1414
/// **Why is this bad?** Enum size is bounded by the largest variant. Having a
15-
/// large variant
16-
/// can penalize the memory layout of that enum.
15+
/// large variant can penalize the memory layout of that enum.
1716
///
18-
/// **Known problems:** None.
17+
/// **Known problems:** This lint obviously cannot take the distribution of
18+
/// variants in your running program into account. It is possible that the
19+
/// smaller variants make up less than 1% of all instances, in which case
20+
/// the overhead is negligible and the boxing is counter-productive. Always
21+
/// measure the change this lint suggests.
1922
///
2023
/// **Example:**
2124
/// ```rust
@@ -52,8 +55,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
5255
let ty = cx.tcx.type_of(did);
5356
let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
5457

55-
let mut smallest_variant: Option<(_, _)> = None;
5658
let mut largest_variant: Option<(_, _)> = None;
59+
let mut second_variant: Option<(_, _)> = None;
5760

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

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

72-
update_if(&mut smallest_variant, grouped, |a, b| b.0 <= a.0);
73-
update_if(&mut largest_variant, grouped, |a, b| b.0 >= a.0);
75+
if grouped.0 >= largest_variant.map_or(0, |x| x.0) {
76+
second_variant = largest_variant;
77+
largest_variant = Some(grouped);
78+
}
7479
}
7580

76-
if let (Some(smallest), Some(largest)) = (smallest_variant, largest_variant) {
77-
let difference = largest.0 - smallest.0;
81+
if let (Some(largest), Some(second)) = (largest_variant, second_variant) {
82+
let difference = largest.0 - second.0;
7883

7984
if difference > self.maximum_size_difference_allowed {
8085
let (i, variant) = largest.1;
@@ -114,16 +119,3 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
114119
}
115120
}
116121
}
117-
118-
fn update_if<T, F>(old: &mut Option<T>, new: T, f: F)
119-
where
120-
F: Fn(&T, &T) -> bool,
121-
{
122-
if let Some(ref mut val) = *old {
123-
if f(val, &new) {
124-
*val = new;
125-
}
126-
} else {
127-
*old = Some(new);
128-
}
129-
}

‎tests/ui/large_enum_variant.stderr

+1-25
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,6 @@ help: consider boxing the large fields to reduce the total size of the enum
1010
LL | B(Box<[i32; 8000]>),
1111
| ^^^^^^^^^^^^^^^^
1212

13-
error: large size difference between variants
14-
--> $DIR/large_enum_variant.rs:18:5
15-
|
16-
LL | C(T, [i32; 8000]),
17-
| ^^^^^^^^^^^^^^^^^
18-
|
19-
help: consider boxing the large fields to reduce the total size of the enum
20-
--> $DIR/large_enum_variant.rs:18:5
21-
|
22-
LL | C(T, [i32; 8000]),
23-
| ^^^^^^^^^^^^^^^^^
24-
2513
error: large size difference between variants
2614
--> $DIR/large_enum_variant.rs:31:5
2715
|
@@ -33,18 +21,6 @@ help: consider boxing the large fields to reduce the total size of the enum
3321
LL | ContainingLargeEnum(Box<LargeEnum>),
3422
| ^^^^^^^^^^^^^^
3523

36-
error: large size difference between variants
37-
--> $DIR/large_enum_variant.rs:34:5
38-
|
39-
LL | ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
40-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
41-
|
42-
help: consider boxing the large fields to reduce the total size of the enum
43-
--> $DIR/large_enum_variant.rs:34:5
44-
|
45-
LL | ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
46-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47-
4824
error: large size difference between variants
4925
--> $DIR/large_enum_variant.rs:41:5
5026
|
@@ -68,5 +44,5 @@ help: consider boxing the large fields to reduce the total size of the enum
6844
LL | StructLikeLarge2 { x: Box<[i32; 8000]> },
6945
| ^^^^^^^^^^^^^^^^
7046

71-
error: aborting due to 6 previous errors
47+
error: aborting due to 4 previous errors
7248

0 commit comments

Comments
 (0)
Please sign in to comment.