Rollup merge of #5248 - ThibsG:ConstValues, r=flip1995

flip1995 · web-flow · commit ceea3c6a35bf · 2020-04-15T20:12:28.000+02:00
Add lint on large non scalar const This PR adds the new lint `non_scalar_const` that aims to warn against `const` declaration of large arrays. For performance, because of inlining, large arrays should be preferably declared as `static`. Note: i made this one to warn on all const arrays, whether they are in a body function or not. I don't know if this is really necessary, i could just reduce this lint to variables out of function scope. Fixes: #400 changelog: add new lint for large non-scalar types declared as const
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1320,6 +1320,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
@@ -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,
+                        );
+                    }
+                );
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -232,6 +232,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;
@@ -582,6 +583,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,
@@ -1026,6 +1028,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);
@@ -1225,6 +1228,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),
@@ -1656,6 +1660,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/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -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),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -948,6 +948,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",
diff --git a/tests/ui/large_const_arrays.fixed 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
@@ -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
@@ -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
+
diff --git a/util/lintlib.py b/util/lintlib.py
@@ -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 = {
@@ -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
 
 
