Skip to content

Commit e70c892

Browse files
committed
test [large_stack_arrays] with proc-macro and make sure not to offer false help messages if in one.
1 parent 666e2f2 commit e70c892

File tree

4 files changed

+101
-28
lines changed

4 files changed

+101
-28
lines changed

clippy_lints/src/large_stack_arrays.rs

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::is_from_proc_macro;
23
use clippy_utils::macros::macro_backtrace;
34
use clippy_utils::source::snippet;
4-
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
5+
use rustc_hir::{ArrayLen, Expr, ExprKind, Item, ItemKind, Node};
56
use rustc_lint::{LateContext, LateLintPass};
67
use rustc_middle::ty::layout::LayoutOf;
78
use rustc_middle::ty::{self, ConstKind};
@@ -27,21 +28,41 @@ declare_clippy_lint! {
2728

2829
pub struct LargeStackArrays {
2930
maximum_allowed_size: u128,
31+
prev_vec_macro_callsite: Option<Span>,
3032
}
3133

3234
impl LargeStackArrays {
3335
#[must_use]
3436
pub fn new(maximum_allowed_size: u128) -> Self {
35-
Self { maximum_allowed_size }
37+
Self {
38+
maximum_allowed_size,
39+
prev_vec_macro_callsite: None,
40+
}
41+
}
42+
43+
/// Check if the given span of an expr is already in a `vec!` call.
44+
fn is_from_vec_macro(&mut self, cx: &LateContext<'_>, span: Span) -> bool {
45+
// First, we check if this is span is within the last encountered `vec!` macro's root callsite.
46+
self.prev_vec_macro_callsite
47+
.is_some_and(|vec_mac| vec_mac.contains(span))
48+
|| {
49+
// Then, we try backtracking the macro expansions, to see if there's a `vec!` macro,
50+
// and update the `prev_vec_macro_callsite`.
51+
let res = macro_backtrace(span).any(|mac| cx.tcx.is_diagnostic_item(sym::vec_macro, mac.def_id));
52+
if res {
53+
self.prev_vec_macro_callsite = Some(span.source_callsite());
54+
}
55+
res
56+
}
3657
}
3758
}
3859

3960
impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);
4061

4162
impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
42-
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
63+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
4364
if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
44-
&& !is_from_vec_macro(cx, expr.span)
65+
&& !self.is_from_vec_macro(cx, expr.span)
4566
&& let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind()
4667
&& let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
4768
&& let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)
@@ -66,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
6687
self.maximum_allowed_size
6788
),
6889
|diag| {
69-
if !expr.span.from_expansion() {
90+
if !might_be_expanded(cx, expr) {
7091
diag.help(format!(
7192
"consider allocating on the heap with `vec!{}.into_boxed_slice()`",
7293
snippet(cx, expr.span, "[...]")
@@ -78,7 +99,20 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
7899
}
79100
}
80101

81-
/// We shouldn't lint messages if the expr is already in a `vec!` call
82-
fn is_from_vec_macro(cx: &LateContext<'_>, expr_span: Span) -> bool {
83-
macro_backtrace(expr_span).any(|mac| cx.tcx.is_diagnostic_item(sym::vec_macro, mac.def_id))
102+
/// Only giving help messages if the expr does not contains macro expanded codes.
103+
fn might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool {
104+
/// Check if the span of `ArrayLen` of a repeat expression is within the expr's span,
105+
/// if not, meaning this repeat expr is definitely from some proc-macro.
106+
///
107+
/// This is a fail-safe to a case where even the `is_from_proc_macro` is unable to determain the
108+
/// correct result.
109+
fn repeat_expr_might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool {
110+
let ExprKind::Repeat(_, ArrayLen::Body(anon_const)) = expr.kind else {
111+
return false;
112+
};
113+
let len_span = cx.tcx.def_span(anon_const.def_id);
114+
!expr.span.contains(len_span)
115+
}
116+
117+
expr.span.from_expansion() || is_from_proc_macro(cx, expr) || repeat_expr_might_be_expanded(cx, expr)
84118
}

tests/ui/auxiliary/proc_macros.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use proc_macro::token_stream::IntoIter;
99
use proc_macro::Delimiter::{self, Brace, Parenthesis};
1010
use proc_macro::Spacing::{self, Alone, Joint};
1111
use proc_macro::{Group, Ident, Literal, Punct, Span, TokenStream, TokenTree as TT};
12+
use syn::spanned::Spanned;
1213

1314
type Result<T> = core::result::Result<T, TokenStream>;
1415

@@ -124,6 +125,22 @@ fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Resul
124125
Ok(())
125126
}
126127

128+
/// Takes an array repeat expression such as `[0_u32; 2]`, and return the tokens with 10 times the
129+
/// original size, which turns to `[0_u32; 20]`.
130+
#[proc_macro]
131+
pub fn make_it_big(input: TokenStream) -> TokenStream {
132+
let mut expr_repeat = syn::parse_macro_input!(input as syn::ExprRepeat);
133+
let len_span = expr_repeat.len.span();
134+
if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len {
135+
if let syn::Lit::Int(lit_int) = &expr_lit.lit {
136+
let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter");
137+
let new_val = orig_val.saturating_mul(10);
138+
expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val);
139+
}
140+
}
141+
quote::quote!(#expr_repeat).into()
142+
}
143+
127144
/// Within the item this attribute is attached to, an `inline!` macro is available which expands the
128145
/// contained tokens as though they came from a macro expansion.
129146
///

tests/ui/large_stack_arrays.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
//@aux-build:proc_macros.rs
12
#![warn(clippy::large_stack_arrays)]
23
#![allow(clippy::large_enum_variant)]
34

5+
extern crate proc_macros;
6+
47
#[derive(Clone, Copy)]
58
struct S {
69
pub data: [u64; 32],
@@ -74,12 +77,17 @@ fn issue_12586() {
7477
::std::vec![$($id),*]
7578
}
7679
}
80+
macro_rules! create_then_move {
81+
($id:ident; $n:literal) => {{
82+
let _x_ = [$id; $n];
83+
_x_
84+
}};
85+
}
7786

7887
let x = [0u32; 50_000];
7988
let y = vec![x, x, x, x, x];
8089
let y = vec![dummy![x, x, x, x, x]];
8190
let y = vec![dummy![[x, x, x, x, x]]];
82-
//~^ ERROR: allocating a local array larger than 512000 bytes
8391
let y = dummy![x, x, x, x, x];
8492
let y = [x, x, dummy!(x), x, x];
8593
//~^ ERROR: allocating a local array larger than 512000 bytes
@@ -88,4 +96,9 @@ fn issue_12586() {
8896
let y = dummy!(vec![dummy![x, x, x, x, x]]);
8997
let y = dummy![[x, x, x, x, x]];
9098
//~^ ERROR: allocating a local array larger than 512000 bytes
99+
100+
let y = proc_macros::make_it_big!([x; 1]);
101+
//~^ ERROR: allocating a local array larger than 512000 bytes
102+
let y = vec![proc_macros::make_it_big!([x; 10])];
103+
let y = vec![create_then_move![x; 5]; 5];
91104
}

tests/ui/large_stack_arrays.stderr

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: allocating a local array larger than 512000 bytes
2-
--> tests/ui/large_stack_arrays.rs:29:14
2+
--> tests/ui/large_stack_arrays.rs:32:14
33
|
44
LL | let _x = [build(); 3];
55
| ^^^^^^^^^^^^
@@ -9,71 +9,63 @@ LL | let _x = [build(); 3];
99
= help: to override `-D warnings` add `#[allow(clippy::large_stack_arrays)]`
1010

1111
error: allocating a local array larger than 512000 bytes
12-
--> tests/ui/large_stack_arrays.rs:32:14
12+
--> tests/ui/large_stack_arrays.rs:35:14
1313
|
1414
LL | let _y = [build(), build(), build()];
1515
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
1616
|
1717
= help: consider allocating on the heap with `vec![build(), build(), build()].into_boxed_slice()`
1818

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

2727
error: allocating a local array larger than 512000 bytes
28-
--> tests/ui/large_stack_arrays.rs:40:9
28+
--> tests/ui/large_stack_arrays.rs:43:9
2929
|
3030
LL | [S { data: [0; 32] }; 5000],
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
3232
|
3333
= help: consider allocating on the heap with `vec![S { data: [0; 32] }; 5000].into_boxed_slice()`
3434

3535
error: allocating a local array larger than 512000 bytes
36-
--> tests/ui/large_stack_arrays.rs:42:9
36+
--> tests/ui/large_stack_arrays.rs:45:9
3737
|
3838
LL | [Some(""); 20_000_000],
3939
| ^^^^^^^^^^^^^^^^^^^^^^
4040
|
4141
= help: consider allocating on the heap with `vec![Some(""); 20_000_000].into_boxed_slice()`
4242

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

5151
error: allocating a local array larger than 512000 bytes
52-
--> tests/ui/large_stack_arrays.rs:46:9
52+
--> tests/ui/large_stack_arrays.rs:49:9
5353
|
5454
LL | [0u8; usize::MAX],
5555
| ^^^^^^^^^^^^^^^^^
5656
|
5757
= help: consider allocating on the heap with `vec![0u8; usize::MAX].into_boxed_slice()`
5858

5959
error: allocating a local array larger than 512000 bytes
60-
--> tests/ui/large_stack_arrays.rs:81:25
61-
|
62-
LL | let y = vec![dummy![[x, x, x, x, x]]];
63-
| ^^^^^^^^^^^^^^^
64-
|
65-
= help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
66-
67-
error: allocating a local array larger than 512000 bytes
68-
--> tests/ui/large_stack_arrays.rs:84:13
60+
--> tests/ui/large_stack_arrays.rs:92:13
6961
|
7062
LL | let y = [x, x, dummy!(x), x, x];
7163
| ^^^^^^^^^^^^^^^^^^^^^^^
7264
|
7365
= help: consider allocating on the heap with `vec![x, x, dummy!(x), x, x].into_boxed_slice()`
7466

7567
error: allocating a local array larger than 512000 bytes
76-
--> tests/ui/large_stack_arrays.rs:67:13
68+
--> tests/ui/large_stack_arrays.rs:70:13
7769
|
7870
LL | [$a, $b, $a, $b]
7971
| ^^^^^^^^^^^^^^^^
@@ -84,12 +76,29 @@ LL | let y = dummy![x => x];
8476
= note: this error originates in the macro `dummy` (in Nightly builds, run with -Z macro-backtrace for more info)
8577

8678
error: allocating a local array larger than 512000 bytes
87-
--> tests/ui/large_stack_arrays.rs:89:20
79+
--> tests/ui/large_stack_arrays.rs:97:20
8880
|
8981
LL | let y = dummy![[x, x, x, x, x]];
9082
| ^^^^^^^^^^^^^^^
9183
|
9284
= help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
9385

94-
error: aborting due to 11 previous errors
86+
error: allocating a local array larger than 512000 bytes
87+
--> tests/ui/large_stack_arrays.rs:100:39
88+
|
89+
LL | let y = proc_macros::make_it_big!([x; 1]);
90+
| ^^^^^^
91+
92+
error: allocating a local array larger than 512000 bytes
93+
--> tests/ui/large_stack_arrays.rs:82:23
94+
|
95+
LL | let _x_ = [$id; $n];
96+
| ^^^^^^^^^
97+
...
98+
LL | let y = vec![create_then_move![x; 5]; 5];
99+
| ----------------------- in this macro invocation
100+
|
101+
= note: this error originates in the macro `create_then_move` (in Nightly builds, run with -Z macro-backtrace for more info)
102+
103+
error: aborting due to 12 previous errors
95104

0 commit comments

Comments
 (0)