Skip to content

Commit

Permalink
Add lint on large non scalar const
Browse files Browse the repository at this point in the history
  • Loading branch information
ThibsG committed Mar 14, 2020
1 parent 8fd7e31 commit 946484a
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ Released 2018-09-13
[`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
[`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
[`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
[`non_scalar_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_scalar_const
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ pub mod new_without_default;
pub mod no_effect;
pub mod non_copy_const;
pub mod non_expressive_names;
pub mod non_scalar_const;
pub mod open_options;
pub mod option_env_unwrap;
pub mod overflow_check_conditional;
Expand Down Expand Up @@ -722,6 +723,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS,
&non_expressive_names::MANY_SINGLE_CHAR_NAMES,
&non_expressive_names::SIMILAR_NAMES,
&non_scalar_const::NON_SCALAR_CONST,
&open_options::NONSENSICAL_OPEN_OPTIONS,
&option_env_unwrap::OPTION_ENV_UNWRAP,
&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
Expand Down Expand Up @@ -1007,6 +1009,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 non_scalar_const::NonScalarConst::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 @@ -1305,6 +1308,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
LintId::of(&non_scalar_const::NON_SCALAR_CONST),
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
Expand Down Expand Up @@ -1646,6 +1650,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::SINGLE_CHAR_PATTERN),
LintId::of(&misc::CMP_OWNED),
LintId::of(&mutex_atomic::MUTEX_ATOMIC),
LintId::of(&non_scalar_const::NON_SCALAR_CONST),
LintId::of(&redundant_clone::REDUNDANT_CLONE),
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
Expand Down
85 changes: 85 additions & 0 deletions clippy_lints/src/non_scalar_const.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::mir::interpret::ConstValue;
use rustc::ty::{self, ConstKind};
use rustc_errors::Applicability;
use rustc_hir::{Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
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` non-scalar types (ie: array) 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 NON_SCALAR_CONST,
perf,
"large non-scalar const variable may cause performance overhead"
}

pub struct NonScalarConst {
maximum_allowed_size: u64,
}

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

impl_lint_pass!(NonScalarConst => [NON_SCALAR_CONST]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonScalarConst {
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,
NON_SCALAR_CONST,
sugg_span,
"large array defined as const",
|db| {
db.span_suggestion(
item.span,
"make this a static item",
"static".to_string(),
Applicability::MachineApplicable,
);
}
);
}
}
}
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 361] = [
pub const ALL_LINTS: [Lint; 362] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1463,6 +1463,13 @@ pub const ALL_LINTS: [Lint; 361] = [
deprecation: None,
module: "unicode",
},
Lint {
name: "non_scalar_const",
group: "perf",
desc: "large non-scalar const variable may cause performance overhead",
deprecation: None,
module: "non_scalar_const",
},
Lint {
name: "nonminimal_bool",
group: "complexity",
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/non_scalar_const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![warn(clippy::non_scalar_const)]

#[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];
}
66 changes: 66 additions & 0 deletions tests/ui/non_scalar_const.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
error: large array defined as const
--> $DIR/non_scalar_const.rs:9:12
|
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::non-scalar-const` implied by `-D warnings`

error: large array defined as const
--> $DIR/non_scalar_const.rs:10:5
|
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/non_scalar_const.rs:11: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/non_scalar_const.rs:20:9
|
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/non_scalar_const.rs:21: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/non_scalar_const.rs:22:9
|
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/non_scalar_const.rs:23: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/non_scalar_const.rs:24:9
|
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/non_scalar_const.rs:25: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

0 comments on commit 946484a

Please sign in to comment.