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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
