Add lint on large non scalar const

Author: ThibsG

CHANGELOG.md

@@ -1273,6 +1273,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

README.md

@@ -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 358 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 359 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:
 

clippy_lints/src/lib.rs

@@ -267,6 +267,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;
@@ -716,6 +717,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,
@@ -1000,6 +1002,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);
@@ -1294,6 +1297,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),
@@ -1633,6 +1637,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),

clippy_lints/src/non_scalar_const.rs

@@ -0,0 +1,89 @@
+use crate::rustc_target::abi::LayoutOf;
+use crate::utils::{snippet_opt, span_lint_and_then};
+use if_chain::if_chain;
+use rustc::mir::interpret::ConstValue;
+use rustc::ty::{self, ConstKind};
+use rustc_hir::{Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::InnerSpan;
+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 code
+    /// pub const a = [0u32; 1_000_000];
+    ///
+    /// // Good code
+    /// 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>, it: &'tcx Item<'_>) {
+        // No suggestion in macros
+        if it.span.from_expansion() {
+            return;
+        }
+        if_chain! {
+            if let ItemKind::Const(hir_ty, _) = &it.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 {
+                // We need to take care about the right "const" position inside the span
+                let mut start_sugg = 0;
+                if let Some(visi) = snippet_opt(cx, it.vis.span) {
+                    let len_visi = visi.len();
+                    if len_visi > 0 {
+                        start_sugg = len_visi + 1;
+                    }
+                }
+                let sugg_span = it.span.from_inner(
+                    InnerSpan::new(start_sugg, start_sugg + "const".len())
+                );
+                span_lint_and_then(
+                    cx,
+                    NON_SCALAR_CONST,
+                    sugg_span,
+                    "large array defined as const",
+                    |db| {
+                        db.span_label(
+                            it.span,
+                            "make this item static"
+                        );
+                    }
+                );
+            }
+        }
+    }
+}

src/lintlist/mod.rs

@@ -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; 358] = [
+pub const ALL_LINTS: [Lint; 359] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1456,6 +1456,13 @@ pub const ALL_LINTS: [Lint; 358] = [
         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",

tests/ui/non_scalar_const.rs

@@ -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];
+}

tests/ui/non_scalar_const.stderr

@@ -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];
+   | -----------^^^^^----------------------------------------------------- make this 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];
+   | ----^^^^^----------------------------------------------- make this 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];
+   | ^^^^^-------------------------------------------
+   | |
+   | make this 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];
+   |     ----^^^^^----------------------------------------------- make this 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];
+   |     ^^^^^-------------------------------------------
+   |     |
+   |     make this 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];
+   |     ----^^^^^----------------------------------------------------------- make this 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];
+   |     ^^^^^-------------------------------------------------------
+   |     |
+   |     make this 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];
+   |     ----^^^^^------------------------------------------------------------- make this 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];
+   |     ^^^^^---------------------------------------------------------
+   |     |
+   |     make this item static
+
+error: aborting due to 9 previous errors
+