From 380f7218b3876225e726617531d9f02d42da0e38 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sat, 29 Feb 2020 18:41:18 +0100 Subject: [PATCH] Add lint on large const arrays --- CHANGELOG.md | 1 + clippy_lints/src/large_const_arrays.rs | 85 ++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ src/lintlist/mod.rs | 7 +++ tests/ui/large_const_arrays.fixed | 37 +++++++++++ tests/ui/large_const_arrays.rs | 37 +++++++++++ tests/ui/large_const_arrays.stderr | 76 +++++++++++++++++++++++ 7 files changed, 247 insertions(+) create mode 100644 clippy_lints/src/large_const_arrays.rs create mode 100644 tests/ui/large_const_arrays.fixed create mode 100644 tests/ui/large_const_arrays.rs create mode 100644 tests/ui/large_const_arrays.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac3cace204..cbc7d98c8782 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs new file mode 100644 index 000000000000..2f62e6ad9dc5 --- /dev/null +++ b/clippy_lints/src/large_const_arrays.rs @@ -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` 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, + ); + } + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b106113c2a98..909115f7a3a9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -580,6 +580,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, @@ -1024,6 +1025,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); @@ -1222,6 +1224,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), @@ -1651,6 +1654,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), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b3c77f3f4814..771b6d49634c 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -941,6 +941,13 @@ pub static ref ALL_LINTS: Vec = 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", diff --git a/tests/ui/large_const_arrays.fixed b/tests/ui/large_const_arrays.fixed new file mode 100644 index 000000000000..c5af07c8a172 --- /dev/null +++ b/tests/ui/large_const_arrays.fixed @@ -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]; +} diff --git a/tests/ui/large_const_arrays.rs b/tests/ui/large_const_arrays.rs new file mode 100644 index 000000000000..a160b9f8ad5b --- /dev/null +++ b/tests/ui/large_const_arrays.rs @@ -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]; +} diff --git a/tests/ui/large_const_arrays.stderr b/tests/ui/large_const_arrays.stderr new file mode 100644 index 000000000000..3fb0acbca67d --- /dev/null +++ b/tests/ui/large_const_arrays.stderr @@ -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 +