fix: large_stack_frames FP on compiler generated targets

profetia · profetia · commit 4d142c1b0549 · 2025-07-13T23:06:55.000+08:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6582,6 +6582,7 @@ Released 2018-09-13
 [`allow-expect-in-consts`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-consts
 [`allow-expect-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-tests
 [`allow-indexing-slicing-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-indexing-slicing-in-tests
+[`allow-large-stack-frames-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-large-stack-frames-in-tests
 [`allow-mixed-uninlined-format-args`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-mixed-uninlined-format-args
 [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
 [`allow-panic-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-tests
diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md
@@ -111,6 +111,16 @@ Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
 * [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)
 
 
+## `allow-large-stack-frames-in-tests`
+Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
+
+**Default Value:** `true`
+
+---
+**Affected lints:**
+* [`large_stack_frames`](https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames)
+
+
 ## `allow-mixed-uninlined-format-args`
 Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
 
diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs
@@ -372,6 +372,9 @@ define_Conf! {
     /// Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
     #[lints(indexing_slicing)]
     allow_indexing_slicing_in_tests: bool = false,
+    /// Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
+    #[lints(large_stack_frames)]
+    allow_large_stack_frames_in_tests: bool = true,
     /// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
     #[lints(uninlined_format_args)]
     allow_mixed_uninlined_format_args: bool = true,
diff --git a/clippy_lints/src/large_stack_frames.rs b/clippy_lints/src/large_stack_frames.rs
@@ -2,15 +2,17 @@ use std::{fmt, ops};
 
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::fn_has_unsatisfiable_preds;
+use clippy_utils::macros::{MacroCall, macro_backtrace};
 use clippy_utils::source::SpanRangeExt;
+use clippy_utils::{fn_has_unsatisfiable_preds, is_in_test};
+use rustc_errors::Diag;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{Body, FnDecl};
 use rustc_lexer::is_ident;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
-use rustc_span::Span;
+use rustc_span::{MacroKind, Span, SyntaxContext};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -83,12 +85,14 @@ declare_clippy_lint! {
 
 pub struct LargeStackFrames {
     maximum_allowed_size: u64,
+    allow_large_stack_frames_in_tests: bool,
 }
 
 impl LargeStackFrames {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
             maximum_allowed_size: conf.stack_size_threshold,
+            allow_large_stack_frames_in_tests: conf.allow_large_stack_frames_in_tests,
         }
     }
 }
@@ -165,54 +169,95 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackFrames {
         if frame_size.exceeds_limit(limit) {
             // Point at just the function name if possible, because lints that span
             // the entire body and don't have to are less legible.
-            let fn_span = match fn_kind {
-                FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
-                FnKind::Closure => entire_fn_span,
+            let (fn_span, fn_name) = match fn_kind {
+                FnKind::ItemFn(ident, _, _) => (ident.span, format!("function `{}`", ident.name)),
+                FnKind::Method(ident, _) => (ident.span, format!("method `{}`", ident.name)),
+                FnKind::Closure => (entire_fn_span, "closure".to_string()),
             };
 
+            // Don't lint inside tests if configured to not do so.
+            if self.allow_large_stack_frames_in_tests && is_in_test(cx.tcx, cx.tcx.local_def_id_to_hir_id(local_def_id))
+            {
+                return;
+            }
+
+            let explain_lint = |diag: &mut Diag<'_, ()>, ctxt: SyntaxContext| {
+                // Point out the largest individual contribution to this size, because
+                // it is the most likely to be unintentionally large.
+                if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size)
+                    && let local_span = local.source_info.span
+                    && local_span.ctxt() == ctxt
+                {
+                    let size = Space::Used(size); // pluralizes for us
+                    let ty = local.ty;
+
+                    // TODO: Is there a cleaner, robust way to ask this question?
+                    // The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
+                    // and that doesn't get us the true name in scope rather than the span text either.
+                    if let Some(name) = local_span.get_source_text(cx)
+                        && is_ident(&name)
+                    {
+                        // If the local is an ordinary named variable,
+                        // print its name rather than relying solely on the span.
+                        diag.span_label(
+                            local_span,
+                            format!("`{name}` is the largest part, at {size} for type `{ty}`"),
+                        );
+                    } else {
+                        diag.span_label(
+                            local_span,
+                            format!("this is the largest part, at {size} for type `{ty}`"),
+                        );
+                    }
+                }
+
+                // Explain why we are linting this and not other functions.
+                diag.note(format!(
+                    "{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
+                ));
+
+                // Explain why the user should care, briefly.
+                diag.note_once(
+                    "allocating large amounts of stack space can overflow the stack \
+                        and cause the program to abort",
+                );
+            };
+
+            if fn_span.from_expansion() {
+                // Don't lint functions that are generated by special compiling targets, like `--test`.
+                if fn_span.is_dummy() && fn_span.source_callsite().is_dummy() {
+                    return;
+                }
+
+                span_lint_and_then(
+                    cx,
+                    LARGE_STACK_FRAMES,
+                    fn_span.source_callsite(),
+                    format!("{} generated by this macro may allocate a lot of stack space", fn_name),
+                    |diag| {
+                        diag.span_label(fn_span, format!("this function has a stack frame size of {frame_size}"));
+
+                        if let Some(MacroCall {
+                            kind: MacroKind::Derive,
+                            ..
+                        }) = macro_backtrace(fn_span).last()
+                        {
+                            explain_lint(diag, SyntaxContext::root());
+                        } else {
+                            explain_lint(diag, fn_span.ctxt());
+                        }
+                    },
+                );
+                return;
+            }
+
             span_lint_and_then(
                 cx,
                 LARGE_STACK_FRAMES,
                 fn_span,
                 format!("this function may allocate {frame_size} on the stack"),
                 |diag| {
-                    // Point out the largest individual contribution to this size, because
-                    // it is the most likely to be unintentionally large.
-                    if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size) {
-                        let local_span: Span = local.source_info.span;
-                        let size = Space::Used(size); // pluralizes for us
-                        let ty = local.ty;
-
-                        // TODO: Is there a cleaner, robust way to ask this question?
-                        // The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
-                        // and that doesn't get us the true name in scope rather than the span text either.
-                        if let Some(name) = local_span.get_source_text(cx)
-                            && is_ident(&name)
-                        {
-                            // If the local is an ordinary named variable,
-                            // print its name rather than relying solely on the span.
-                            diag.span_label(
-                                local_span,
-                                format!("`{name}` is the largest part, at {size} for type `{ty}`"),
-                            );
-                        } else {
-                            diag.span_label(
-                                local_span,
-                                format!("this is the largest part, at {size} for type `{ty}`"),
-                            );
-                        }
-                    }
-
-                    // Explain why we are linting this and not other functions.
-                    diag.note(format!(
-                        "{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
-                    ));
-
-                    // Explain why the user should care, briefly.
-                    diag.note_once(
-                        "allocating large amounts of stack space can overflow the stack \
-                        and cause the program to abort",
-                    );
+                    explain_lint(diag, SyntaxContext::root());
                 },
             );
         }
diff --git a/tests/ui-toml/large_stack_frames_for_macros/clippy.toml b/tests/ui-toml/large_stack_frames_for_macros/clippy.toml
@@ -0,0 +1 @@
+stack-size-threshold = 0
diff --git a/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs b/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs
@@ -0,0 +1,41 @@
+//@ normalize-stderr-test: "\b10000(08|16|32)\b" -> "100$$PTR"
+//@ normalize-stderr-test: "\b2500(060|120)\b" -> "250$$PTR"
+
+#![warn(clippy::large_stack_frames)]
+
+extern crate serde;
+use serde::{Deserialize, Serialize};
+
+struct ArrayDefault<const N: usize>([u8; N]);
+
+macro_rules! mac {
+    ($name:ident) => {
+        fn foo() {
+            let $name = 1;
+            println!("macro_name called");
+        }
+
+        fn bar() {
+            let $name = ArrayDefault([0; 1000]);
+        }
+    };
+}
+
+mac!(something);
+//~^ large_stack_frames
+//~| large_stack_frames
+
+#[derive(Deserialize, Serialize)]
+//~^ large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+//~| large_stack_frames
+struct S {
+    a: [u128; 31],
+}
+
+fn main() {}
diff --git a/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr b/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr
@@ -0,0 +1,94 @@
+error: function `foo` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:24:1
+   |
+LL |         fn foo() {
+   |            --- this function has a stack frame size of 92 bytes
+...
+LL | mac!(something);
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: 92 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+   = note: allocating large amounts of stack space can overflow the stack and cause the program to abort
+   = note: `-D clippy::large-stack-frames` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::large_stack_frames)]`
+
+error: function `bar` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:24:1
+   |
+LL |         fn bar() {
+   |            --- this function has a stack frame size of 2000 bytes
+LL |             let $name = ArrayDefault([0; 1000]);
+   |                                      --------- this is the largest part, at 1000 bytes for type `[u8; 1000]`
+...
+LL | mac!(something);
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: 2000 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `expecting` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 57 bytes
+   |
+   = note: 57 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `visit_u64` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 10 bytes
+   |
+   = note: 10 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `visit_str` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 19 bytes
+   |
+   = note: 19 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `visit_bytes` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 35 bytes
+   |
+   = note: 35 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `expecting` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 57 bytes
+   |
+   = note: 57 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method `visit_seq` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 3137 bytes
+   |
+   = note: 3137 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `visit_map` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^ this function has a stack frame size of 4798 bytes
+   |
+   = note: 4798 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: method `serialize` generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:28:23
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |                       ^^^^^^^^^ this function has a stack frame size of 170 bytes
+   |
+   = note: 170 bytes is larger than Clippy's configured `stack-size-threshold` of 0
+
+error: aborting due to 10 previous errors
+
diff --git a/tests/ui-toml/large_stack_frames_for_special_targets/clippy.toml b/tests/ui-toml/large_stack_frames_for_special_targets/clippy.toml
@@ -0,0 +1,2 @@
+stack-size-threshold = 0
+allow-large-stack-frames-in-tests = false
diff --git a/tests/ui-toml/large_stack_frames_for_special_targets/large_stack_frames.rs b/tests/ui-toml/large_stack_frames_for_special_targets/large_stack_frames.rs
@@ -0,0 +1,13 @@
+// This test checks if `clippy::large_stack_frames` is working correctly when encountering functions
+// generated by special compiling targets like `--test`.
+//@compile-flags: --test
+//@check-pass
+
+#![warn(clippy::large_stack_frames)]
+
+#[cfg(test)]
+#[allow(clippy::large_stack_frames)]
+mod test {
+    #[test]
+    fn main_test() {}
+}
diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -9,6 +9,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            allow-expect-in-consts
            allow-expect-in-tests
            allow-indexing-slicing-in-tests
+           allow-large-stack-frames-in-tests
            allow-mixed-uninlined-format-args
            allow-one-hash-in-raw-strings
            allow-panic-in-tests
@@ -103,6 +104,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            allow-expect-in-consts
            allow-expect-in-tests
            allow-indexing-slicing-in-tests
+           allow-large-stack-frames-in-tests
            allow-mixed-uninlined-format-args
            allow-one-hash-in-raw-strings
            allow-panic-in-tests
@@ -197,6 +199,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            allow-expect-in-consts
            allow-expect-in-tests
            allow-indexing-slicing-in-tests
+           allow-large-stack-frames-in-tests
            allow-mixed-uninlined-format-args
            allow-one-hash-in-raw-strings
            allow-panic-in-tests

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+stack-size-threshold = 0`
	`2`	`+allow-large-stack-frames-in-tests = false`