[clang-tools-extra] r358400 - [clangd] Wait for compile command in ASTWorker instead of ClangdServer

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 15 05:32:29 PDT 2019


Author: ioeric
Date: Mon Apr 15 05:32:28 2019
New Revision: 358400

URL: http://llvm.org/viewvc/llvm-project?rev=358400&view=rev
Log:
[clangd] Wait for compile command in ASTWorker instead of ClangdServer

Summary:
This makes addDocument non-blocking and would also allow code completion
(in fallback mode) to run when worker waits for the compile command.

Reviewers: sammccall, ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: javed.absar, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D60607

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/Compiler.h
    clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
    clang-tools-extra/trunk/clangd/TUScheduler.cpp
    clang-tools-extra/trunk/clangd/TUScheduler.h
    clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
    clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
    clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Apr 15 05:32:28 2019
@@ -109,7 +109,7 @@ ClangdServer::ClangdServer(const GlobalC
                            const FileSystemProvider &FSProvider,
                            DiagnosticsConsumer &DiagConsumer,
                            const Options &Opts)
-    : CDB(CDB), FSProvider(FSProvider),
+    : FSProvider(FSProvider),
       DynamicIdx(Opts.BuildDynamicSymbolIndex
                      ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
                      : nullptr),
@@ -121,7 +121,7 @@ ClangdServer::ClangdServer(const GlobalC
       // is parsed.
       // FIXME(ioeric): this can be slow and we may be able to index on less
       // critical paths.
-      WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+      WorkScheduler(CDB, Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
                     llvm::make_unique<UpdateIndexCallbacks>(DynamicIdx.get(),
                                                             DiagConsumer),
                     Opts.UpdateDebounce, Opts.RetentionPolicy) {
@@ -155,12 +155,8 @@ void ClangdServer::addDocument(PathRef F
     Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
-  // FIXME: some build systems like Bazel will take time to preparing
-  // environment to build the file, it would be nice if we could emit a
-  // "PreparingBuild" status to inform users, it is non-trivial given the
-  // current implementation.
+  // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.CompileCommand = getCompileCommand(File);
   Inputs.FS = FSProvider.getFileSystem();
   Inputs.Contents = Contents;
   Inputs.Opts = std::move(Opts);
@@ -543,14 +539,6 @@ void ClangdServer::typeHierarchy(PathRef
   WorkScheduler.runWithAST("Type Hierarchy", File, Bind(Action, std::move(CB)));
 }
 
-tooling::CompileCommand ClangdServer::getCompileCommand(PathRef File) {
-  trace::Span Span("GetCompileCommand");
-  llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
-  if (!C) // FIXME: Suppress diagnostics? Let the user know?
-    C = CDB.getFallbackCommand(File);
-  return std::move(*C);
-}
-
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Apr 15 05:32:28 2019
@@ -266,9 +266,6 @@ private:
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
-  tooling::CompileCommand getCompileCommand(PathRef File);
-
-  const GlobalCompilationDatabase &CDB;
   const FileSystemProvider &FSProvider;
 
   Path ResourceDir;

Modified: clang-tools-extra/trunk/clangd/Compiler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.h?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Compiler.h (original)
+++ clang-tools-extra/trunk/clangd/Compiler.h Mon Apr 15 05:32:28 2019
@@ -16,9 +16,9 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
 
 #include "../clang-tidy/ClangTidyOptions.h"
+#include "GlobalCompilationDatabase.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Tooling/CompilationDatabase.h"
 

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Mon Apr 15 05:32:28 2019
@@ -63,10 +63,11 @@ GlobalCompilationDatabase::getFallbackCo
   if (llvm::sys::path::extension(File) == ".h")
     Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
-  return tooling::CompileCommand(llvm::sys::path::parent_path(File),
-                                 llvm::sys::path::filename(File),
-                                 std::move(Argv),
-                                 /*Output=*/"");
+  tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),
+                              llvm::sys::path::filename(File), std::move(Argv),
+                              /*Output=*/"");
+  Cmd.Heuristic = "clangd fallback";
+  return Cmd;
 }
 
 DirectoryBasedGlobalCompilationDatabase::

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Mon Apr 15 05:32:28 2019
@@ -43,10 +43,13 @@
 
 #include "TUScheduler.h"
 #include "Cancellation.h"
+#include "Compiler.h"
+#include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "Trace.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
@@ -154,10 +157,10 @@ class ASTWorkerHandle;
 /// worker.
 class ASTWorker {
   friend class ASTWorkerHandle;
-  ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache,
-            Semaphore &Barrier, bool RunSync,
-            steady_clock::duration UpdateDebounce,
-            bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
+  ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
+            TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync,
+            steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory,
+            ParsingCallbacks &Callbacks);
 
 public:
   /// Create a new ASTWorker and return a handle to it.
@@ -165,12 +168,11 @@ public:
   /// is null, all requests will be processed on the calling thread
   /// synchronously instead. \p Barrier is acquired when processing each
   /// request, it is used to limit the number of actively running threads.
-  static ASTWorkerHandle create(PathRef FileName,
-                                TUScheduler::ASTCache &IdleASTs,
-                                AsyncTaskRunner *Tasks, Semaphore &Barrier,
-                                steady_clock::duration UpdateDebounce,
-                                bool StorePreamblesInMemory,
-                                ParsingCallbacks &Callbacks);
+  static ASTWorkerHandle
+  create(PathRef FileName, const GlobalCompilationDatabase &CDB,
+         TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+         Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+         bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
   ~ASTWorker();
 
   void update(ParseInputs Inputs, WantDiagnostics);
@@ -185,6 +187,9 @@ public:
   /// It may be delivered immediately, or later on the worker thread.
   void getCurrentPreamble(
       llvm::unique_function<void(std::shared_ptr<const PreambleData>)>);
+  /// Returns compile command from the current file inputs.
+  tooling::CompileCommand getCurrentCompileCommand() const;
+
   /// Wait for the first build of preamble to finish. Preamble itself can be
   /// accessed via getPossiblyStalePreamble(). Note that this function will
   /// return after an unsuccessful build of the preamble too, i.e. result of
@@ -215,6 +220,10 @@ private:
   Deadline scheduleLocked();
   /// Should the first task in the queue be skipped instead of run?
   bool shouldSkipHeadLocked() const;
+  /// This is private because `FileInputs.FS` is not thread-safe and thus not
+  /// safe to share. Callers should make sure not to expose `FS` via a public
+  /// interface.
+  std::shared_ptr<const ParseInputs> getCurrentFileInputs() const;
 
   struct Request {
     llvm::unique_function<void()> Action;
@@ -231,6 +240,7 @@ private:
   const steady_clock::duration UpdateDebounce;
   /// File that ASTWorker is responsible for.
   const Path FileName;
+  const GlobalCompilationDatabase &CDB;
   /// Whether to keep the built preambles in memory or on disk.
   const bool StorePreambleInMemory;
   /// Callback invoked when preamble or main file AST is built.
@@ -239,13 +249,15 @@ private:
   TUStatus Status;
 
   Semaphore &Barrier;
-  /// Inputs, corresponding to the current state of AST.
-  ParseInputs FileInputs;
   /// Whether the diagnostics for the current FileInputs were reported to the
   /// users before.
   bool DiagsWereReported = false;
   /// Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
+  /// File inputs, currently being used by the worker.
+  /// Inputs are written and read by the worker thread, compile command can also
+  /// be consumed by clients of ASTWorker.
+  std::shared_ptr<const ParseInputs> FileInputs;         /* GUARDED_BY(Mutex) */
   std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
   /// Becomes ready when the first preamble build finishes.
   Notification PreambleWasBuilt;
@@ -306,14 +318,13 @@ private:
   std::shared_ptr<ASTWorker> Worker;
 };
 
-ASTWorkerHandle ASTWorker::create(PathRef FileName,
-                                  TUScheduler::ASTCache &IdleASTs,
-                                  AsyncTaskRunner *Tasks, Semaphore &Barrier,
-                                  steady_clock::duration UpdateDebounce,
-                                  bool StorePreamblesInMemory,
-                                  ParsingCallbacks &Callbacks) {
+ASTWorkerHandle
+ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,
+                  TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+                  Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+                  bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) {
   std::shared_ptr<ASTWorker> Worker(
-      new ASTWorker(FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks,
+      new ASTWorker(FileName, CDB, IdleASTs, Barrier, /*RunSync=*/!Tasks,
                     UpdateDebounce, StorePreamblesInMemory, Callbacks));
   if (Tasks)
     Tasks->runAsync("worker:" + llvm::sys::path::filename(FileName),
@@ -322,15 +333,23 @@ ASTWorkerHandle ASTWorker::create(PathRe
   return ASTWorkerHandle(std::move(Worker));
 }
 
-ASTWorker::ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache,
-                     Semaphore &Barrier, bool RunSync,
-                     steady_clock::duration UpdateDebounce,
+ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
+                     TUScheduler::ASTCache &LRUCache, Semaphore &Barrier,
+                     bool RunSync, steady_clock::duration UpdateDebounce,
                      bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
     : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
-      FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
+      FileName(FileName), CDB(CDB),
+      StorePreambleInMemory(StorePreamblesInMemory),
       Callbacks(Callbacks), Status{TUAction(TUAction::Idle, ""),
                                    TUStatus::BuildDetails()},
-      Barrier(Barrier), Done(false) {}
+      Barrier(Barrier), Done(false) {
+  auto Inputs = std::make_shared<ParseInputs>();
+  // Set a fallback command because compile command can be accessed before
+  // `Inputs` is initialized. Other fields are only used after initialization
+  // from client inputs.
+  Inputs->CompileCommand = CDB.getFallbackCommand(FileName);
+  FileInputs = std::move(Inputs);
+}
 
 ASTWorker::~ASTWorker() {
   // Make sure we remove the cached AST, if any.
@@ -345,14 +364,28 @@ ASTWorker::~ASTWorker() {
 void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
   llvm::StringRef TaskName = "Update";
   auto Task = [=]() mutable {
+    // Get the actual command as `Inputs` does not have a command.
+    // FIXME: some build systems like Bazel will take time to preparing
+    // environment to build the file, it would be nice if we could emit a
+    // "PreparingBuild" status to inform users, it is non-trivial given the
+    // current implementation.
+    if (auto Cmd = CDB.getCompileCommand(FileName))
+      Inputs.CompileCommand = *Cmd;
+    else
+      // FIXME: consider using old command if it's not a fallback one.
+      Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
+    auto PrevInputs = getCurrentFileInputs();
     // Will be used to check if we can avoid rebuilding the AST.
     bool InputsAreTheSame =
-        std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
+        std::tie(PrevInputs->CompileCommand, PrevInputs->Contents) ==
         std::tie(Inputs.CompileCommand, Inputs.Contents);
 
-    tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand);
+    tooling::CompileCommand OldCommand = PrevInputs->CompileCommand;
     bool PrevDiagsWereReported = DiagsWereReported;
-    FileInputs = Inputs;
+    {
+      std::lock_guard<std::mutex> Lock(Mutex);
+      FileInputs = std::make_shared<ParseInputs>(Inputs);
+    }
     DiagsWereReported = false;
     emitTUStatus({TUAction::BuildingPreamble, TaskName});
     log("Updating file {0} with command {1}\n[{2}]\n{3}", FileName,
@@ -475,15 +508,16 @@ void ASTWorker::runWithAST(
     if (isCancelled())
       return Action(llvm::make_error<CancelledError>());
     llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
+    auto CurrentInputs = getCurrentFileInputs();
     if (!AST) {
       std::unique_ptr<CompilerInvocation> Invocation =
-          buildCompilerInvocation(FileInputs);
+          buildCompilerInvocation(*CurrentInputs);
       // Try rebuilding the AST.
       llvm::Optional<ParsedAST> NewAST =
           Invocation
               ? buildAST(FileName,
                          llvm::make_unique<CompilerInvocation>(*Invocation),
-                         FileInputs, getPossiblyStalePreamble())
+                         *CurrentInputs, getPossiblyStalePreamble())
               : None;
       AST = NewAST ? llvm::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
     }
@@ -494,7 +528,7 @@ void ASTWorker::runWithAST(
     if (!*AST)
       return Action(llvm::make_error<llvm::StringError>(
           "invalid AST", llvm::errc::invalid_argument));
-    Action(InputsAndAST{FileInputs, **AST});
+    Action(InputsAndAST{*CurrentInputs, **AST});
   };
   startTask(Name, Bind(Task, std::move(Action)),
             /*UpdateType=*/None);
@@ -536,6 +570,16 @@ void ASTWorker::getCurrentPreamble(
 
 void ASTWorker::waitForFirstPreamble() const { PreambleWasBuilt.wait(); }
 
+std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  return FileInputs;
+}
+
+tooling::CompileCommand ASTWorker::getCurrentCompileCommand() const {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  return FileInputs->CompileCommand;
+}
+
 std::size_t ASTWorker::getUsedBytes() const {
   // Note that we don't report the size of ASTs currently used for processing
   // the in-flight requests. We used this information for debugging purposes
@@ -774,16 +818,16 @@ FileStatus TUStatus::render(PathRef File
 struct TUScheduler::FileData {
   /// Latest inputs, passed to TUScheduler::update().
   std::string Contents;
-  tooling::CompileCommand Command;
   ASTWorkerHandle Worker;
 };
 
-TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
+TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB,
+                         unsigned AsyncThreadsCount,
                          bool StorePreamblesInMemory,
                          std::unique_ptr<ParsingCallbacks> Callbacks,
                          std::chrono::steady_clock::duration UpdateDebounce,
                          ASTRetentionPolicy RetentionPolicy)
-    : StorePreamblesInMemory(StorePreamblesInMemory),
+    : CDB(CDB), StorePreamblesInMemory(StorePreamblesInMemory),
       Callbacks(Callbacks ? move(Callbacks)
                           : llvm::make_unique<ParsingCallbacks>()),
       Barrier(AsyncThreadsCount),
@@ -822,13 +866,13 @@ void TUScheduler::update(PathRef File, P
   if (!FD) {
     // Create a new worker to process the AST-related tasks.
     ASTWorkerHandle Worker = ASTWorker::create(
-        File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
-        Barrier, UpdateDebounce, StorePreamblesInMemory, *Callbacks);
-    FD = std::unique_ptr<FileData>(new FileData{
-        Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
+        File, CDB, *IdleASTs,
+        WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
+        UpdateDebounce, StorePreamblesInMemory, *Callbacks);
+    FD = std::unique_ptr<FileData>(
+        new FileData{Inputs.Contents, std::move(Worker)});
   } else {
     FD->Contents = Inputs.Contents;
-    FD->Command = Inputs.CompileCommand;
   }
   FD->Worker->update(std::move(Inputs), WantDiags);
 }
@@ -876,7 +920,8 @@ void TUScheduler::runWithPreamble(llvm::
     SPAN_ATTACH(Tracer, "file", File);
     std::shared_ptr<const PreambleData> Preamble =
         It->second->Worker->getPossiblyStalePreamble();
-    Action(InputsAndPreamble{It->second->Contents, It->second->Command,
+    Action(InputsAndPreamble{It->second->Contents,
+                             It->second->Worker->getCurrentCompileCommand(),
                              Preamble.get()});
     return;
   }
@@ -923,7 +968,7 @@ void TUScheduler::runWithPreamble(llvm::
   PreambleTasks->runAsync(
       "task:" + llvm::sys::path::filename(File),
       Bind(Task, std::string(Name), std::string(File), It->second->Contents,
-           It->second->Command,
+           Worker->getCurrentCompileCommand(),
            Context::current().derive(kFileBeingProcessed, File),
            std::move(ConsistentPreamble), std::move(Action)));
 }

Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Mon Apr 15 05:32:28 2019
@@ -11,6 +11,7 @@
 
 #include "ClangdUnit.h"
 #include "Function.h"
+#include "GlobalCompilationDatabase.h"
 #include "Threading.h"
 #include "index/CanonicalIncludes.h"
 #include "llvm/ADT/Optional.h"
@@ -125,7 +126,8 @@ public:
 /// FIXME(sammccall): pull out a scheduler options struct.
 class TUScheduler {
 public:
-  TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
+  TUScheduler(const GlobalCompilationDatabase &CDB, unsigned AsyncThreadsCount,
+              bool StorePreamblesInMemory,
               std::unique_ptr<ParsingCallbacks> ASTCallbacks,
               std::chrono::steady_clock::duration UpdateDebounce,
               ASTRetentionPolicy RetentionPolicy);
@@ -141,10 +143,11 @@ public:
   std::vector<Path> getFilesWithCachedAST() const;
 
   /// Schedule an update for \p File. Adds \p File to a list of tracked files if
-  /// \p File was not part of it before.
-  /// If diagnostics are requested (Yes), and the context is cancelled before
-  /// they are prepared, they may be skipped if eventual-consistency permits it
-  /// (i.e. WantDiagnostics is downgraded to Auto).
+  /// \p File was not part of it before. The compile command in \p Inputs is
+  /// ignored; worker queries CDB to get the actual compile command.
+  /// If diagnostics are requested (Yes), and the context is cancelled
+  /// before they are prepared, they may be skipped if eventual-consistency
+  /// permits it (i.e. WantDiagnostics is downgraded to Auto).
   void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
   /// Remove \p File from the list of tracked files and schedule removal of its
@@ -217,6 +220,7 @@ public:
   static llvm::Optional<llvm::StringRef> getFileBeingProcessedInContext();
 
 private:
+  const GlobalCompilationDatabase &CDB;
   const bool StorePreamblesInMemory;
   std::unique_ptr<ParsingCallbacks> Callbacks; // not nullptr
   Semaphore Barrier;

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Mon Apr 15 05:32:28 2019
@@ -1099,6 +1099,64 @@ TEST_F(ClangdVFSTest, FallbackWhenPreamb
                                 Field(&CodeCompletion::Scope, "ns::"))));
 }
 
+TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  // Returns compile command only when notified.
+  class DelayedCompilationDatabase : public GlobalCompilationDatabase {
+  public:
+    DelayedCompilationDatabase(Notification &CanReturnCommand)
+        : CanReturnCommand(CanReturnCommand) {}
+
+    llvm::Optional<tooling::CompileCommand>
+    getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
+      // FIXME: make this timeout and fail instead of waiting forever in case
+      // something goes wrong.
+      CanReturnCommand.wait();
+      auto FileName = llvm::sys::path::filename(File);
+      std::vector<std::string> CommandLine = {"clangd", "-ffreestanding", File};
+      return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
+                                      FileName, std::move(CommandLine), "")};
+    }
+
+    std::vector<std::string> ExtraClangFlags;
+
+  private:
+    Notification &CanReturnCommand;
+  };
+
+  Notification CanReturnCommand;
+  DelayedCompilationDatabase CDB(CanReturnCommand);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  Annotations Code(R"cpp(
+    namespace ns { int xyz; }
+    using namespace ns;
+    int main() {
+       xy^
+    })cpp");
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, Code.code());
+
+  // Sleep for some time to make sure code completion is not run because update
+  // hasn't been scheduled.
+  std::this_thread::sleep_for(std::chrono::milliseconds(10));
+  auto Opts = clangd::CodeCompleteOptions();
+  Opts.AllowFallback = true;
+
+  auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
+  EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
+
+  CanReturnCommand.notify();
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(),
+                                       clangd::CodeCompleteOptions()))
+                  .Completions,
+              ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
+                                Field(&CodeCompletion::Scope, "ns::"))));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon Apr 15 05:32:28 2019
@@ -1386,7 +1386,7 @@ TEST(CompletionTest, NonDocComments) {
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
   CDB.ExtraClangFlags.push_back("-fno-delayed-template-parsing");
-  Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  runAddDocument(Server, FooCpp, Source.code(), WantDiagnostics::Yes);
   CodeCompleteResult Completions = cantFail(runCodeComplete(
       Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));
 

Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=358400&r1=358399&r2=358400&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Mon Apr 15 05:32:28 2019
@@ -21,8 +21,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-using ::testing::_;
-using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
@@ -103,7 +101,7 @@ Key<llvm::unique_function<void(PathRef F
     TUSchedulerTests::DiagsCallbackKey;
 
 TEST_F(TUSchedulerTests, MissingFiles) {
-  TUScheduler S(getDefaultAsyncThreadsCount(),
+  TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                 /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
@@ -154,7 +152,7 @@ TEST_F(TUSchedulerTests, WantDiagnostics
     // thread until we've scheduled them all.
     Notification Ready;
     TUScheduler S(
-        getDefaultAsyncThreadsCount(),
+        CDB, getDefaultAsyncThreadsCount(),
         /*StorePreamblesInMemory=*/true, captureDiags(),
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
@@ -184,7 +182,7 @@ TEST_F(TUSchedulerTests, WantDiagnostics
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic<int> CallbackCount(0);
   {
-    TUScheduler S(getDefaultAsyncThreadsCount(),
+    TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                   /*StorePreamblesInMemory=*/true, captureDiags(),
                   /*UpdateDebounce=*/std::chrono::seconds(1),
                   ASTRetentionPolicy());
@@ -220,7 +218,7 @@ TEST_F(TUSchedulerTests, PreambleConsist
   {
     Notification InconsistentReadDone; // Must live longest.
     TUScheduler S(
-        getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
+        CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
         /*ASTCallbacks=*/nullptr,
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
@@ -277,7 +275,7 @@ TEST_F(TUSchedulerTests, Cancellation) {
   {
     Notification Proceed; // Ensure we schedule everything.
     TUScheduler S(
-        getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
+        CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
         /*ASTCallbacks=*/captureDiags(),
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
@@ -346,7 +344,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
 
   // Run TUScheduler and collect some stats.
   {
-    TUScheduler S(getDefaultAsyncThreadsCount(),
+    TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                   /*StorePreamblesInMemory=*/true, captureDiags(),
                   /*UpdateDebounce=*/std::chrono::milliseconds(50),
                   ASTRetentionPolicy());
@@ -437,10 +435,11 @@ TEST_F(TUSchedulerTests, EvictedAST) {
   std::atomic<int> BuiltASTCounter(0);
   ASTRetentionPolicy Policy;
   Policy.MaxRetainedASTs = 2;
-  TUScheduler S(
-      /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-      /*ASTCallbacks=*/nullptr,
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
+  TUScheduler S(CDB,
+                /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
+                /*ASTCallbacks=*/nullptr,
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
     int* a;
@@ -487,11 +486,11 @@ TEST_F(TUSchedulerTests, EvictedAST) {
 }
 
 TEST_F(TUSchedulerTests, EmptyPreamble) {
-  TUScheduler S(
-      /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-      /*ASTCallbacks=*/nullptr,
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
-      ASTRetentionPolicy());
+  TUScheduler S(CDB,
+                /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
+                /*ASTCallbacks=*/nullptr,
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
 
   auto Foo = testPath("foo.cpp");
   auto Header = testPath("foo.h");
@@ -532,11 +531,11 @@ TEST_F(TUSchedulerTests, EmptyPreamble)
 TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
   // Testing strategy: we update the file and schedule a few preamble reads at
   // the same time. All reads should get the same non-null preamble.
-  TUScheduler S(
-      /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-      /*ASTCallbacks=*/nullptr,
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
-      ASTRetentionPolicy());
+  TUScheduler S(CDB,
+                /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
+                /*ASTCallbacks=*/nullptr,
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
   auto NonEmptyPreamble = R"cpp(
     #define FOO 1
@@ -564,11 +563,11 @@ TEST_F(TUSchedulerTests, RunWaitsForPrea
 }
 
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
-  TUScheduler S(
-      /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-      /*StorePreambleInMemory=*/true, captureDiags(),
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
-      ASTRetentionPolicy());
+  TUScheduler S(CDB,
+                /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+                /*StorePreambleInMemory=*/true, captureDiags(),
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
 
   auto Source = testPath("foo.cpp");
   auto Header = testPath("foo.h");
@@ -617,11 +616,11 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChan
 }
 
 TEST_F(TUSchedulerTests, NoChangeDiags) {
-  TUScheduler S(
-      /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-      /*StorePreambleInMemory=*/true, captureDiags(),
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
-      ASTRetentionPolicy());
+  TUScheduler S(CDB,
+                /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+                /*StorePreambleInMemory=*/true, captureDiags(),
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
 
   auto FooCpp = testPath("foo.cpp");
   auto Contents = "int a; int b;";
@@ -652,7 +651,7 @@ TEST_F(TUSchedulerTests, NoChangeDiags)
 }
 
 TEST_F(TUSchedulerTests, Run) {
-  TUScheduler S(/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+  TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
                 /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());




More information about the cfe-commits mailing list