aaron.ballman added a comment. In D142401#4096498 <https://reviews.llvm.org/D142401#4096498>, @erichkeane wrote:
> In D142401#4096478 <https://reviews.llvm.org/D142401#4096478>, @aaron.ballman > wrote: > >> In D142401#4082356 <https://reviews.llvm.org/D142401#4082356>, @cor3ntin >> wrote: >> >>> In D142401#4074959 <https://reviews.llvm.org/D142401#4074959>, @shafik >>> wrote: >>> >>>> I could not find any tests that test for this warning. >>> >>> I'm unable to trigger the warning at all. I feel i should but just using >>> that function resolves the crash, probably because it allocates more stack >>> in a separate thread, and there is actually some code that avoids marking >>> the same object used twice. >>> But the warning might still trigger on some platform, so i suspect the test >>> i added might, sometimes, trigger a warning. I'm not exactly sure how to >>> handle that >> >> Tests for stack space are always really challenging because triggering the >> failure is so system specific. We only have one test for it >> (`test/SemaTemplate/stack-exhaustion.cpp`) >> >> But do we need to run these on separate threads in all of those places to >> address the issue? >> >> "and there is actually some code that avoids marking the same object used >> twice." makes me wonder if we're missing an early return somewhere that >> would reduce stack usage more and avoid needing to spin up threads for this. >> (The upside to this change is that the code compiles more often but the >> downside is that we're not addressing the memory pressure issue, just >> kicking the can down the road a bit.) > > I agree for the most part, but the 'runWithSufficientStackSpace' doesn't run > in a separate thread, it just wraps it in a function that makes our > diagnostic for running out of memory better. But I think you're right, we > probably should have SOMETHING that lets us exit early or defer constructing > the initializers or something. That's not entirely accurate: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Stack.h#L40 which calls into https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Stack.cpp#L66 It MIGHT get run in a new thread (if that's enabled) or it might not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142401/new/ https://reviews.llvm.org/D142401 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
