Fix large_stack_frames FP on compiler generated targets (#15101)

Closes rust-lang/rust-clippy#15099

The compiler will generate a special `main` function for the tests,
which caused this FP.

changelog: [`large_stack_frames`] fix FP on compiler generated targets

Author: blyxyas

CHANGELOG.md

@@ -7083,6 +7083,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

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)`
 

clippy_config/src/conf.rs

@@ -373,6 +373,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,

clippy_lints/src/large_stack_frames.rs

@@ -2,15 +2,16 @@ 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::source::SpanRangeExt;
+use clippy_utils::source::{HasSession, SpanRangeExt};
+use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, 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::{Span, SyntaxContext};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -83,12 +84,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,
         }
     }
 }
@@ -152,67 +155,122 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackFrames {
         let mir = cx.tcx.optimized_mir(def_id);
         let typing_env = mir.typing_env(cx.tcx);
 
-        let sizes_of_locals = || {
-            mir.local_decls.iter().filter_map(|local| {
+        let sizes_of_locals = mir
+            .local_decls
+            .iter()
+            .filter_map(|local| {
                 let layout = cx.tcx.layout_of(typing_env.as_query_input(local.ty)).ok()?;
                 Some((local, layout.size.bytes()))
             })
-        };
+            .collect::<Vec<_>>();
 
-        let frame_size = sizes_of_locals().fold(Space::Used(0), |sum, (_, size)| sum + size);
+        let frame_size = sizes_of_locals
+            .iter()
+            .fold(Space::Used(0), |sum, (_, size)| sum + *size);
 
         let limit = self.maximum_allowed_size;
         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.iter().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 on the main function generated by `--test` target
+                if cx.sess().is_test_crate() && is_entrypoint_fn(cx, local_def_id.to_def_id()) {
+                    return;
+                }
+
+                let is_from_external_macro = fn_span.in_external_macro(cx.sess().source_map());
+                span_lint_and_then(
+                    cx,
+                    LARGE_STACK_FRAMES,
+                    fn_span.source_callsite(),
+                    format!(
+                        "{} generated by this macro may allocate a lot of stack space",
+                        if is_from_external_macro {
+                            cx.tcx.def_descr(local_def_id.into())
+                        } else {
+                            fn_name.as_str()
+                        }
+                    ),
+                    |diag| {
+                        if is_from_external_macro {
+                            return;
+                        }
+
+                        diag.span_label(
+                            fn_span,
+                            format!(
+                                "this {} has a stack frame size of {frame_size}",
+                                cx.tcx.def_descr(local_def_id.into())
+                            ),
+                        );
+
+                        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());
                 },
             );
         }

tests/ui-toml/large_stack_frames_for_macros/clippy.toml

@@ -0,0 +1 @@
+stack-size-threshold = 0

tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs

@@ -0,0 +1,42 @@
+//@ignore-target: i686
+//@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() {}

tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr

@@ -0,0 +1,89 @@
+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:25:1
+   |
+LL |         fn foo() {
+   |            --- this function has a stack frame size of 20 bytes
+...
+LL | mac!(something);
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: 20 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:25: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 generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: method generated by this macro may allocate a lot of stack space
+  --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:23
+   |
+LL | #[derive(Deserialize, Serialize)]
+   |                       ^^^^^^^^^
+
+error: aborting due to 10 previous errors
+

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

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)]
+#[expect(clippy::large_stack_frames)]
+mod test {
+    #[test]
+    fn main_test() {}
+}