aaron.ballman added a comment.

Two things to note:

1. I'm not convinced that adding a test case is the right thing to do here. On 
the one hand, we want to verify that we now emit the diagnostic before 
crashing. But on the other hand, this test is extremely expensive to run 
(because it exhausts the stack) and its behavior is highly specific to the test 
platform its being run on. If someone asked me to remove the test coverage 
here, I would not be sad.
2. The `runWithSufficientStackSpace()` implementation is a copy of what's 
already in Sema. I investigated lifting this interface to something more 
common, but the issue is with generating a diagnostic (there's no common 
diagnostic engine between `Sema` and `Parser` that I could find). Given the 
simplicity of the implementation, I figured it's reasonably defensible to 
duplicate the code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124915/new/

https://reviews.llvm.org/D124915

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to