Skip to content

Commit

Permalink
Auto merge of #13485 - GnomedDev:reduce-large-threshold, r=xFrednet
Browse files Browse the repository at this point in the history
Reduce default 'large array' threshold

As-is this threshold is `512kb`, but as #9449 points out this is way too high for most people to consider sensible (why would you want to copy `256kb` of data around on the stack or duplicate it via `const`) and didn't get any discussion when originally added. This PR reduces it the threshold to `1kb`, which is higher than the issue says ("a few cpu words") but helps out for actual codebases.

While reducing this, I found that `large_stack_arrays` was triggering for statically promoted arrays in constants/statics, so I also fixed that up as seen in the difference to [array_size_threshold.stderr](https://github.com/rust-lang/rust-clippy/compare/master...GnomedDev:rust-clippy:reduce-large-threshold?expand=1#diff-4c2a2a855d9ff7777f1d385be0c1bede2a3fc8aaab94837cde27a35235233fc7).

Closes #9449.

changelog: [`large_stack_arrays`]: No longer triggers in `static`/`const` context
changelog: [`large_const_arrays`]: Changed the default of [`array-size-threshold`] from `512kb` to `16kb`
  • Loading branch information
bors committed Oct 6, 2024
2 parents 753629b + 995eb95 commit 2d2e8cc
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 76 deletions.
2 changes: 1 addition & 1 deletion book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
## `array-size-threshold`
The maximum allowed size for arrays on the stack

**Default Value:** `512000`
**Default Value:** `16384`

---
**Affected lints:**
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ define_Conf! {
arithmetic_side_effects_allowed_unary: Vec<String> = <_>::default(),
/// The maximum allowed size for arrays on the stack
#[lints(large_const_arrays, large_stack_arrays)]
array_size_threshold: u64 = 512_000,
array_size_threshold: u64 = 16 * 1024,
/// Suppress lints whenever the suggested change would cause breakage for other crates.
#[lints(
box_collection,
Expand Down
17 changes: 16 additions & 1 deletion clippy_lints/src/large_stack_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ declare_clippy_lint! {
pub struct LargeStackArrays {
maximum_allowed_size: u64,
prev_vec_macro_callsite: Option<Span>,
is_in_const_item: bool,
}

impl LargeStackArrays {
pub fn new(conf: &'static Conf) -> Self {
Self {
maximum_allowed_size: conf.array_size_threshold,
prev_vec_macro_callsite: None,
is_in_const_item: false,
}
}

Expand All @@ -60,8 +62,21 @@ impl LargeStackArrays {
impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);

impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if matches!(item.kind, ItemKind::Static(..) | ItemKind::Const(..)) {
self.is_in_const_item = true;
}
}

fn check_item_post(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if matches!(item.kind, ItemKind::Static(..) | ItemKind::Const(..)) {
self.is_in_const_item = false;
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
if !self.is_in_const_item
&& let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
&& !self.is_from_vec_macro(cx, expr.span)
&& let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind()
&& let ConstKind::Value(_, ty::ValTree::Leaf(element_count)) = cst.kind()
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5182,7 +5182,7 @@ impl ShouldImplTraitCase {
}

#[rustfmt::skip]
const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
static TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
ShouldImplTraitCase::new("std::ops::Add", "add", 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
ShouldImplTraitCase::new("std::convert::AsMut", "as_mut", 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
ShouldImplTraitCase::new("std::convert::AsRef", "as_ref", 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
Expand Down
14 changes: 3 additions & 11 deletions tests/ui-toml/array_size_threshold/array_size_threshold.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,15 @@ LL | const ABOVE: [u8; 11] = [0; 11];
= note: `-D clippy::large-const-arrays` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::large_const_arrays)]`

error: allocating a local array larger than 10 bytes
--> tests/ui-toml/array_size_threshold/array_size_threshold.rs:4:25
|
LL | const ABOVE: [u8; 11] = [0; 11];
| ^^^^^^^
|
= help: consider allocating on the heap with `vec![0; 11].into_boxed_slice()`
= note: `-D clippy::large-stack-arrays` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::large_stack_arrays)]`

error: allocating a local array larger than 10 bytes
--> tests/ui-toml/array_size_threshold/array_size_threshold.rs:8:17
|
LL | let above = [0u8; 11];
| ^^^^^^^^^
|
= help: consider allocating on the heap with `vec![0u8; 11].into_boxed_slice()`
= note: `-D clippy::large-stack-arrays` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::large_stack_arrays)]`

error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

18 changes: 9 additions & 9 deletions tests/ui/large_const_arrays.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ 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];
pub(crate) const G_FOO_PUB_CRATE: [u32; 250] = [0u32; 250];
pub const G_FOO_PUB: [u32; 250] = [0u32; 250];
const G_FOO: [u32; 250] = [0u32; 250];

fn main() {
// Should lint
Expand All @@ -26,10 +26,10 @@ fn main() {
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];
pub const G_BAR_PUB: [u32; 250] = [0u32; 250];
const G_BAR: [u32; 250] = [0u32; 250];
pub const G_BAR_STRUCT_PUB: [S; 4] = [S { data: [0; 32] }; 4];
const G_BAR_STRUCT: [S; 4] = [S { data: [0; 32] }; 4];
pub const G_BAR_S_PUB: [Option<&str>; 50] = [Some("str"); 50];
const G_BAR_S: [Option<&str>; 50] = [Some("str"); 50];
}
18 changes: 9 additions & 9 deletions tests/ui/large_const_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ 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];
pub(crate) const G_FOO_PUB_CRATE: [u32; 250] = [0u32; 250];
pub const G_FOO_PUB: [u32; 250] = [0u32; 250];
const G_FOO: [u32; 250] = [0u32; 250];

fn main() {
// Should lint
Expand All @@ -26,10 +26,10 @@ fn main() {
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];
pub const G_BAR_PUB: [u32; 250] = [0u32; 250];
const G_BAR: [u32; 250] = [0u32; 250];
pub const G_BAR_STRUCT_PUB: [S; 4] = [S { data: [0; 32] }; 4];
const G_BAR_STRUCT: [S; 4] = [S { data: [0; 32] }; 4];
pub const G_BAR_S_PUB: [Option<&str>; 50] = [Some("str"); 50];
const G_BAR_S: [Option<&str>; 50] = [Some("str"); 50];
}
39 changes: 20 additions & 19 deletions tests/ui/large_stack_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum E {
T(u32),
}

const STATIC_PROMOTED_LARGE_ARRAY: &[u8; 512001] = &[0; 512001];
pub static DOESNOTLINT: [u8; 512_001] = [0; 512_001];
pub static DOESNOTLINT2: [u8; 512_001] = {
let x = 0;
Expand All @@ -23,38 +24,38 @@ pub static DOESNOTLINT2: [u8; 512_001] = {

fn issue_10741() {
#[derive(Copy, Clone)]
struct Large([u32; 100_000]);
struct Large([u32; 2048]);

fn build() -> Large {
Large([0; 100_000])
Large([0; 2048])
}

let _x = [build(); 3];
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes

let _y = [build(), build(), build()];
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
}

fn main() {
let bad = (
[0u32; 20_000_000],
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
[S { data: [0; 32] }; 5000],
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
[Some(""); 20_000_000],
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
[E::T(0); 5000],
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
[0u8; usize::MAX],
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
);

let good = (
[0u32; 1000],
[S { data: [0; 32] }; 1000],
[Some(""); 1000],
[E::T(0); 1000],
[0u32; 50],
[S { data: [0; 32] }; 4],
[Some(""); 50],
[E::T(0); 2],
[(); 20_000_000],
);
}
Expand All @@ -68,7 +69,7 @@ fn issue_12586() {
// Weird rule to test help messages.
($a:expr => $b:expr) => {
[$a, $b, $a, $b]
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
};
($id:ident; $n:literal) => {
dummy!(::std::vec![$id;$n])
Expand All @@ -80,26 +81,26 @@ fn issue_12586() {
macro_rules! create_then_move {
($id:ident; $n:literal) => {{
let _x_ = [$id; $n];
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
_x_
}};
}

let x = [0u32; 50_000];
let x = [0u32; 4096];
let y = vec![x, x, x, x, x];
let y = vec![dummy![x, x, x, x, x]];
let y = vec![dummy![[x, x, x, x, x]]];
let y = dummy![x, x, x, x, x];
let y = [x, x, dummy!(x), x, x];
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
let y = dummy![x => x];
let y = dummy![x;5];
let y = dummy!(vec![dummy![x, x, x, x, x]]);
let y = dummy![[x, x, x, x, x]];
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes

let y = proc_macros::make_it_big!([x; 1]);
//~^ ERROR: allocating a local array larger than 512000 bytes
//~^ ERROR: allocating a local array larger than 16384 bytes
let y = vec![proc_macros::make_it_big!([x; 10])];
let y = vec![create_then_move![x; 5]; 5];
}
48 changes: 24 additions & 24 deletions tests/ui/large_stack_arrays.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:32:14
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:33:14
|
LL | let _x = [build(); 3];
| ^^^^^^^^^^^^
Expand All @@ -8,64 +8,64 @@ LL | let _x = [build(); 3];
= note: `-D clippy::large-stack-arrays` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::large_stack_arrays)]`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:35:14
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:36:14
|
LL | let _y = [build(), build(), build()];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![build(), build(), build()].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:41:9
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:42:9
|
LL | [0u32; 20_000_000],
| ^^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![0u32; 20_000_000].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:43:9
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:44:9
|
LL | [S { data: [0; 32] }; 5000],
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![S { data: [0; 32] }; 5000].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:45:9
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:46:9
|
LL | [Some(""); 20_000_000],
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![Some(""); 20_000_000].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:47:9
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:48:9
|
LL | [E::T(0); 5000],
| ^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![E::T(0); 5000].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:49:9
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:50:9
|
LL | [0u8; usize::MAX],
| ^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![0u8; usize::MAX].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:93:13
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:94:13
|
LL | let y = [x, x, dummy!(x), x, x];
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![x, x, dummy!(x), x, x].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:70:13
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:71:13
|
LL | [$a, $b, $a, $b]
| ^^^^^^^^^^^^^^^^
Expand All @@ -75,22 +75,22 @@ LL | let y = dummy![x => x];
|
= note: this error originates in the macro `dummy` (in Nightly builds, run with -Z macro-backtrace for more info)

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:98:20
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:99:20
|
LL | let y = dummy![[x, x, x, x, x]];
| ^^^^^^^^^^^^^^^
|
= help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:101:39
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:102:39
|
LL | let y = proc_macros::make_it_big!([x; 1]);
| ^^^^^^

error: allocating a local array larger than 512000 bytes
--> tests/ui/large_stack_arrays.rs:82:23
error: allocating a local array larger than 16384 bytes
--> tests/ui/large_stack_arrays.rs:83:23
|
LL | let _x_ = [$id; $n];
| ^^^^^^^^^
Expand Down

0 comments on commit 2d2e8cc

Please sign in to comment.