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

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a subscriber: rsmith.
zyounan added a comment.

Thanks for the reply. And I understand (and agree with) the point that it's better to solve this problem once and for all.

> 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)

> That's `CompilerInstance::ExecuteAction`

Sorry for my mistake. I thought placing that in `CompilerInstance` was enough for most tools since it's less likely for developers to invoke the FrontendAction on their own, and for "advanced" users (like clangd) who'd like to control every step handling the AST, maybe it's fine to give them the opportunity to decide if it should crash on infinite recursion?

However, creating such a discrepancy doesn't make much sense. I'm happy to move this call to libclang if this won't break many things.

> Could you clarify what kind of encapsulation would it break?

(That's just my spitballing, please don't mind. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967