ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, bkramer.
Herald added a subscriber: klimek.

We now provide an abstraction of Scheduler that abstracts threading
and resource management in ClangdServer.
No changes to behavior are intended with an exception of changed error
messages.
This patch is preliminary work to allow a revamped threading
implementation that will move the threading code out of CppFile.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.h

Index: clangd/ClangdUnitStore.h
===================================================================
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -27,27 +27,26 @@
 public:
   /// \p ASTCallback is called when a file is parsed synchronously. This should
   /// not be expensive since it blocks diagnostics.
-  explicit CppFileCollection(ASTParsedCallback ASTCallback)
-      : ASTCallback(std::move(ASTCallback)) {}
+  explicit CppFileCollection(bool StorePreamblesInMemory,
+                             std::shared_ptr<PCHContainerOperations> PCHs,
+                             ASTParsedCallback ASTCallback)
+      : ASTCallback(std::move(ASTCallback)), PCHs(std::move(PCHs)),
+        StorePreamblesInMemory(StorePreamblesInMemory) {}
 
-  std::shared_ptr<CppFile>
-  getOrCreateFile(PathRef File, PathRef ResourceDir,
-                  bool StorePreamblesInMemory,
-                  std::shared_ptr<PCHContainerOperations> PCHs) {
+  std::shared_ptr<CppFile> getOrCreateFile(PathRef File) {
     std::lock_guard<std::mutex> Lock(Mutex);
     auto It = OpenedFiles.find(File);
     if (It == OpenedFiles.end()) {
       It = OpenedFiles
                .try_emplace(File, CppFile::Create(File, StorePreamblesInMemory,
-                                                  std::move(PCHs), ASTCallback))
+                                                  PCHs, ASTCallback))
                .first;
     }
     return It->second;
   }
 
-  std::shared_ptr<CppFile> getFile(PathRef File) {
+  std::shared_ptr<CppFile> getFile(PathRef File) const {
     std::lock_guard<std::mutex> Lock(Mutex);
-
     auto It = OpenedFiles.find(File);
     if (It == OpenedFiles.end())
       return nullptr;
@@ -59,9 +58,11 @@
   std::shared_ptr<CppFile> removeIfPresent(PathRef File);
 
 private:
-  std::mutex Mutex;
+  mutable std::mutex Mutex;
   llvm::StringMap<std::shared_ptr<CppFile>> OpenedFiles;
   ASTParsedCallback ASTCallback;
+  std::shared_ptr<PCHContainerOperations> PCHs;
+  bool StorePreamblesInMemory;
 };
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -64,11 +64,8 @@
   ParseInputs(tooling::CompileCommand CompileCommand,
               IntrusiveRefCntPtr<vfs::FileSystem> FS, std::string Contents);
 
-  /// Compilation arguments.
   tooling::CompileCommand CompileCommand;
-  /// FileSystem.
   IntrusiveRefCntPtr<vfs::FileSystem> FS;
-  /// Contents to parse.
   std::string Contents;
 };
 
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -98,21 +98,20 @@
 
 class ClangdServer;
 
-/// Returns a number of a default async threads to use for ClangdScheduler.
+/// Returns a number of a default async threads to use for Scheduler.
 /// Returned value is always >= 1 (i.e. will not cause requests to be processed
 /// synchronously).
 unsigned getDefaultAsyncThreadsCount();
 
-/// Handles running WorkerRequests of ClangdServer on a number of worker
-/// threads.
-class ClangdScheduler {
+/// A simple fixed-size thread pool implementation.
+class SimpleThreadPool {
 public:
   /// If \p AsyncThreadsCount is 0, requests added using addToFront and addToEnd
   /// will be processed synchronously on the calling thread.
   // Otherwise, \p AsyncThreadsCount threads will be created to schedule the
   // requests.
-  ClangdScheduler(unsigned AsyncThreadsCount);
-  ~ClangdScheduler();
+  SimpleThreadPool(unsigned AsyncThreadsCount);
+  ~SimpleThreadPool();
 
   /// Add a new request to run function \p F with args \p As to the start of the
   /// queue. The request will be run on a separate thread.
@@ -149,20 +148,64 @@
 
 private:
   bool RunSynchronously;
-  std::mutex Mutex;
+  mutable std::mutex Mutex;
   /// We run some tasks on separate threads(parsing, CppFile cleanup).
   /// These threads looks into RequestQueue to find requests to handle and
   /// terminate when Done is set to true.
   std::vector<std::thread> Workers;
   /// Setting Done to true will make the worker threads terminate.
   bool Done = false;
-  /// A queue of requests. Elements of this vector are async computations (i.e.
-  /// results of calling std::async(std::launch::deferred, ...)).
+  /// A queue of requests.
   std::deque<UniqueFunction<void()>> RequestQueue;
   /// Condition variable to wake up worker threads.
   std::condition_variable RequestCV;
 };
 
+/// Handles running tasks for ClangdServer and managing the resources (e.g.,
+/// preambles and ASTs) for opened files.
+class Scheduler {
+public:
+  Scheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
+            std::shared_ptr<PCHContainerOperations> PCHs,
+            ASTParsedCallback ASTCallback);
+
+  /// Get the CompileCommand passed at the last scheduleUpdate call for \p File.
+  llvm::Optional<tooling::CompileCommand> getLatestCommand(PathRef File) 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.
+  void scheduleUpdate(
+      Context Ctx, PathRef File, ParseInputs Inputs,
+      UniqueFunction<void(Context, llvm::Optional<std::vector<DiagWithFixIts>>)>
+          OnUpdated);
+
+  /// Remove \p File from the list of tracked files and schedule removal of its
+  /// resources. \p Action will be called when resources are freed.
+  /// If an error occurs during processing, it is forwarded to the \p Action
+  /// callback.
+  void scheduleRemove(PathRef File, UniqueFunction<void(llvm::Error)> Action);
+
+  /// Schedule an async read of the AST. \p Action will be called when AST is
+  /// ready.
+  /// If an error occurs during processing, it is forwarded to the \p Action
+  /// callback. If there was no error, AST is never null.
+  void
+  scheduleASTRead(PathRef File,
+                  UniqueFunction<void(llvm::Expected<ParsedAST &>)> Action);
+
+  /// Schedule an async read of the Preamble. Preamble passed to \p Action may
+  /// be stale.
+  /// If an error occurs during processing, it is forwarded to the \p Action
+  /// callback.
+  void schedulePreambleRead(
+      PathRef File,
+      UniqueFunction<void(llvm::Expected<const PreambleData *>)> Action);
+
+private:
+  CppFileCollection Units;
+  SimpleThreadPool Executor;
+};
+
 /// Provides API to manage ASTs for a collection of C++ files and request
 /// various language features.
 /// Currently supports async diagnostics, code completion, formatting and goto
@@ -330,13 +373,9 @@
 
   std::future<Context>
   scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents,
-                          std::shared_ptr<CppFile> Resources,
                           Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS,
                           bool UpdateCompileCommand);
 
-  std::future<Context>
-  scheduleCancelRebuild(Context Ctx, std::shared_ptr<CppFile> Resources);
-
   GlobalCompilationDatabase &CDB;
   DiagnosticsConsumer &DiagConsumer;
   FileSystemProvider &FSProvider;
@@ -351,22 +390,20 @@
   std::unique_ptr<FileIndex> FileIdx;
   // If present, a merged view of FileIdx and an external index. Read via Index.
   std::unique_ptr<SymbolIndex> MergedIndex;
-  CppFileCollection Units;
   std::string ResourceDir;
   // If set, this represents the workspace path.
   llvm::Optional<std::string> RootPath;
   std::shared_ptr<PCHContainerOperations> PCHs;
-  bool StorePreamblesInMemory;
   /// Used to serialize diagnostic callbacks.
   /// FIXME(ibiryukov): get rid of an extra map and put all version counters
   /// into CppFile.
   std::mutex DiagnosticsMutex;
   /// Maps from a filename to the latest version of reported diagnostics.
   llvm::StringMap<DocVersion> ReportedDiagnosticVersions;
   // WorkScheduler has to be the last member, because its destructor has to be
   // called before all other members to stop the worker thread that references
-  // ClangdServer
-  ClangdScheduler WorkScheduler;
+  // ClangdServer.
+  Scheduler WorkScheduler;
 };
 
 } // namespace clangd
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -32,6 +32,26 @@
 
 namespace {
 
+// Issues an async read of AST and waits for results.
+template <class Ret, class Func>
+Ret waitForASTAction(Scheduler &S, PathRef File, Func &&F) {
+  std::packaged_task<Ret(llvm::Expected<ParsedAST &>)> Task(
+      std::forward<Func>(F));
+  auto Future = Task.get_future();
+  S.scheduleASTRead(File, std::move(Task));
+  return Future.get();
+}
+
+// Issues an async read of preamble and waits for results.
+template <class Ret, class Func>
+Ret waitForPreambleAction(Scheduler &S, PathRef File, Func &&F) {
+  std::packaged_task<Ret(llvm::Expected<const PreambleData *>)> Task(
+      std::forward<Func>(F));
+  auto Future = Task.get_future();
+  S.schedulePreambleRead(File, std::move(Task));
+  return Future.get();
+}
+
 tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB,
                                           PathRef File, PathRef ResourceDir) {
   llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
@@ -87,8 +107,7 @@
     return 1;
   return HardwareConcurrency;
 }
-
-ClangdScheduler::ClangdScheduler(unsigned AsyncThreadsCount)
+SimpleThreadPool::SimpleThreadPool(unsigned AsyncThreadsCount)
     : RunSynchronously(AsyncThreadsCount == 0) {
   if (RunSynchronously) {
     // Don't start the worker thread if we're running synchronously
@@ -114,7 +133,7 @@
           assert(!RequestQueue.empty() && "RequestQueue was empty");
 
           // We process requests starting from the front of the queue. Users of
-          // ClangdScheduler have a way to prioritise their requests by putting
+          // SimpleThreadPool have a way to prioritise their requests by putting
           // them to the either side of the queue (using either addToEnd or
           // addToFront).
           Request = std::move(RequestQueue.front());
@@ -127,7 +146,7 @@
   }
 }
 
-ClangdScheduler::~ClangdScheduler() {
+SimpleThreadPool::~SimpleThreadPool() {
   if (RunSynchronously)
     return; // no worker thread is running in that case
 
@@ -142,6 +161,99 @@
     Worker.join();
 }
 
+Scheduler::Scheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
+                     std::shared_ptr<PCHContainerOperations> PCHs,
+                     ASTParsedCallback ASTCallback)
+    : Units(StorePreamblesInMemory, std::move(PCHs), std::move(ASTCallback)),
+      Executor(AsyncThreadsCount) {}
+
+llvm::Optional<tooling::CompileCommand>
+Scheduler::getLatestCommand(PathRef File) const {
+  auto Resources = Units.getFile(File);
+  if (!Resources)
+    return llvm::None;
+  return Resources->getLatestCommand();
+}
+
+void Scheduler::scheduleUpdate(
+    Context Ctx, PathRef File, ParseInputs Inputs,
+    UniqueFunction<void(Context Ctx,
+                        llvm::Optional<std::vector<DiagWithFixIts>>)>
+        OnUpdated) {
+  auto Resources = Units.getOrCreateFile(File);
+  auto DeferredRebuild = Resources->deferRebuild(std::move(Inputs));
+
+  Executor.addToFront(
+      [](Context Ctx, decltype(OnUpdated) OnUpdated,
+         decltype(DeferredRebuild) DeferredRebuild) {
+        auto Diags = DeferredRebuild(Ctx);
+        OnUpdated(std::move(Ctx), Diags);
+      },
+      std::move(Ctx), std::move(OnUpdated), std::move(DeferredRebuild));
+}
+
+void Scheduler::scheduleRemove(PathRef File,
+                               UniqueFunction<void(llvm::Error)> Action) {
+  auto Resources = Units.removeIfPresent(File);
+  if (!Resources) {
+    Action(llvm::make_error<llvm::StringError>(
+        "trying to remove non-added document", llvm::errc::invalid_argument));
+    return;
+  }
+
+  auto DeferredCancel = Resources->deferCancelRebuild();
+  Executor.addToFront(
+      [](decltype(Action) Action, decltype(DeferredCancel) DeferredCancel) {
+        DeferredCancel();
+        Action(llvm::Error::success());
+      },
+      std::move(Action), std::move(DeferredCancel));
+}
+
+void Scheduler::scheduleASTRead(
+    PathRef File, UniqueFunction<void(llvm::Expected<ParsedAST &>)> Action) {
+  auto Resources = Units.getFile(File);
+  if (!Resources) {
+    Action(llvm::make_error<llvm::StringError>(
+        "trying to get AST for non-added document",
+        llvm::errc::invalid_argument));
+    return;
+  }
+  // We currently do all the reads of the AST on a running thread to avoid
+  // inconsistent states coming from subsequent updates.
+  Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) {
+    if (AST)
+      Action(*AST);
+    else
+      Action(llvm::make_error<llvm::StringError>(
+          "Could not build AST for the latest file update",
+          llvm::errc::invalid_argument));
+  });
+}
+
+void Scheduler::schedulePreambleRead(
+    PathRef File,
+    UniqueFunction<void(llvm::Expected<const PreambleData *>)> Action) {
+  std::shared_ptr<CppFile> Resources = Units.getFile(File);
+  if (!Resources) {
+    Action(llvm::make_error<llvm::StringError>(
+        "trying to get preamble for non-added document",
+        llvm::errc::invalid_argument));
+    return;
+  }
+
+  std::shared_ptr<const PreambleData> Preamble =
+      Resources->getPossiblyStalePreamble();
+  Executor.addToFront(
+      [Resources, Preamble](decltype(Action) Action) mutable {
+        if (!Preamble)
+          Preamble = Resources->getPossiblyStalePreamble();
+
+        Action(Preamble.get());
+      },
+      std::move(Action));
+}
+
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
                            DiagnosticsConsumer &DiagConsumer,
                            FileSystemProvider &FSProvider,
@@ -151,19 +263,18 @@
                            llvm::Optional<StringRef> ResourceDir)
     : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
       FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
-      // Pass a callback into `Units` to extract symbols from a newly parsed
-      // file and rebuild the file index synchronously each time an AST is
-      // parsed.
-      // FIXME(ioeric): this can be slow and we may be able to index on less
-      // critical paths.
-      Units(FileIdx
-                ? [this](const Context &Ctx, PathRef Path,
-                         ParsedAST *AST) { FileIdx->update(Ctx, Path, AST); }
-                : ASTParsedCallback()),
       ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
       PCHs(std::make_shared<PCHContainerOperations>()),
-      StorePreamblesInMemory(StorePreamblesInMemory),
-      WorkScheduler(AsyncThreadsCount) {
+      // Pass a callback into `WorkScheduler` to extract symbols from a newly
+      // parsed file and rebuild the file index synchronously each time an AST
+      // is parsed.
+      // FIXME(ioeric): this can be slow and we may be able to index on less
+      // critical paths.
+      WorkScheduler(
+          AsyncThreadsCount, StorePreamblesInMemory, this->PCHs,
+          FileIdx ? [this](const Context &Ctx, PathRef Path,
+                           ParsedAST *AST) { FileIdx->update(Ctx, Path, AST); }
+                  : ASTParsedCallback()) {
   if (FileIdx && StaticIdx) {
     MergedIndex = mergeIndex(FileIdx.get(), StaticIdx);
     Index = MergedIndex.get();
@@ -185,33 +296,39 @@
 std::future<Context> ClangdServer::addDocument(Context Ctx, PathRef File,
                                                StringRef Contents) {
   DocVersion Version = DraftMgr.updateDraft(File, Contents);
-
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  std::shared_ptr<CppFile> Resources =
-      Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs);
   return scheduleReparseAndDiags(std::move(Ctx), File,
                                  VersionedDraft{Version, Contents.str()},
-                                 std::move(Resources), std::move(TaggedFS),
-                                 /*UpdateCompileCommand=*/false);
+                                 TaggedFS, /*UpdateCompileCommand=*/false);
 }
 
 std::future<Context> ClangdServer::removeDocument(Context Ctx, PathRef File) {
   DraftMgr.removeDraft(File);
-  std::shared_ptr<CppFile> Resources = Units.removeIfPresent(File);
-  return scheduleCancelRebuild(std::move(Ctx), std::move(Resources));
+
+  std::promise<Context> DonePromise;
+  std::future<Context> DoneFuture = DonePromise.get_future();
+
+  auto Callback = BindWithForward(
+      [](Context Ctx, std::promise<Context> DonePromise, llvm::Error Err) {
+        // FIXME(ibiryukov): allow to report error to the caller.
+        // Discard error, if any.
+        (void)(bool) Err;
+        DonePromise.set_value(std::move(Ctx));
+      },
+      std::move(Ctx), std::move(DonePromise));
+
+  WorkScheduler.scheduleRemove(File, std::move(Callback));
+  return DoneFuture;
 }
 
 std::future<Context> ClangdServer::forceReparse(Context Ctx, PathRef File) {
   auto FileContents = DraftMgr.getDraft(File);
   assert(FileContents.Draft &&
          "forceReparse() was called for non-added document");
 
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  std::shared_ptr<CppFile> Resources =
-      Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs);
-  return scheduleReparseAndDiags(std::move(Ctx), File, FileContents,
-                                 std::move(Resources), std::move(TaggedFS),
-                                 /*UpdateCompileCommand=*/true);
+  return scheduleReparseAndDiags(std::move(Ctx), File, std::move(FileContents),
+                                 TaggedFS, /*UpdateCompileCommand=*/true);
 }
 
 std::future<std::pair<Context, Tagged<CompletionList>>>
@@ -243,104 +360,92 @@
     IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
   using CallbackType = UniqueFunction<void(Context, Tagged<CompletionList>)>;
 
-  std::string Contents;
-  if (OverridenContents) {
-    Contents = *OverridenContents;
-  } else {
-    auto FileContents = DraftMgr.getDraft(File);
-    assert(FileContents.Draft &&
-           "codeComplete is called for non-added document");
-
-    Contents = std::move(*FileContents.Draft);
-  }
-
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
   if (UsedFS)
     *UsedFS = TaggedFS.Value;
 
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  assert(Resources && "Calling completion on non-added file");
-
-  // Remember the current Preamble and use it when async task starts executing.
-  // At the point when async task starts executing, we may have a different
-  // Preamble in Resources. However, we assume the Preamble that we obtain here
-  // is reusable in completion more often.
-  std::shared_ptr<const PreambleData> Preamble =
-      Resources->getPossiblyStalePreamble();
   // Copy completion options for passing them to async task handler.
   auto CodeCompleteOpts = Opts;
   if (!CodeCompleteOpts.Index) // Respect overridden index.
     CodeCompleteOpts.Index = Index;
 
-  // Copy File, as it is a PathRef that will go out of scope before Task is
-  // executed.
-  Path FileStr = File;
-  // Copy PCHs to avoid accessing this->PCHs concurrently
-  std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
+  auto Command = WorkScheduler.getLatestCommand(File);
+  assert(Command && "codeComplete called for non-added document");
 
-  assert(Resources->getLatestCommand() &&
-         "CppFile is in inconsistent state, missing CompileCommand");
-  tooling::CompileCommand CompileCommand = *Resources->getLatestCommand();
-
-  // A task that will be run asynchronously.
-  auto Task =
-      // 'mutable' to reassign Preamble variable.
-      [FileStr, Preamble, Resources, Contents, Pos, CodeCompleteOpts, TaggedFS,
-       PCHs, CompileCommand](Context Ctx, CallbackType Callback) mutable {
-        if (!Preamble) {
-          // Maybe we built some preamble before processing this request.
-          Preamble = Resources->getPossiblyStalePreamble();
-        }
-        // FIXME(ibiryukov): even if Preamble is non-null, we may want to check
-        // both the old and the new version in case only one of them matches.
-        CompletionList Result = clangd::codeComplete(
-            Ctx, FileStr, CompileCommand,
-            Preamble ? &Preamble->Preamble : nullptr, Contents, Pos,
-            TaggedFS.Value, PCHs, CodeCompleteOpts);
+  std::string Contents;
+  if (OverridenContents) {
+    Contents = OverridenContents->str();
+  } else {
+    VersionedDraft Latest = DraftMgr.getDraft(File);
+    assert(Latest.Draft && "codeComplete called for non-added document");
+    Contents = *Latest.Draft;
+  }
 
-        Callback(std::move(Ctx),
-                 make_tagged(std::move(Result), std::move(TaggedFS.Tag)));
-      };
+  // Copy PCHs to avoid accessing this->PCHs concurrently
+  std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
+  auto Task = [PCHs, Pos, TaggedFS, Command, CodeCompleteOpts](
+                  Context Ctx, std::string Contents, Path File,
+                  CallbackType Callback,
+                  llvm::Expected<const PreambleData *> PreambleErr) {
+    assert(PreambleErr &&
+           "error when trying to read preamble for codeComplete");
+    auto Preamble = *PreambleErr;
+
+    // FIXME(ibiryukov): even if Preamble is non-null, we may want to check
+    // both the old and the new version in case only one of them matches.
+    CompletionList Result = clangd::codeComplete(
+        Ctx, File, *Command, Preamble ? &Preamble->Preamble : nullptr, Contents,
+        Pos, TaggedFS.Value, PCHs, CodeCompleteOpts);
+
+    Callback(std::move(Ctx),
+             make_tagged(std::move(Result), std::move(TaggedFS.Tag)));
+  };
 
-  WorkScheduler.addToFront(std::move(Task), std::move(Ctx),
-                           std::move(Callback));
+  WorkScheduler.schedulePreambleRead(
+      File, BindWithForward(Task, std::move(Ctx), std::move(Contents),
+                            File.str(), std::move(Callback)));
 }
 
 llvm::Expected<Tagged<SignatureHelp>>
 ClangdServer::signatureHelp(const Context &Ctx, PathRef File, Position Pos,
                             llvm::Optional<StringRef> OverridenContents,
                             IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
-  std::string DraftStorage;
-  if (!OverridenContents) {
-    auto FileContents = DraftMgr.getDraft(File);
-    if (!FileContents.Draft)
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  if (UsedFS)
+    *UsedFS = TaggedFS.Value;
+
+  std::string Contents;
+  if (OverridenContents) {
+    Contents = OverridenContents->str();
+  } else {
+    VersionedDraft Latest = DraftMgr.getDraft(File);
+    if (!Latest.Draft)
       return llvm::make_error<llvm::StringError>(
           "signatureHelp is called for non-added document",
           llvm::errc::invalid_argument);
-
-    DraftStorage = std::move(*FileContents.Draft);
-    OverridenContents = DraftStorage;
+    Contents = std::move(*Latest.Draft);
   }
 
-  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  if (UsedFS)
-    *UsedFS = TaggedFS.Value;
-
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  if (!Resources)
+  auto Command = WorkScheduler.getLatestCommand(File);
+  if (!Command)
     return llvm::make_error<llvm::StringError>(
         "signatureHelp is called for non-added document",
         llvm::errc::invalid_argument);
 
-  assert(Resources->getLatestCommand() &&
-         "CppFile is in inconsistent state, missing CompileCommand");
-  tooling::CompileCommand CompileCommand = *Resources->getLatestCommand();
-
-  auto Preamble = Resources->getPossiblyStalePreamble();
-  auto Result = clangd::signatureHelp(
-      Ctx, File, CompileCommand, Preamble ? &Preamble->Preamble : nullptr,
-      *OverridenContents, Pos, TaggedFS.Value, PCHs);
-  return make_tagged(std::move(Result), TaggedFS.Tag);
+  auto Action = [=, &Ctx](llvm::Expected<const PreambleData *> PreambleErr)
+      -> Expected<Tagged<SignatureHelp>> {
+    if (!PreambleErr)
+      return PreambleErr.takeError();
+    auto Preamble = *PreambleErr;
+
+    return make_tagged(
+        clangd::signatureHelp(Ctx, File, *Command,
+                              Preamble ? &Preamble->Preamble : nullptr,
+                              Contents, Pos, TaggedFS.Value, PCHs),
+        TaggedFS.Tag);
+  };
+  return waitForPreambleAction<Expected<Tagged<SignatureHelp>>>(WorkScheduler,
+                                                                File, Action);
 }
 
 llvm::Expected<tooling::Replacements>
@@ -372,49 +477,54 @@
 Expected<std::vector<tooling::Replacement>>
 ClangdServer::rename(const Context &Ctx, PathRef File, Position Pos,
                      llvm::StringRef NewName) {
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  RefactoringResultCollector ResultCollector;
-  Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) {
+  using RetType = Expected<std::vector<tooling::Replacement>>;
+  auto Action = [=](Expected<ParsedAST &> AST) -> RetType {
+    if (!AST)
+      return AST.takeError();
+
+    RefactoringResultCollector ResultCollector;
     const SourceManager &SourceMgr = AST->getASTContext().getSourceManager();
     const FileEntry *FE =
         SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
     if (!FE)
-      return;
+      return llvm::make_error<llvm::StringError>(
+          "rename called for non-added document", llvm::errc::invalid_argument);
     SourceLocation SourceLocationBeg =
         clangd::getBeginningOfIdentifier(*AST, Pos, FE);
     tooling::RefactoringRuleContext Context(
         AST->getASTContext().getSourceManager());
     Context.setASTContext(AST->getASTContext());
     auto Rename = clang::tooling::RenameOccurrences::initiate(
         Context, SourceRange(SourceLocationBeg), NewName.str());
-    if (!Rename) {
-      ResultCollector.Result = Rename.takeError();
-      return;
-    }
+    if (!Rename)
+      return Rename.takeError();
+
     Rename->invoke(ResultCollector, Context);
-  });
-  assert(ResultCollector.Result.hasValue());
-  if (!ResultCollector.Result.getValue())
-    return ResultCollector.Result->takeError();
-
-  std::vector<tooling::Replacement> Replacements;
-  for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
-    tooling::Replacements ChangeReps = Change.getReplacements();
-    for (const auto &Rep : ChangeReps) {
-      // FIXME: Right now we only support renaming the main file, so we drop
-      // replacements not for the main file. In the future, we might consider to
-      // support:
-      //   * rename in any included header
-      //   * rename only in the "main" header
-      //   * provide an error if there are symbols we won't rename (e.g.
-      //     std::vector)
-      //   * rename globally in project
-      //   * rename in open files
-      if (Rep.getFilePath() == File)
-        Replacements.push_back(Rep);
+
+    assert(ResultCollector.Result.hasValue());
+    if (!ResultCollector.Result.getValue())
+      return ResultCollector.Result->takeError();
+
+    std::vector<tooling::Replacement> Replacements;
+    for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
+      tooling::Replacements ChangeReps = Change.getReplacements();
+      for (const auto &Rep : ChangeReps) {
+        // FIXME: Right now we only support renaming the main file, so we
+        // drop replacements not for the main file. In the future, we might
+        // consider to support:
+        //   * rename in any included header
+        //   * rename only in the "main" header
+        //   * provide an error if there are symbols we won't rename (e.g.
+        //     std::vector)
+        //   * rename globally in project
+        //   * rename in open files
+        if (Rep.getFilePath() == File)
+          Replacements.push_back(Rep);
+      }
     }
-  }
-  return Replacements;
+    return Replacements;
+  };
+  return waitForASTAction<RetType>(WorkScheduler, File, std::move(Action));
 }
 
 llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
@@ -425,40 +535,33 @@
 }
 
 std::string ClangdServer::dumpAST(PathRef File) {
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  if (!Resources)
-    return "<non-added file>";
+  auto Action = [](llvm::Expected<ParsedAST &> AST) -> std::string {
+    if (!AST)
+      return "<no-ast>";
+
+    std::string Result;
 
-  std::string Result;
-  Resources->getAST().get()->runUnderLock([&Result](ParsedAST *AST) {
     llvm::raw_string_ostream ResultOS(Result);
-    if (AST) {
-      clangd::dumpAST(*AST, ResultOS);
-    } else {
-      ResultOS << "<no-ast>";
-    }
+    clangd::dumpAST(*AST, ResultOS);
     ResultOS.flush();
-  });
-  return Result;
+
+    return Result;
+  };
+  return waitForASTAction<std::string>(WorkScheduler, File, std::move(Action));
 }
 
 llvm::Expected<Tagged<std::vector<Location>>>
 ClangdServer::findDefinitions(const Context &Ctx, PathRef File, Position Pos) {
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
 
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  if (!Resources)
-    return llvm::make_error<llvm::StringError>(
-        "findDefinitions called on non-added file",
-        llvm::errc::invalid_argument);
-
-  std::vector<Location> Result;
-  Resources->getAST().get()->runUnderLock([Pos, &Result, &Ctx](ParsedAST *AST) {
+  using RetType = llvm::Expected<Tagged<std::vector<Location>>>;
+  auto Action = [=, &Ctx](llvm::Expected<ParsedAST &> AST) -> RetType {
     if (!AST)
-      return;
-    Result = clangd::findDefinitions(Ctx, *AST, Pos);
-  });
-  return make_tagged(std::move(Result), TaggedFS.Tag);
+      return AST.takeError();
+    auto Result = clangd::findDefinitions(Ctx, *AST, Pos);
+    return make_tagged(std::move(Result), TaggedFS.Tag);
+  };
+  return waitForASTAction<RetType>(WorkScheduler, File, Action);
 }
 
 llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
@@ -545,111 +648,64 @@
 
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
 
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  if (!Resources)
-    return llvm::make_error<llvm::StringError>(
-        "findDocumentHighlights called on non-added file",
-        llvm::errc::invalid_argument);
-
-  std::vector<DocumentHighlight> Result;
-  llvm::Optional<llvm::Error> Err;
-  Resources->getAST().get()->runUnderLock([Pos, &Ctx, &Err,
-                                           &Result](ParsedAST *AST) {
-    if (!AST) {
-      Err = llvm::make_error<llvm::StringError>("Invalid AST",
-                                                llvm::errc::invalid_argument);
-      return;
-    }
-    Result = clangd::findDocumentHighlights(Ctx, *AST, Pos);
-  });
-
-  if (Err)
-    return std::move(*Err);
-  return make_tagged(Result, TaggedFS.Tag);
+  using RetType = llvm::Expected<Tagged<std::vector<DocumentHighlight>>>;
+  auto Action = [=, &Ctx](llvm::Expected<ParsedAST &> AST) -> RetType {
+    if (!AST)
+      return AST.takeError();
+    auto Result = clangd::findDocumentHighlights(Ctx, *AST, Pos);
+    return make_tagged(std::move(Result), TaggedFS.Tag);
+  };
+  return waitForASTAction<RetType>(WorkScheduler, File, Action);
 }
 
 std::future<Context> ClangdServer::scheduleReparseAndDiags(
     Context Ctx, PathRef File, VersionedDraft Contents,
-    std::shared_ptr<CppFile> Resources,
     Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS,
     bool UpdateCompileCommand) {
+  DocVersion Version = Contents.Version;
 
-  llvm::Optional<tooling::CompileCommand> FileCmd =
-      Resources->getLatestCommand();
-  tooling::CompileCommand CompileCommand =
-      (UpdateCompileCommand || !FileCmd)
+  auto PrevCommand =
+      !UpdateCompileCommand ? WorkScheduler.getLatestCommand(File) : llvm::None;
+  tooling::CompileCommand Command =
+      UpdateCompileCommand || !PrevCommand
           ? getCompileCommand(CDB, File, ResourceDir)
-          : *FileCmd;
+          : std::move(*PrevCommand);
+
+  using OptDiags = llvm::Optional<std::vector<DiagWithFixIts>>;
+  Path FileStr = File.str();
+  VFSTag Tag = std::move(TaggedFS.Tag);
 
-  ParseInputs Inputs(std::move(CompileCommand), std::move(TaggedFS.Value),
-                     *std::move(Contents.Draft));
-  assert(Contents.Draft && "Draft must have contents");
-  UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(const Context &)>
-      DeferredRebuild = Resources->deferRebuild(std::move(Inputs));
   std::promise<Context> DonePromise;
   std::future<Context> DoneFuture = DonePromise.get_future();
 
-  DocVersion Version = Contents.Version;
-  Path FileStr = File;
-  VFSTag Tag = TaggedFS.Tag;
-  auto ReparseAndPublishDiags =
-      [this, FileStr, Version,
-       Tag](UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(
-                const Context &)>
-                DeferredRebuild,
-            std::promise<Context> DonePromise, Context Ctx) -> void {
+  auto Callback = [this, Version, FileStr,
+                   Tag](std::promise<Context> DonePromise, Context Ctx,
+                        OptDiags Diags) {
     auto Guard = onScopeExit([&]() { DonePromise.set_value(std::move(Ctx)); });
-
-    auto CurrentVersion = DraftMgr.getVersion(FileStr);
-    if (CurrentVersion != Version)
-      return; // This request is outdated
-
-    auto Diags = DeferredRebuild(Ctx);
     if (!Diags)
-      return; // A new reparse was requested before this one completed.
+      return;
 
-    // We need to serialize access to resulting diagnostics to avoid calling
-    // `onDiagnosticsReady` in the wrong order.
+    // We need to serialize access to resulting diagnostics to avoid
+    // calling `onDiagnosticsReady` in the wrong order.
     std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
     DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr];
     // FIXME(ibiryukov): get rid of '<' comparison here. In the current
-    // implementation diagnostics will not be reported after version counters'
-    // overflow. This should not happen in practice, since DocVersion is a
-    // 64-bit unsigned integer.
+    // implementation diagnostics will not be reported after version
+    // counters' overflow. This should not happen in practice, since
+    // DocVersion is a 64-bit unsigned integer.
     if (Version < LastReportedDiagsVersion)
       return;
     LastReportedDiagsVersion = Version;
 
-    DiagConsumer.onDiagnosticsReady(Ctx, FileStr,
-                                    make_tagged(std::move(*Diags), Tag));
+    DiagConsumer.onDiagnosticsReady(
+        Ctx, FileStr, make_tagged(std::move(*Diags), std::move(Tag)));
   };
 
-  WorkScheduler.addToFront(std::move(ReparseAndPublishDiags),
-                           std::move(DeferredRebuild), std::move(DonePromise),
-                           std::move(Ctx));
-  return DoneFuture;
-}
-
-std::future<Context>
-ClangdServer::scheduleCancelRebuild(Context Ctx,
-                                    std::shared_ptr<CppFile> Resources) {
-  std::promise<Context> DonePromise;
-  std::future<Context> DoneFuture = DonePromise.get_future();
-  if (!Resources) {
-    // No need to schedule any cleanup.
-    DonePromise.set_value(std::move(Ctx));
-    return DoneFuture;
-  }
-
-  UniqueFunction<void()> DeferredCancel = Resources->deferCancelRebuild();
-  auto CancelReparses = [Resources](std::promise<Context> DonePromise,
-                                    UniqueFunction<void()> DeferredCancel,
-                                    Context Ctx) {
-    DeferredCancel();
-    DonePromise.set_value(std::move(Ctx));
-  };
-  WorkScheduler.addToFront(std::move(CancelReparses), std::move(DonePromise),
-                           std::move(DeferredCancel), std::move(Ctx));
+  WorkScheduler.scheduleUpdate(
+      std::move(Ctx), File,
+      ParseInputs(Command, std::move(TaggedFS.Value),
+                  std::move(*Contents.Draft)),
+      BindWithForward(Callback, std::move(DonePromise)));
   return DoneFuture;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to