Skip to content

Commit aa31918

Browse files
tesujitesuji
committed
add stack threshold indicates that llvm inlines the array
1 parent 6745434 commit aa31918

File tree

4 files changed

+79
-90
lines changed

4 files changed

+79
-90
lines changed

‎clippy_lints/src/let_arr_const.rs

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
1+
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::implements_trait;
4+
use clippy_utils::visitors::is_const_evaluatable;
45
use hir::{BindingMode, ExprKind, PatKind, Stmt, StmtKind};
56
use rustc_errors::Applicability;
67
use rustc_hir as hir;
@@ -38,58 +39,77 @@ declare_clippy_lint! {
3839

3940
declare_lint_pass!(LetArrConst => [LET_ARR_CONST]);
4041

41-
impl LateLintPass<'_> for LetArrConst {
42-
// should lint on `let array` which non-mut and:
43-
// if let repeat:
44-
// if let expr: copy
45-
// todo: repair clippy::author
46-
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
47-
if let StmtKind::Let(ref local) = stmt.kind
42+
// FIXME: See also LARGE_CONST_ARRAYS, LARGE_STACK_ARRAYS.
43+
impl<'tcx> LateLintPass<'tcx> for LetArrConst {
44+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
45+
// let pat: ty = init;
46+
if let StmtKind::Let(local) = stmt.kind
4847
&& let PatKind::Binding(BindingMode::NONE, _, _name, None) = local.pat.kind
49-
&& let Some(ref init) = local.init
48+
&& let Some(init) = local.init
5049
&& !init.span.from_expansion()
5150
{
51+
// LLVM optimizes the load of 16 byte as a single `mov`.
52+
// Bigger values make more `mov` instructions generated.
53+
// While changing code as this lint suggests, it becomes
54+
// a single load (`lea`) of an address in `.rodata`.
55+
const STACK_THRESHOLD: u64 = 16;
56+
// lint only `if size_of(init) > STACK_THRESHOLD`
57+
let ty = cx.typeck_results().expr_ty(init);
58+
if let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty))
59+
&& let size = layout.layout.size()
60+
&& size.bytes() <= STACK_THRESHOLD
61+
{
62+
return;
63+
}
64+
5265
let mut applicability = Applicability::MachineApplicable;
5366
let lang_items = cx.tcx.lang_items();
5467
let Some(copy_id) = lang_items.copy_trait() else {
5568
return;
5669
};
57-
// `let arr = [<Copy type>; 42];
70+
71+
let generic_msg = "using `static` to push the array to read-only section of program";
72+
let report_static = || {
73+
span_lint_and_help(
74+
cx,
75+
LET_ARR_CONST,
76+
local.span,
77+
"declaring a read-only array on the stack",
78+
None,
79+
generic_msg,
80+
);
81+
};
82+
5883
let mut should = false;
59-
if let ExprKind::Repeat(ref value, _length) = init.kind {
84+
// if init is [<Copy type>; 42]
85+
if let ExprKind::Repeat(value, _length) = init.kind {
6086
let ty = cx.typeck_results().expr_ty(value);
6187
if !implements_trait(cx, ty, copy_id, &[]) {
62-
span_lint_and_help(
63-
cx,
64-
LET_ARR_CONST,
65-
local.span,
66-
"declaring a read-only array on the stack",
67-
None,
68-
"using `static` to push the array to read-only section of program",
69-
);
88+
report_static();
7089
return;
7190
}
7291
should = true;
7392
}
74-
// `let arr = [1, 2, 3, 4];
75-
if let ExprKind::Array([ref expr, ..]) = init.kind
93+
// if init is [1, 2, 3, 4]
94+
if let ExprKind::Array(items @ [ref expr, ..]) = init.kind
7695
&& let ty = cx.typeck_results().expr_ty(expr)
7796
&& implements_trait(cx, ty, copy_id, &[])
7897
{
79-
should = true;
98+
if items.iter().all(|expr| is_const_evaluatable(cx, expr)) {
99+
should = true;
100+
}
80101
}
81102

82103
if should {
83104
let snippet = snippet_with_applicability(cx, init.span, "_", &mut applicability);
84-
let sugg = format!("*&{snippet}");
85-
span_lint_and_sugg(
105+
let msg = format!("{generic_msg} or try: `*&{snippet}`");
106+
span_lint_and_help(
86107
cx,
87108
LET_ARR_CONST,
88109
init.span,
89110
"declaring a read-only array on the stack",
90-
"using `static` to push the array to read-only section of program or try",
91-
sugg,
92-
applicability,
111+
None,
112+
msg,
93113
);
94114
}
95115
}

‎tests/ui/let_arr_const.fixed

Lines changed: 0 additions & 52 deletions
This file was deleted.

‎tests/ui/let_arr_const.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ pub mod issue_rust_73825 {
1212
pub fn do_not() {
1313
let _repeat = *&[0; 64];
1414
let _arr = *&[0, 1, 2, 3, 4];
15+
16+
let (x, y) = (1, 2);
17+
let _arr = [x, y];
18+
let _arr = [42u8; 16];
1519
let _arr = mac_gen_arr!();
1620
let _arr = gen_array();
1721
let _arr = gen_array_non_copy();
22+
let _arr = [gen_array_no_const(), gen_array_no_const()];
1823
{
1924
let mut arr = [0; 32];
2025
arr[1] = 42;
@@ -30,6 +35,10 @@ pub mod issue_rust_73825 {
3035
}
3136
}
3237

38+
fn gen_array_no_const() -> [u32; 42] {
39+
unimplemented!()
40+
}
41+
3342
const fn gen_array() -> [u32; 42] {
3443
unimplemented!()
3544
}
@@ -41,7 +50,9 @@ pub mod issue_rust_73825 {
4150
pub fn do_it() {
4251
// Copy type
4352
let _repeat: [i32; 64] = [0; 64];
53+
let _repeat = [42u8; 17];
4454
let _arr = [0, 1, 3, 5, 7, 8];
55+
let _arr = [gen_array(), gen_array()];
4556
// Non Copy type
4657
{
4758
let _repeat_const = [const { String::new() }; 32];

‎tests/ui/let_arr_const.stderr

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,52 @@
11
error: declaring a read-only array on the stack
2-
--> tests/ui/let_arr_const.rs:43:34
2+
--> tests/ui/let_arr_const.rs:52:34
33
|
44
LL | let _repeat: [i32; 64] = [0; 64];
55
| ^^^^^^^
66
|
7+
= help: using `static` to push the array to read-only section of program or try: `*&[0; 64]`
78
= note: `-D clippy::let-arr-const` implied by `-D warnings`
89
= help: to override `-D warnings` add `#[allow(clippy::let_arr_const)]`
9-
help: using `static` to push the array to read-only section of program or try
10+
11+
error: declaring a read-only array on the stack
12+
--> tests/ui/let_arr_const.rs:53:23
13+
|
14+
LL | let _repeat = [42u8; 17];
15+
| ^^^^^^^^^^
1016
|
11-
LL | let _repeat: [i32; 64] = *&[0; 64];
12-
| ~~~~~~~~~
17+
= help: using `static` to push the array to read-only section of program or try: `*&[42u8; 17]`
1318

1419
error: declaring a read-only array on the stack
15-
--> tests/ui/let_arr_const.rs:44:20
20+
--> tests/ui/let_arr_const.rs:54:20
1621
|
1722
LL | let _arr = [0, 1, 3, 5, 7, 8];
1823
| ^^^^^^^^^^^^^^^^^^
1924
|
20-
help: using `static` to push the array to read-only section of program or try
25+
= help: using `static` to push the array to read-only section of program or try: `*&[0, 1, 3, 5, 7, 8]`
26+
27+
error: declaring a read-only array on the stack
28+
--> tests/ui/let_arr_const.rs:55:20
29+
|
30+
LL | let _arr = [gen_array(), gen_array()];
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
2132
|
22-
LL | let _arr = *&[0, 1, 3, 5, 7, 8];
23-
| ~~~~~~~~~~~~~~~~~~~~
33+
= help: using `static` to push the array to read-only section of program or try: `*&[gen_array(), gen_array()]`
2434

2535
error: declaring a read-only array on the stack
26-
--> tests/ui/let_arr_const.rs:47:13
36+
--> tests/ui/let_arr_const.rs:58:13
2737
|
2838
LL | let _repeat_const = [const { String::new() }; 32];
2939
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3040
|
3141
= help: using `static` to push the array to read-only section of program
3242

3343
error: declaring a read-only array on the stack
34-
--> tests/ui/let_arr_const.rs:49:13
44+
--> tests/ui/let_arr_const.rs:60:13
3545
|
3646
LL | let _repeat = [EMPTY; 32];
3747
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
3848
|
3949
= help: using `static` to push the array to read-only section of program
4050

41-
error: aborting due to 4 previous errors
51+
error: aborting due to 6 previous errors
4252

0 commit comments

Comments
 (0)