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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 29 13:03:41 PDT 2023


sammccall added a comment.

In D158967#4626156 <https://reviews.llvm.org/D158967#4626156>, @rsmith wrote:

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

clangd calls `FrontendAction::Execute()` (in `clangd::ParsedAST::build`, in `clang::PrecompiledPreamble::Build`, and in `clangd::BackgroundIndex::index`). I doubt we're the only one.
All of these run somewhere under `main()` or in a thread spawned by an `AsyncTaskRunner`.

So to me this fallback makes sense, but we should also add calls to those two places so we really get the bottom of the stack.

(If noteBottomOfStack() were part of llvm support, then this could be built into llvm::Thread...)



================
Comment at: clang-tools-extra/clangd/tool/Check.cpp:445
            const ClangdLSPServer::Options &Opts) {
+  clang::noteBottomOfStack();
   std::optional<Range> LineRange;
----------------
I like the idea that we only parse on the main thread with `-check`, but it's not quite true: we also do this with `-sync`. I think this should go in main() instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967



More information about the cfe-commits mailing list