Skip to content

Commit 4a0e6c8

Browse files
committed
New large_enum_variants linter
Added linter to detect large variants and suggesting to box them
1 parent 0474fe2 commit 4a0e6c8

File tree

6 files changed

+103
-1
lines changed

6 files changed

+103
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ All notable changes to this project will be documented in this file.
193193
[`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop
194194
[`enum_clike_unportable_variant`]: https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant
195195
[`enum_glob_use`]: https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use
196+
[`enum_large_variant`]: https://github.com/Manishearth/rust-clippy/wiki#enum_large_variant
196197
[`enum_variant_names`]: https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names
197198
[`eq_op`]: https://github.com/Manishearth/rust-clippy/wiki#eq_op
198199
[`eval_order_dependence`]: https://github.com/Manishearth/rust-clippy/wiki#eval_order_dependence

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Table of contents:
1717

1818
## Lints
1919

20-
There are 166 lints included in this crate:
20+
There are 167 lints included in this crate:
2121

2222
name | default | triggers on
2323
---------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -55,6 +55,7 @@ name
5555
[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}`, which should block or sleep
5656
[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`
5757
[enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | use items that import all variants of an enum
58+
[enum_large_variant](https://github.com/Manishearth/rust-clippy/wiki#enum_large_variant) | warn | large variants on an enum
5859
[enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | enums where all variants share a prefix/postfix
5960
[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`)
6061
[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
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//! lint when there are large variants on an enum
2+
3+
use rustc::lint::*;
4+
use rustc::hir::*;
5+
use utils::span_help_and_lint;
6+
use rustc::ty::layout::TargetDataLayout;
7+
8+
/// **What it does:** Checks for large variants on enums.
9+
///
10+
/// **Why is this bad?** Enum size is bounded by the largest variant. Having a large variant
11+
/// can penalize the memory layout of that enum.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
/// ```rust
17+
/// enum Test {
18+
/// A(i32),
19+
/// B([i32; 8000]),
20+
/// }
21+
/// ```
22+
declare_lint! {
23+
pub ENUM_LARGE_VARIANT,
24+
Warn,
25+
"large variants on an enum"
26+
}
27+
28+
#[derive(Copy,Clone)]
29+
pub struct EnumLargeVariant {
30+
maximum_variant_size_allowed: u64,
31+
}
32+
33+
impl EnumLargeVariant {
34+
pub fn new(maximum_variant_size_allowed: u64) -> Self {
35+
EnumLargeVariant {
36+
maximum_variant_size_allowed: maximum_variant_size_allowed,
37+
}
38+
}
39+
}
40+
41+
impl LintPass for EnumLargeVariant {
42+
fn get_lints(&self) -> LintArray {
43+
lint_array!(ENUM_LARGE_VARIANT)
44+
}
45+
}
46+
47+
impl LateLintPass for EnumLargeVariant {
48+
fn check_variant(&mut self, cx: &LateContext, variant: &Variant, _ :&Generics) {
49+
let data_layout = TargetDataLayout::parse(cx.sess());
50+
let param_env = cx.tcx.empty_parameter_environment();
51+
let infcx = cx.tcx.borrowck_fake_infer_ctxt(param_env);
52+
let mut variant_size = 0;
53+
54+
for field in variant.node.data.fields() {
55+
let ty = cx.tcx.node_id_to_type(field.id);
56+
if let Ok(layout) = ty.layout(&infcx) {
57+
variant_size += layout.size(&data_layout).bytes();
58+
}
59+
}
60+
61+
if variant_size > self.maximum_variant_size_allowed {
62+
span_help_and_lint(
63+
cx,
64+
ENUM_LARGE_VARIANT,
65+
variant.span,
66+
&format!("large enum variant found on variant `{}`", variant.node.name),
67+
"consider boxing the large branches to reduce the total size of the enum",
68+
);
69+
}
70+
}
71+
}

clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ pub mod drop_ref;
7474
pub mod entry;
7575
pub mod enum_clike;
7676
pub mod enum_glob_use;
77+
pub mod enum_large_variant;
7778
pub mod enum_variants;
7879
pub mod eq_op;
7980
pub mod escape;
@@ -260,6 +261,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
260261
reg.register_late_lint_pass(box assign_ops::AssignOps);
261262
reg.register_late_lint_pass(box let_if_seq::LetIfSeq);
262263
reg.register_late_lint_pass(box eval_order_dependence::EvalOrderDependence);
264+
reg.register_late_lint_pass(box enum_large_variant::EnumLargeVariant::new(conf.enum_variant_size_threshold));
263265

264266
reg.register_lint_group("clippy_restrictions", vec![
265267
arithmetic::FLOAT_ARITHMETIC,
@@ -326,6 +328,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
326328
drop_ref::DROP_REF,
327329
entry::MAP_ENTRY,
328330
enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
331+
enum_large_variant::ENUM_LARGE_VARIANT,
329332
enum_variants::ENUM_VARIANT_NAMES,
330333
eq_op::EQ_OP,
331334
escape::BOXED_LOCAL,

clippy_lints/src/utils/conf.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ define_Conf! {
162162
("too-large-for-stack", too_large_for_stack, 200 => u64),
163163
/// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger
164164
("enum-variant-name-threshold", enum_variant_name_threshold, 3 => u64),
165+
/// Lint: ENUM_LARGE_VARIANT. The maximum size of a emum's variant to avoid box suggestion
166+
("enum-variant-size-threshold", enum_variant_size_threshold, 200 => u64),
165167
}
166168

167169
/// Read the `toml` configuration file. The function will ignore “File not found” errors iif
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#![feature(plugin)]
2+
#![plugin(clippy)]
3+
4+
#![allow(dead_code)]
5+
#![allow(unused_variables)]
6+
#![deny(enum_large_variant)]
7+
8+
enum LargeEnum {
9+
A(i32),
10+
B([i32; 8000]), //~ ERROR large enum variant found on variant `B`
11+
}
12+
13+
enum AnotherLargeEnum {
14+
VariantOk(i32, u32),
15+
ContainingLargeEnum(LargeEnum), //~ ERROR large enum variant found on variant `ContainingLargeEnum`
16+
ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]), //~ ERROR large enum variant found on variant `ContainingMoreThanOneField`
17+
VoidVariant,
18+
StructLikeLittle { x: i32, y: i32 },
19+
StructLikeLarge { x: [i32; 8000], y: i32 }, //~ ERROR large enum variant found on variant `StructLikeLarge`
20+
}
21+
22+
fn main() {
23+
24+
}

0 commit comments

Comments
 (0)