ilya-biryukov updated this revision to Diff 131105.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Renamed SimpleThreadPool to ThreadPool
- Removed PCHs from Scheduler's constructor
- Renamed waitFor(AST|Preamble)Action to blocking(AST|Preamble)Read
- Updates
- Remove getLastCommand from Scheduler's interface, pass Inputs to actions 
instead


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174

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

Index: clangd/ClangdUnitStore.h
===================================================================
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -27,26 +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, 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;
@@ -58,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/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 ThreadPool {
 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();
+  ThreadPool(unsigned AsyncThreadsCount);
+  ~ThreadPool();
 
   /// 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,77 @@
 
 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;
 };
 
+
+struct InputsAndAST {
+  const ParseInputs &Inputs;
+  ParsedAST &AST;
+};
+
+struct InputsAndPreamble {
+  const ParseInputs &Inputs;
+  const PreambleData* Preamble;
+};
+
+/// 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,
+            ASTParsedCallback ASTCallback);
+
+  /// 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.
+  /// FIXME(ibiryukov): the callback passed to this function is not used, we
+  /// should remove it.
+  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<InputsAndAST>)> 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<InputsAndPreamble>)> Action);
+
+private:
+  const ParseInputs& getInputs(PathRef File);
+
+private:
+  llvm::StringMap<ParseInputs> CachedInputs;
+  CppFileCollection Units;
+  ThreadPool 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,12 +386,8 @@
 
   std::future<Context>
   scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents,
-                          std::shared_ptr<CppFile> Resources,
                           Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
 
-  std::future<Context>
-  scheduleCancelRebuild(Context Ctx, std::shared_ptr<CppFile> Resources);
-
   // Stores compile commands produced by GlobalCompilationDatabase.
   class CompileArgsCache {
   public:
@@ -368,21 +420,19 @@
   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;
   // 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 blockingASTRead(Scheduler &S, PathRef File, Func &&F) {
+  std::packaged_task<Ret(llvm::Expected<InputsAndAST>)> 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 blockingPreambleRead(Scheduler &S, PathRef File, Func &&F) {
+  std::packaged_task<Ret(llvm::Expected<InputsAndPreamble>)> 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);
@@ -88,7 +108,7 @@
   return HardwareConcurrency;
 }
 
-ClangdScheduler::ClangdScheduler(unsigned AsyncThreadsCount)
+ThreadPool::ThreadPool(unsigned AsyncThreadsCount)
     : RunSynchronously(AsyncThreadsCount == 0) {
   if (RunSynchronously) {
     // Don't start the worker thread if we're running synchronously
@@ -114,7 +134,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
+          // ThreadPool 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 +147,7 @@
   }
 }
 
-ClangdScheduler::~ClangdScheduler() {
+ThreadPool::~ThreadPool() {
   if (RunSynchronously)
     return; // no worker thread is running in that case
 
@@ -142,6 +162,106 @@
     Worker.join();
 }
 
+Scheduler::Scheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
+                     ASTParsedCallback ASTCallback)
+    : Units(StorePreamblesInMemory, std::make_shared<PCHContainerOperations>(),
+            std::move(ASTCallback)),
+      Executor(AsyncThreadsCount) {}
+
+void Scheduler::scheduleUpdate(
+    Context Ctx, PathRef File, ParseInputs Inputs,
+    UniqueFunction<void(Context Ctx,
+                        llvm::Optional<std::vector<DiagWithFixIts>>)>
+        OnUpdated) {
+  CachedInputs[File] = Inputs;
+
+  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) {
+  CachedInputs.erase(File);
+
+  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<InputsAndAST>)> 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;
+  }
+
+  const ParseInputs &Inputs = getInputs(File);
+  // We currently block the calling thread until AST is available and run the
+  // action on the calling thread to avoid inconsistent states coming from
+  // subsequent updates.
+  // FIXME(ibiryukov): this should be moved to the worker threads.
+  Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) {
+    if (AST)
+      Action(InputsAndAST{Inputs, *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<InputsAndPreamble>)> 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;
+  }
+
+  ParseInputs Inputs = getInputs(File);
+  std::shared_ptr<const PreambleData> Preamble =
+      Resources->getPossiblyStalePreamble();
+  Executor.addToFront(
+      [Resources, Preamble, Inputs](decltype(Action) Action) mutable {
+        if (!Preamble)
+          Preamble = Resources->getPossiblyStalePreamble();
+
+        Action(InputsAndPreamble{Inputs, Preamble.get()});
+      },
+      std::move(Action));
+}
+
+const ParseInputs &Scheduler::getInputs(PathRef File) {
+  auto It = CachedInputs.find(File);
+  assert(It != CachedInputs.end());
+  return It->second;
+}
+
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
                            DiagnosticsConsumer &DiagConsumer,
                            FileSystemProvider &FSProvider,
@@ -153,18 +273,17 @@
                   ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
       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.
+      PCHs(std::make_shared<PCHContainerOperations>()),
+      // 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.
-      Units(FileIdx
-                ? [this](const Context &Ctx, PathRef Path,
-                         ParsedAST *AST) { FileIdx->update(Ctx, Path, AST); }
-                : ASTParsedCallback()),
-      PCHs(std::make_shared<PCHContainerOperations>()),
-      StorePreamblesInMemory(StorePreamblesInMemory),
-      WorkScheduler(AsyncThreadsCount) {
+      WorkScheduler(
+          AsyncThreadsCount, StorePreamblesInMemory,
+          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();
@@ -186,21 +305,30 @@
 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, StorePreamblesInMemory, PCHs);
   return scheduleReparseAndDiags(std::move(Ctx), File,
                                  VersionedDraft{Version, Contents.str()},
-                                 std::move(Resources), std::move(TaggedFS));
+                                 std::move(TaggedFS));
 }
 
 std::future<Context> ClangdServer::removeDocument(Context Ctx, PathRef File) {
   DraftMgr.removeDraft(File);
   CompileArgs.invalidate(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) {
@@ -213,10 +341,8 @@
   CompileArgs.invalidate(File);
 
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  std::shared_ptr<CppFile> Resources =
-      Units.getOrCreateFile(File, StorePreamblesInMemory, PCHs);
-  return scheduleReparseAndDiags(std::move(Ctx), File, FileContents,
-                                 std::move(Resources), std::move(TaggedFS));
+  return scheduleReparseAndDiags(std::move(Ctx), File, std::move(FileContents),
+                                 TaggedFS);
 }
 
 std::future<std::pair<Context, Tagged<CompletionList>>>
@@ -248,99 +374,85 @@
     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;
+  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;
+  }
+
   // Copy PCHs to avoid accessing this->PCHs concurrently
   std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
+  auto Task = [PCHs, Pos, TaggedFS, CodeCompleteOpts](
+                  Context Ctx, std::string Contents, Path File,
+                  CallbackType Callback,
+                  llvm::Expected<InputsAndPreamble> InpPreamble) {
+    assert(InpPreamble &&
+           "error when trying to read preamble for codeComplete");
+    auto Preamble = InpPreamble->Preamble;
+    auto &Command = InpPreamble->Inputs.CompileCommand;
+
+    // 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)));
+  };
 
-  tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File);
-
-  // 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);
-
-        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)
-      return llvm::make_error<llvm::StringError>(
-          "signatureHelp is called for non-added document",
-          llvm::errc::invalid_argument);
-
-    DraftStorage = std::move(*FileContents.Draft);
-    OverridenContents = DraftStorage;
-  }
-
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
   if (UsedFS)
     *UsedFS = TaggedFS.Value;
 
-  std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  if (!Resources)
-    return llvm::make_error<llvm::StringError>(
-        "signatureHelp is called for non-added document",
-        llvm::errc::invalid_argument);
+  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);
+    Contents = std::move(*Latest.Draft);
+  }
 
-  auto Preamble = Resources->getPossiblyStalePreamble();
-  auto Result =
-      clangd::signatureHelp(Ctx, File, CompileArgs.getCompileCommand(File),
-                            Preamble ? &Preamble->Preamble : nullptr,
-                            *OverridenContents, Pos, TaggedFS.Value, PCHs);
-  return make_tagged(std::move(Result), TaggedFS.Tag);
+  auto Action = [=, &Ctx](llvm::Expected<InputsAndPreamble> InpPreamble)
+      -> Expected<Tagged<SignatureHelp>> {
+    if (!InpPreamble)
+      return InpPreamble.takeError();
+    auto Preamble = InpPreamble->Preamble;
+    auto &Command = InpPreamble->Inputs.CompileCommand;
+
+    return make_tagged(
+        clangd::signatureHelp(Ctx, File, Command,
+                              Preamble ? &Preamble->Preamble : nullptr,
+                              Contents, Pos, TaggedFS.Value, PCHs),
+        TaggedFS.Tag);
+  };
+  return blockingPreambleRead<Expected<Tagged<SignatureHelp>>>(WorkScheduler,
+                                                               File, Action);
 }
 
 llvm::Expected<tooling::Replacements>
@@ -372,49 +484,55 @@
 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) {
-    const SourceManager &SourceMgr = AST->getASTContext().getSourceManager();
+  using RetType = Expected<std::vector<tooling::Replacement>>;
+  auto Action = [=](Expected<InputsAndAST> InpAST) -> RetType {
+    if (!InpAST)
+      return InpAST.takeError();
+    auto &AST = InpAST->AST;
+
+    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);
+        clangd::getBeginningOfIdentifier(AST, Pos, FE);
     tooling::RefactoringRuleContext Context(
-        AST->getASTContext().getSourceManager());
-    Context.setASTContext(AST->getASTContext());
+        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 blockingASTRead<RetType>(WorkScheduler, File, std::move(Action));
 }
 
 llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
@@ -425,40 +543,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<InputsAndAST> InpAST) -> std::string {
+    if (!InpAST)
+      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(InpAST->AST, ResultOS);
     ResultOS.flush();
-  });
-  return Result;
+
+    return Result;
+  };
+  return blockingASTRead<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) {
-    if (!AST)
-      return;
-    Result = clangd::findDefinitions(Ctx, *AST, Pos);
-  });
-  return make_tagged(std::move(Result), TaggedFS.Tag);
+  using RetType = llvm::Expected<Tagged<std::vector<Location>>>;
+  auto Action = [=, &Ctx](llvm::Expected<InputsAndAST> InpAST) -> RetType {
+    if (!InpAST)
+      return InpAST.takeError();
+    auto Result = clangd::findDefinitions(Ctx, InpAST->AST, Pos);
+    return make_tagged(std::move(Result), TaggedFS.Tag);
+  };
+  return blockingASTRead<RetType>(WorkScheduler, File, Action);
 }
 
 llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
@@ -545,58 +656,34 @@
 
   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<InputsAndAST> InpAST) -> RetType {
+    if (!InpAST)
+      return InpAST.takeError();
+    auto Result = clangd::findDocumentHighlights(Ctx, InpAST->AST, Pos);
+    return make_tagged(std::move(Result), TaggedFS.Tag);
+  };
+  return blockingASTRead<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) {
-  assert(Contents.Draft && "Draft must have contents");
-  ParseInputs Inputs = {CompileArgs.getCompileCommand(File),
-                        std::move(TaggedFS.Value), *std::move(Contents.Draft)};
+  tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
 
-  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();
+  using OptDiags = llvm::Optional<std::vector<DiagWithFixIts>>;
 
   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 Guard = onScopeExit([&]() { DonePromise.set_value(std::move(Ctx)); });
+  Path FileStr = File.str();
+  VFSTag Tag = std::move(TaggedFS.Tag);
 
-    auto CurrentVersion = DraftMgr.getVersion(FileStr);
-    if (CurrentVersion != Version)
-      return; // This request is outdated
+  std::promise<Context> DonePromise;
+  std::future<Context> DoneFuture = DonePromise.get_future();
 
-    auto Diags = DeferredRebuild(Ctx);
+  auto Callback = [this, Version, FileStr,
+                   Tag](std::promise<Context> DonePromise, Context Ctx,
+                        OptDiags Diags) {
+    auto Guard = onScopeExit([&]() { DonePromise.set_value(std::move(Ctx)); });
     if (!Diags)
       return; // A new reparse was requested before this one completed.
 
@@ -612,36 +699,15 @@
       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{std::move(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