Move large_stack_frames to suspicious

[y21]

This has been in nursery ever since its initial merge, with no FP reports. This lint would have caught rust-lang/rust#132050 (comment) so I think this could be valuable as a warn-by-default lint.

changelog: Move large_stack_frames to suspicious (now warn-by-default)

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[y21]

Hm, this overlaps quite a bit with large_stack_arrays. As far as I can tell large_stack_frames lints on most of large_stack_arrays test cases (it has a lower size limit so not all), though large_stack_arrays is in pedantic.

Looking at the lintcheck results for the top 200 crates, there are 16 hits from impls generated by this macro: https://docs.rs/hex/latest/src/hex/lib.rs.html#206 , which looks about expected to me - some of them definitely create way too large arrays and are basically impossible to call.

[Jarcho]

The current implementation isn't particularly great. Adding up all the named locals can cause very large over-estimates, and ignoring temporaries can cause large under-estimates.

There's also a discrepancy between the stack size on release and debug builds. I don't think debug builds overlap mir locals at all, but release builds definitely do. This can lead to cases where the debug builds use way more stack memory, e.g.

fn foo(x: &[u8]) -> u8 {
    std::hint::black_box(x)[0] + 1
}

fn main() {
    let x = 0u8;
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
    let x = foo(&[x; 1024*1024]);
}

The debug build would store each temporary array separately on the stack, but the release build would overlap all of them in the same space. A similar thing happens in the linked issue where the compiler doesn't overlap things it could be.

[y21]

Yeah, the lint description also mentions that this is going to be inaccurate for release builds (though the diagnostic doesn't make this very clear), but I'd think most people want their programs to also run in dev builds, so it sounds like a useful warning even if that's only true for unoptimized builds.

Or is this also very inaccurate even for dev builds? I'm not sure what kinds of optimisations run in dev builds.

[Jarcho]

It will only underestimate on debug builds since they allocate enough stack space for all locals, not just named locals. This needs at least two limits to consider since optimized and non-optimized builds can have wildly different stack sizes. Even getting estimates for both of those is hard since it has to recompute the size of generator state machines.

[y21]

The lint looks at all locals too, not just named ones, no? Specifically mir.local_decls, which contains the arguments, named variables, temporaries and the return place if I'm not mistaken.

Also, can you explain what you mean by two limits? Like, having a separate configurable limit for release & debug builds and the lint selects one depending on the opt-level for the current session? I feel like that would interact poorly with #[expect].

[Jarcho]

I should have done more than a quick look. The named locals in the code doesn't do any filtering.

I would say have the lint compute both and compare both. If it weren't for generators it wouldn't be too bad.

[y21]

I would say have the lint compute both and compare both.

What's "both" referring to here? And compare them to do what?

Also, why are generators special here? rustc seems smart enough not to include locals that aren't used across yields points in the generator state machine, but it doesn't merge any locals in the actual coroutine either so it seems fine that we lint e.g. here. Unless you mean something else.

[Jarcho]

Both being the estimated unoptimized stack size and optimized stack size. This can be done, except the layout of generator types is different between the two. The rustc issue you linked is an example of generator types having a very large difference between the optimized and unoptimized size.