[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: unittests/clangd/ClangdTests.cpp:1148
+  EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
+  // Identifier-based fallback completion doesn't know about "symbol" scope.
+  EXPECT_THAT(Res.Completions,
----------------
ilya-biryukov wrote:
> Not strictly related to this patch, but maybe we could add a flag to completion results to indicate if the completion happened via a fallback mode or not?
> 
> Would make the test code more straightforward and the tests like these would not rely on a particular implementation of the fallback mode (e.g. I can imagine the fallback mode learning about the scopes later on)
We are setting the context to `Recovery` and make fallback as part of `Recovery`. Do you we should distinguish fallback mode from `Recovery`?


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:1390
   Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   CodeCompleteResult Completions = cantFail(runCodeComplete(
----------------
ilya-biryukov wrote:
> Could you expand why we need this?
Added a comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60607