Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint on large non scalar const #5248

Merged
merged 2 commits into from
Apr 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,7 @@ Released 2018-09-13
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
[`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays
[`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups
[`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
Expand Down
85 changes: 85 additions & 0 deletions clippy_lints/src/large_const_arrays.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use crate::rustc_target::abi::LayoutOf;
use crate::utils::span_lint_and_then;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::interpret::ConstValue;
use rustc_middle::ty::{self, ConstKind};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{BytePos, Pos, Span};
use rustc_typeck::hir_ty_to_ty;

declare_clippy_lint! {
/// **What it does:** Checks for large `const` arrays that should
/// be defined as `static` instead.
///
/// **Why is this bad?** Performance: const variables are inlined upon use.
/// Static items result in only one instance and has a fixed location in memory.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// // Bad
/// pub const a = [0u32; 1_000_000];
///
/// // Good
/// pub static a = [0u32; 1_000_000];
/// ```
pub LARGE_CONST_ARRAYS,
perf,
"large non-scalar const array may cause performance overhead"
}

pub struct LargeConstArrays {
maximum_allowed_size: u64,
}

impl LargeConstArrays {
#[must_use]
pub fn new(maximum_allowed_size: u64) -> Self {
Self { maximum_allowed_size }
}
}

impl_lint_pass!(LargeConstArrays => [LARGE_CONST_ARRAYS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeConstArrays {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
if_chain! {
if !item.span.from_expansion();
if let ItemKind::Const(hir_ty, _) = &item.kind;
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
if let ty::Array(element_type, cst) = ty.kind;
if let ConstKind::Value(val) = cst.val;
if let ConstValue::Scalar(element_count) = val;
if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx);
if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes());
if self.maximum_allowed_size < element_count * element_size;

then {
let hi_pos = item.ident.span.lo() - BytePos::from_usize(1);
let sugg_span = Span::new(
hi_pos - BytePos::from_usize("const".len()),
hi_pos,
item.span.ctxt(),
);
span_lint_and_then(
cx,
LARGE_CONST_ARRAYS,
item.span,
"large array defined as const",
|db| {
db.span_suggestion(
sugg_span,
"make this a static item",
"static".to_string(),
Applicability::MachineApplicable,
);
}
);
}
}
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ mod inline_fn_without_body;
mod int_plus_one;
mod integer_division;
mod items_after_statements;
mod large_const_arrays;
mod large_enum_variant;
mod large_stack_arrays;
mod len_zero;
Expand Down Expand Up @@ -580,6 +581,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&int_plus_one::INT_PLUS_ONE,
&integer_division::INTEGER_DIVISION,
&items_after_statements::ITEMS_AFTER_STATEMENTS,
&large_const_arrays::LARGE_CONST_ARRAYS,
&large_enum_variant::LARGE_ENUM_VARIANT,
&large_stack_arrays::LARGE_STACK_ARRAYS,
&len_zero::LEN_WITHOUT_IS_EMPTY,
Expand Down Expand Up @@ -1024,6 +1026,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome);
let array_size_threshold = conf.array_size_threshold;
store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold));
store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic);
store.register_early_pass(|| box as_conversions::AsConversions);
store.register_early_pass(|| box utils::internal_lints::ProduceIce);
Expand Down Expand Up @@ -1222,6 +1225,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
LintId::of(&int_plus_one::INT_PLUS_ONE),
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
Expand Down Expand Up @@ -1651,6 +1655,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&bytecount::NAIVE_BYTECOUNT),
LintId::of(&entry::MAP_ENTRY),
LintId::of(&escape::BOXED_LOCAL),
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(&loops::MANUAL_MEMCPY),
LintId::of(&loops::NEEDLESS_COLLECT),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ define_Conf! {
(trivial_copy_size_limit, "trivial_copy_size_limit": Option<u64>, None),
/// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have
(too_many_lines_threshold, "too_many_lines_threshold": u64, 100),
/// Lint: LARGE_STACK_ARRAYS. The maximum allowed size for arrays on the stack
/// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack
(array_size_threshold, "array_size_threshold": u64, 512_000),
/// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed
(vec_box_size_threshold, "vec_box_size_threshold": u64, 4096),
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "non_expressive_names",
},
Lint {
name: "large_const_arrays",
group: "perf",
desc: "large non-scalar const array may cause performance overhead",
deprecation: None,
module: "large_const_arrays",
},
Lint {
name: "large_digit_groups",
group: "pedantic",
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/large_const_arrays.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// run-rustfix

#![warn(clippy::large_const_arrays)]
#![allow(dead_code)]

#[derive(Clone, Copy)]
pub struct S {
pub data: [u64; 32],
}

// Should lint
pub(crate) static FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000];
pub static FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
static FOO: [u32; 1_000_000] = [0u32; 1_000_000];

// Good
pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000];
pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000];
const G_FOO: [u32; 1_000] = [0u32; 1_000];

fn main() {
// Should lint
pub static BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
static BAR: [u32; 1_000_000] = [0u32; 1_000_000];
pub static BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000];
static BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000];
pub static BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000];
static BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000];

// Good
pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000];
const G_BAR: [u32; 1_000] = [0u32; 1_000];
pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500];
const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500];
pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200];
const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200];
}
37 changes: 37 additions & 0 deletions tests/ui/large_const_arrays.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// run-rustfix

#![warn(clippy::large_const_arrays)]
#![allow(dead_code)]

#[derive(Clone, Copy)]
pub struct S {
pub data: [u64; 32],
}

// Should lint
pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000];
pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
const FOO: [u32; 1_000_000] = [0u32; 1_000_000];

// Good
pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000];
pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000];
const G_FOO: [u32; 1_000] = [0u32; 1_000];

fn main() {
// Should lint
pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
const BAR: [u32; 1_000_000] = [0u32; 1_000_000];
pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000];
const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000];
pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000];
const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000];

// Good
pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000];
const G_BAR: [u32; 1_000] = [0u32; 1_000];
pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500];
const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500];
pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200];
const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200];
}
76 changes: 76 additions & 0 deletions tests/ui/large_const_arrays.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
error: large array defined as const
--> $DIR/large_const_arrays.rs:12:1
|
LL | pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000];
| ^^^^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`
|
= note: `-D clippy::large-const-arrays` implied by `-D warnings`

error: large array defined as const
--> $DIR/large_const_arrays.rs:13:1
|
LL | pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
| ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:14:1
|
LL | const FOO: [u32; 1_000_000] = [0u32; 1_000_000];
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:23:5
|
LL | pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000];
| ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:24:5
|
LL | const BAR: [u32; 1_000_000] = [0u32; 1_000_000];
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:25:5
|
LL | pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000];
| ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:26:5
|
LL | const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000];
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:27:5
|
LL | pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000];
| ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: large array defined as const
--> $DIR/large_const_arrays.rs:28:5
|
LL | const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000];
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: make this a static item: `static`

error: aborting due to 9 previous errors

8 changes: 4 additions & 4 deletions util/lintlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
group_re = re.compile(r'''\s*([a-z_][a-z_0-9]+)''')
conf_re = re.compile(r'''define_Conf! {\n([^}]*)\n}''', re.MULTILINE)
confvar_re = re.compile(
r'''/// Lint: (\w+)\. (.*)\n\s*\([^,]+,\s+"([^"]+)":\s+([^,]+),\s+([^\.\)]+).*\),''', re.MULTILINE)
r'''/// Lint: ([\w,\s]+)\. (.*)\n\s*\([^,]+,\s+"([^"]+)":\s+([^,]+),\s+([^\.\)]+).*\),''', re.MULTILINE)
comment_re = re.compile(r'''\s*/// ?(.*)''')

lint_levels = {
Expand Down Expand Up @@ -93,9 +93,9 @@ def parse_configs(path):
match = re.search(conf_re, contents)
confvars = re.findall(confvar_re, match.group(1))

for (lint, doc, name, ty, default) in confvars:
configs[lint.lower()] = Config(name.replace("_", "-"), ty, doc, default)

for (lints, doc, name, ty, default) in confvars:
for lint in lints.split(','):
configs[lint.strip().lower()] = Config(name.replace("_", "-"), ty, doc, default)
return configs


Expand Down