[PATCH] D158967: [clang][clangd] Ensure the stack bottom before building AST

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D158967#4621361 <https://reviews.llvm.org/D158967#4621361>, @zyounan wrote:

>> Currently the code in CompilerInstance::ExecuteAction seems to acknowledge that there should be a fallback. I'm suggesting to move this fallback down to a function that actually runs parsing.
>
> One thing I'm afraid of is, that there are/were some compatible reasons that made us decide not to insert this call at `ASTFrontendAction::ExecuteAction` nor `clang::ParseAST`. (I didn't see the explanation in D66361 <https://reviews.llvm.org/D66361>. Richard, could you kindly explain why? @rsmith)

D66361 <https://reviews.llvm.org/D66361> wasn't intended to cover all possible uses by itself. The recovery from deep recursion is best-effort, and it was expected that some tools that call into Clang in less common ways would need to manually call `noteBottomOfStack` if they wanted to enable this recovery. I put the fallback call in `CompilerInstance::ExecuteAction` because I thought it would do the right thing in most cases, and not require any effort for most tools. Adding another fallback in `ASTFrontendAction::ExecuteAction` seems OK to me if that's an entry point that's commonly used by tools. (Where is that being called from in clangd's case? Is there a higher point in the call stack where we could put the call to `noteBottomOfStack` instead?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967