[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 8 07:07:58 PDT 2019


ioeric added inline comments.


================
Comment at: clangd/TUScheduler.cpp:881
+  // asynchronous mode, as TU update should finish before this is run.
+  if (!It->second->Worker->isFirstPreambleBuilt() &&
+      Consistency == StaleOrAbsent && PreambleTasks) {
----------------
sammccall wrote:
> Does this need to be a special case at the top of TUScheduler::runWithPreamble, rather than unified with the rest of the body?
> 
> e.g. the `if (!PreambleTasks)` appears to block appears to handle that case correctly already, and the `Consistency == Consistent` block won't trigger.
> I think all you need to do is make the `Worker->waitForFirstPreamble()` line conditional?
> 
> (This means no changes to ASTWorker, Notification etc)
whoops! absolutely right!


================
Comment at: unittests/clangd/ClangdTests.cpp:1080
+  // This will make compile command broken and preamble absent.
+  CDB.ExtraClangFlags = {"yolo.cc"};
+  Server.addDocument(FooCpp, Code.code());
----------------
sammccall wrote:
> (`-this-flag-does-not-exist` might be clearer)
This would still cause preamble to be built:
```
Updating file /clangd-test/foo.cpp with command [/clangd-test] clang -this-flag-does-not-exist /clangd-test/foo.cpp
Ignored diagnostic. unknown argument: '-this-flag-does-not-exist'
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59811





More information about the cfe-commits mailing list