Skip to content

Commit e78dbc0

Browse files
committed
fix: large_stack_frames FP on compiler generated targets
1 parent 7e2d26f commit e78dbc0

File tree

10 files changed

+258
-42
lines changed

10 files changed

+258
-42
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6582,6 +6582,7 @@ Released 2018-09-13
65826582
[`allow-expect-in-consts`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-consts
65836583
[`allow-expect-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-tests
65846584
[`allow-indexing-slicing-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-indexing-slicing-in-tests
6585+
[`allow-large-stack-frames-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-large-stack-frames-in-tests
65856586
[`allow-mixed-uninlined-format-args`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-mixed-uninlined-format-args
65866587
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
65876588
[`allow-panic-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-tests

book/src/lint_configuration.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,16 @@ Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
111111
* [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)
112112

113113

114+
## `allow-large-stack-frames-in-tests`
115+
Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
116+
117+
**Default Value:** `true`
118+
119+
---
120+
**Affected lints:**
121+
* [`large_stack_frames`](https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames)
122+
123+
114124
## `allow-mixed-uninlined-format-args`
115125
Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
116126

clippy_config/src/conf.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@ define_Conf! {
372372
/// Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
373373
#[lints(indexing_slicing)]
374374
allow_indexing_slicing_in_tests: bool = false,
375+
/// Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
376+
#[lints(large_stack_frames)]
377+
allow_large_stack_frames_in_tests: bool = true,
375378
/// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
376379
#[lints(uninlined_format_args)]
377380
allow_mixed_uninlined_format_args: bool = true,

clippy_lints/src/large_stack_frames.rs

Lines changed: 90 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@ use std::{fmt, ops};
22

33
use clippy_config::Conf;
44
use clippy_utils::diagnostics::span_lint_and_then;
5-
use clippy_utils::fn_has_unsatisfiable_preds;
5+
use clippy_utils::macros::{MacroCall, macro_backtrace};
66
use clippy_utils::source::SpanRangeExt;
7+
use clippy_utils::{fn_has_unsatisfiable_preds, is_in_test};
8+
use rustc_errors::Diag;
79
use rustc_hir::def_id::LocalDefId;
810
use rustc_hir::intravisit::FnKind;
911
use rustc_hir::{Body, FnDecl};
1012
use rustc_lexer::is_ident;
1113
use rustc_lint::{LateContext, LateLintPass};
1214
use rustc_session::impl_lint_pass;
13-
use rustc_span::Span;
15+
use rustc_span::{MacroKind, Span, SyntaxContext};
1416

1517
declare_clippy_lint! {
1618
/// ### What it does
@@ -83,12 +85,14 @@ declare_clippy_lint! {
8385

8486
pub struct LargeStackFrames {
8587
maximum_allowed_size: u64,
88+
allow_large_stack_frames_in_tests: bool,
8689
}
8790

8891
impl LargeStackFrames {
8992
pub fn new(conf: &'static Conf) -> Self {
9093
Self {
9194
maximum_allowed_size: conf.stack_size_threshold,
95+
allow_large_stack_frames_in_tests: conf.allow_large_stack_frames_in_tests,
9296
}
9397
}
9498
}
@@ -165,54 +169,98 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackFrames {
165169
if frame_size.exceeds_limit(limit) {
166170
// Point at just the function name if possible, because lints that span
167171
// the entire body and don't have to are less legible.
168-
let fn_span = match fn_kind {
169-
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
170-
FnKind::Closure => entire_fn_span,
172+
let (fn_span, fn_kind, fn_name) = match fn_kind {
173+
FnKind::ItemFn(ident, _, _) => (ident.span, "function", format!("function `{}`", ident.name)),
174+
FnKind::Method(ident, _) => (ident.span, "method", format!("method `{}`", ident.name)),
175+
FnKind::Closure => (entire_fn_span, "closure", "closure".to_string()),
171176
};
172177

178+
// Don't lint inside tests if configured to not do so.
179+
if self.allow_large_stack_frames_in_tests && is_in_test(cx.tcx, cx.tcx.local_def_id_to_hir_id(local_def_id))
180+
{
181+
return;
182+
}
183+
184+
let explain_lint = |diag: &mut Diag<'_, ()>, ctxt: SyntaxContext| {
185+
// Point out the largest individual contribution to this size, because
186+
// it is the most likely to be unintentionally large.
187+
if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size)
188+
&& let local_span = local.source_info.span
189+
&& local_span.ctxt() == ctxt
190+
{
191+
let size = Space::Used(size); // pluralizes for us
192+
let ty = local.ty;
193+
194+
// TODO: Is there a cleaner, robust way to ask this question?
195+
// The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
196+
// and that doesn't get us the true name in scope rather than the span text either.
197+
if let Some(name) = local_span.get_source_text(cx)
198+
&& is_ident(&name)
199+
{
200+
// If the local is an ordinary named variable,
201+
// print its name rather than relying solely on the span.
202+
diag.span_label(
203+
local_span,
204+
format!("`{name}` is the largest part, at {size} for type `{ty}`"),
205+
);
206+
} else {
207+
diag.span_label(
208+
local_span,
209+
format!("this is the largest part, at {size} for type `{ty}`"),
210+
);
211+
}
212+
}
213+
214+
// Explain why we are linting this and not other functions.
215+
diag.note(format!(
216+
"{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
217+
));
218+
219+
// Explain why the user should care, briefly.
220+
diag.note_once(
221+
"allocating large amounts of stack space can overflow the stack \
222+
and cause the program to abort",
223+
);
224+
};
225+
226+
if fn_span.from_expansion() {
227+
// Don't lint functions that are generated by special compiling targets, like `--test`.
228+
if fn_span.is_dummy() && fn_span.source_callsite().is_dummy() {
229+
return;
230+
}
231+
232+
span_lint_and_then(
233+
cx,
234+
LARGE_STACK_FRAMES,
235+
fn_span.source_callsite(),
236+
format!("{fn_name} generated by this macro may allocate a lot of stack space"),
237+
|diag| {
238+
diag.span_label(
239+
fn_span,
240+
format!("this {fn_kind} has a stack frame size of {frame_size}"),
241+
);
242+
243+
if let Some(MacroCall {
244+
kind: MacroKind::Derive,
245+
..
246+
}) = macro_backtrace(fn_span).last()
247+
{
248+
explain_lint(diag, SyntaxContext::root());
249+
} else {
250+
explain_lint(diag, fn_span.ctxt());
251+
}
252+
},
253+
);
254+
return;
255+
}
256+
173257
span_lint_and_then(
174258
cx,
175259
LARGE_STACK_FRAMES,
176260
fn_span,
177261
format!("this function may allocate {frame_size} on the stack"),
178262
|diag| {
179-
// Point out the largest individual contribution to this size, because
180-
// it is the most likely to be unintentionally large.
181-
if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size) {
182-
let local_span: Span = local.source_info.span;
183-
let size = Space::Used(size); // pluralizes for us
184-
let ty = local.ty;
185-
186-
// TODO: Is there a cleaner, robust way to ask this question?
187-
// The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
188-
// and that doesn't get us the true name in scope rather than the span text either.
189-
if let Some(name) = local_span.get_source_text(cx)
190-
&& is_ident(&name)
191-
{
192-
// If the local is an ordinary named variable,
193-
// print its name rather than relying solely on the span.
194-
diag.span_label(
195-
local_span,
196-
format!("`{name}` is the largest part, at {size} for type `{ty}`"),
197-
);
198-
} else {
199-
diag.span_label(
200-
local_span,
201-
format!("this is the largest part, at {size} for type `{ty}`"),
202-
);
203-
}
204-
}
205-
206-
// Explain why we are linting this and not other functions.
207-
diag.note(format!(
208-
"{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
209-
));
210-
211-
// Explain why the user should care, briefly.
212-
diag.note_once(
213-
"allocating large amounts of stack space can overflow the stack \
214-
and cause the program to abort",
215-
);
263+
explain_lint(diag, SyntaxContext::root());
216264
},
217265
);
218266
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
stack-size-threshold = 0
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//@ normalize-stderr-test: "\b10000(08|16|32)\b" -> "100$$PTR"
2+
//@ normalize-stderr-test: "\b2500(060|120)\b" -> "250$$PTR"
3+
4+
#![warn(clippy::large_stack_frames)]
5+
6+
extern crate serde;
7+
use serde::{Deserialize, Serialize};
8+
9+
struct ArrayDefault<const N: usize>([u8; N]);
10+
11+
macro_rules! mac {
12+
($name:ident) => {
13+
fn foo() {
14+
let $name = 1;
15+
println!("macro_name called");
16+
}
17+
18+
fn bar() {
19+
let $name = ArrayDefault([0; 1000]);
20+
}
21+
};
22+
}
23+
24+
mac!(something);
25+
//~^ large_stack_frames
26+
//~| large_stack_frames
27+
28+
#[derive(Deserialize, Serialize)]
29+
//~^ large_stack_frames
30+
//~| large_stack_frames
31+
//~| large_stack_frames
32+
//~| large_stack_frames
33+
//~| large_stack_frames
34+
//~| large_stack_frames
35+
//~| large_stack_frames
36+
//~| large_stack_frames
37+
struct S {
38+
a: [u128; 31],
39+
}
40+
41+
fn main() {}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
error: function `foo` generated by this macro may allocate a lot of stack space
2+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:24:1
3+
|
4+
LL | fn foo() {
5+
| --- this function has a stack frame size of 92 bytes
6+
...
7+
LL | mac!(something);
8+
| ^^^^^^^^^^^^^^^
9+
|
10+
= note: 92 bytes is larger than Clippy's configured `stack-size-threshold` of 0
11+
= note: allocating large amounts of stack space can overflow the stack and cause the program to abort
12+
= note: `-D clippy::large-stack-frames` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::large_stack_frames)]`
14+
15+
error: function `bar` generated by this macro may allocate a lot of stack space
16+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:24:1
17+
|
18+
LL | fn bar() {
19+
| --- this function has a stack frame size of 2000 bytes
20+
LL | let $name = ArrayDefault([0; 1000]);
21+
| --------- this is the largest part, at 1000 bytes for type `[u8; 1000]`
22+
...
23+
LL | mac!(something);
24+
| ^^^^^^^^^^^^^^^
25+
|
26+
= note: 2000 bytes is larger than Clippy's configured `stack-size-threshold` of 0
27+
28+
error: method `expecting` generated by this macro may allocate a lot of stack space
29+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
30+
|
31+
LL | #[derive(Deserialize, Serialize)]
32+
| ^^^^^^^^^^^ this method has a stack frame size of 57 bytes
33+
|
34+
= note: 57 bytes is larger than Clippy's configured `stack-size-threshold` of 0
35+
36+
error: method `visit_u64` generated by this macro may allocate a lot of stack space
37+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
38+
|
39+
LL | #[derive(Deserialize, Serialize)]
40+
| ^^^^^^^^^^^ this method has a stack frame size of 10 bytes
41+
|
42+
= note: 10 bytes is larger than Clippy's configured `stack-size-threshold` of 0
43+
44+
error: method `visit_str` generated by this macro may allocate a lot of stack space
45+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
46+
|
47+
LL | #[derive(Deserialize, Serialize)]
48+
| ^^^^^^^^^^^ this method has a stack frame size of 19 bytes
49+
|
50+
= note: 19 bytes is larger than Clippy's configured `stack-size-threshold` of 0
51+
52+
error: method `visit_bytes` generated by this macro may allocate a lot of stack space
53+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
54+
|
55+
LL | #[derive(Deserialize, Serialize)]
56+
| ^^^^^^^^^^^ this method has a stack frame size of 35 bytes
57+
|
58+
= note: 35 bytes is larger than Clippy's configured `stack-size-threshold` of 0
59+
60+
error: method `expecting` generated by this macro may allocate a lot of stack space
61+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
62+
|
63+
LL | #[derive(Deserialize, Serialize)]
64+
| ^^^^^^^^^^^ this method has a stack frame size of 57 bytes
65+
|
66+
= note: 57 bytes is larger than Clippy's configured `stack-size-threshold` of 0
67+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
68+
69+
error: method `visit_seq` generated by this macro may allocate a lot of stack space
70+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
71+
|
72+
LL | #[derive(Deserialize, Serialize)]
73+
| ^^^^^^^^^^^ this method has a stack frame size of 3137 bytes
74+
|
75+
= note: 3137 bytes is larger than Clippy's configured `stack-size-threshold` of 0
76+
77+
error: method `visit_map` generated by this macro may allocate a lot of stack space
78+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
79+
|
80+
LL | #[derive(Deserialize, Serialize)]
81+
| ^^^^^^^^^^^ this method has a stack frame size of 4798 bytes
82+
|
83+
= note: 4798 bytes is larger than Clippy's configured `stack-size-threshold` of 0
84+
85+
error: method `serialize` generated by this macro may allocate a lot of stack space
86+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:23
87+
|
88+
LL | #[derive(Deserialize, Serialize)]
89+
| ^^^^^^^^^ this method has a stack frame size of 170 bytes
90+
|
91+
= note: 170 bytes is larger than Clippy's configured `stack-size-threshold` of 0
92+
93+
error: aborting due to 10 previous errors
94+
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
stack-size-threshold = 0
2+
allow-large-stack-frames-in-tests = false
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This test checks if `clippy::large_stack_frames` is working correctly when encountering functions
2+
// generated by special compiling targets like `--test`.
3+
//@compile-flags: --test
4+
//@check-pass
5+
6+
#![warn(clippy::large_stack_frames)]
7+
8+
#[cfg(test)]
9+
#[allow(clippy::large_stack_frames)]
10+
mod test {
11+
#[test]
12+
fn main_test() {}
13+
}

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
99
allow-expect-in-consts
1010
allow-expect-in-tests
1111
allow-indexing-slicing-in-tests
12+
allow-large-stack-frames-in-tests
1213
allow-mixed-uninlined-format-args
1314
allow-one-hash-in-raw-strings
1415
allow-panic-in-tests
@@ -103,6 +104,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
103104
allow-expect-in-consts
104105
allow-expect-in-tests
105106
allow-indexing-slicing-in-tests
107+
allow-large-stack-frames-in-tests
106108
allow-mixed-uninlined-format-args
107109
allow-one-hash-in-raw-strings
108110
allow-panic-in-tests
@@ -197,6 +199,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
197199
allow-expect-in-consts
198200
allow-expect-in-tests
199201
allow-indexing-slicing-in-tests
202+
allow-large-stack-frames-in-tests
200203
allow-mixed-uninlined-format-args
201204
allow-one-hash-in-raw-strings
202205
allow-panic-in-tests

0 commit comments

Comments
 (0)