[clang-tools-extra] r323420 - [clangd] Moved caching of compile commands to ClangdServer

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 25 06:19:21 PST 2018


Author: ibiryukov
Date: Thu Jan 25 06:19:21 2018
New Revision: 323420

URL: http://llvm.org/viewvc/llvm-project?rev=323420&view=rev
Log:
[clangd] Moved caching of compile commands to ClangdServer

Summary:
It allows to get rid of CppFile::getLastCommand and simplify the
code in the upcoming threading patch.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, ioeric, cfe-commits

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

Modified:
    clang-tools-extra/trunk/clangd/CMakeLists.txt
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/ClangdUnit.cpp
    clang-tools-extra/trunk/clangd/ClangdUnit.h
    clang-tools-extra/trunk/clangd/ClangdUnitStore.h

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Jan 25 06:19:21 2018
@@ -9,8 +9,9 @@ add_clang_library(clangDaemon
   ClangdUnitStore.cpp
   CodeComplete.cpp
   CodeCompletionStrings.cpp
-  Context.cpp
+  CompileArgsCache.cpp
   Compiler.cpp
+  Context.cpp
   DraftStore.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Jan 25 06:19:21 2018
@@ -32,18 +32,6 @@ using namespace clang::clangd;
 
 namespace {
 
-tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB,
-                                          PathRef File, PathRef ResourceDir) {
-  llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
-  if (!C) // FIXME: Suppress diagnostics? Let the user know?
-    C = CDB.getFallbackCommand(File);
-
-  // Inject the resource dir.
-  // FIXME: Don't overwrite it if it's already there.
-  C->CommandLine.push_back("-resource-dir=" + ResourceDir.str());
-  return std::move(*C);
-}
-
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
@@ -149,7 +137,9 @@ ClangdServer::ClangdServer(GlobalCompila
                            bool StorePreamblesInMemory,
                            bool BuildDynamicSymbolIndex, SymbolIndex *StaticIdx,
                            llvm::Optional<StringRef> ResourceDir)
-    : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
+    : CompileArgs(CDB,
+                  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
@@ -160,7 +150,6 @@ ClangdServer::ClangdServer(GlobalCompila
                 ? [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) {
@@ -188,15 +177,16 @@ std::future<Context> ClangdServer::addDo
 
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
   std::shared_ptr<CppFile> Resources =
-      Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs);
+      Units.getOrCreateFile(File, StorePreamblesInMemory, PCHs);
   return scheduleReparseAndDiags(std::move(Ctx), File,
                                  VersionedDraft{Version, Contents.str()},
-                                 std::move(Resources), std::move(TaggedFS),
-                                 /*AllowCachedCompileFlags=*/true);
+                                 std::move(Resources), 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));
 }
@@ -206,12 +196,15 @@ std::future<Context> ClangdServer::force
   assert(FileContents.Draft &&
          "forceReparse() was called for non-added document");
 
+  // forceReparse promises to request new compilation flags from CDB, so we
+  // remove any cahced flags.
+  CompileArgs.invalidate(File);
+
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
   std::shared_ptr<CppFile> Resources =
-      Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs);
+      Units.getOrCreateFile(File, StorePreamblesInMemory, PCHs);
   return scheduleReparseAndDiags(std::move(Ctx), File, FileContents,
-                                 std::move(Resources), std::move(TaggedFS),
-                                 /*AllowCachedCompileFlags=*/false);
+                                 std::move(Resources), std::move(TaggedFS));
 }
 
 std::future<std::pair<Context, Tagged<CompletionList>>>
@@ -277,11 +270,7 @@ void ClangdServer::codeComplete(
   Path FileStr = File;
   // Copy PCHs to avoid accessing this->PCHs concurrently
   std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
-
-  assert(Resources->getLastCommand() &&
-         "CppFile is in inconsistent state, missing CompileCommand");
-  tooling::CompileCommand CompileCommand = *Resources->getLastCommand();
-
+  tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File);
   // A task that will be run asynchronously.
   auto Task =
       // 'mutable' to reassign Preamble variable.
@@ -332,12 +321,9 @@ ClangdServer::signatureHelp(const Contex
         "signatureHelp is called for non-added document",
         llvm::errc::invalid_argument);
 
-  assert(Resources->getLastCommand() &&
-         "CppFile is in inconsistent state, missing CompileCommand");
-
   auto Preamble = Resources->getPossiblyStalePreamble();
   auto Result =
-      clangd::signatureHelp(Ctx, File, *Resources->getLastCommand(),
+      clangd::signatureHelp(Ctx, File, CompileArgs.getCompileCommand(File),
                             Preamble ? &Preamble->Preamble : nullptr,
                             *OverridenContents, Pos, TaggedFS.Value, PCHs);
   return make_tagged(std::move(Result), TaggedFS.Tag);
@@ -571,18 +557,11 @@ ClangdServer::findDocumentHighlights(con
 std::future<Context> ClangdServer::scheduleReparseAndDiags(
     Context Ctx, PathRef File, VersionedDraft Contents,
     std::shared_ptr<CppFile> Resources,
-    Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS,
-    bool AllowCachedCompileFlags) {
-  llvm::Optional<tooling::CompileCommand> ReusedCommand;
-  if (AllowCachedCompileFlags)
-    ReusedCommand = Resources->getLastCommand();
-  tooling::CompileCommand Command =
-      ReusedCommand ? std::move(*ReusedCommand)
-                    : getCompileCommand(CDB, File, ResourceDir);
-
-  ParseInputs Inputs = {std::move(Command), std::move(TaggedFS.Value),
-                        *std::move(Contents.Draft)};
+    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)};
+
   UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(const Context &)>
       DeferredRebuild = Resources->deferRebuild(std::move(Inputs));
   std::promise<Context> DonePromise;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Jan 25 06:19:21 2018
@@ -13,6 +13,7 @@
 #include "ClangdUnit.h"
 #include "ClangdUnitStore.h"
 #include "CodeComplete.h"
+#include "CompileArgsCache.h"
 #include "DraftStore.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
@@ -331,13 +332,12 @@ private:
   std::future<Context>
   scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents,
                           std::shared_ptr<CppFile> Resources,
-                          Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS,
-                          bool AllowCachedCompileFlags);
+                          Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
 
   std::future<Context>
   scheduleCancelRebuild(Context Ctx, std::shared_ptr<CppFile> Resources);
 
-  GlobalCompilationDatabase &CDB;
+  CompileArgsCache CompileArgs;
   DiagnosticsConsumer &DiagConsumer;
   FileSystemProvider &FSProvider;
   DraftStore DraftMgr;
@@ -352,7 +352,6 @@ private:
   // 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;

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu Jan 25 06:19:21 2018
@@ -455,7 +455,6 @@ CppFile::deferRebuild(ParseInputs &&Inpu
       this->ASTPromise = std::promise<std::shared_ptr<ParsedASTWrapper>>();
       this->ASTFuture = this->ASTPromise.get_future();
     }
-    this->LastCommand = Inputs.CompileCommand;
   } // unlock Mutex.
   // Notify about changes to RebuildCounter.
   RebuildCond.notify_all();
@@ -636,11 +635,6 @@ std::shared_future<std::shared_ptr<Parse
   return ASTFuture;
 }
 
-llvm::Optional<tooling::CompileCommand> CppFile::getLastCommand() const {
-  std::lock_guard<std::mutex> Lock(Mutex);
-  return LastCommand;
-}
-
 CppFile::RebuildGuard::RebuildGuard(CppFile &File,
                                     unsigned RequestRebuildCounter)
     : File(File), RequestRebuildCounter(RequestRebuildCounter) {

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Thu Jan 25 06:19:21 2018
@@ -216,14 +216,6 @@ public:
   /// always be non-null.
   std::shared_future<std::shared_ptr<ParsedASTWrapper>> getAST() const;
 
-  /// Get the latest CompileCommand used to build this CppFile. Returns
-  /// llvm::None before first call to rebuild() or after calls to
-  /// cancelRebuild().
-  // In practice we always call rebuild() when adding a CppFile to the
-  // CppFileCollection, and only `cancelRebuild()` after removing it. This means
-  // files in the CppFileCollection always have a compile command available.
-  llvm::Optional<tooling::CompileCommand> getLastCommand() const;
-
 private:
   /// A helper guard that manages the state of CppFile during rebuild.
   class RebuildGuard {
@@ -251,7 +243,6 @@ private:
   bool RebuildInProgress;
   /// Condition variable to indicate changes to RebuildInProgress.
   std::condition_variable RebuildCond;
-  llvm::Optional<tooling::CompileCommand> LastCommand;
 
   /// Promise and future for the latests AST. Fulfilled during rebuild.
   /// We use std::shared_ptr here because MVSC fails to compile non-copyable

Modified: clang-tools-extra/trunk/clangd/ClangdUnitStore.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnitStore.h?rev=323420&r1=323419&r2=323420&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnitStore.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnitStore.h Thu Jan 25 06:19:21 2018
@@ -31,8 +31,7 @@ public:
       : ASTCallback(std::move(ASTCallback)) {}
 
   std::shared_ptr<CppFile>
-  getOrCreateFile(PathRef File, PathRef ResourceDir,
-                  bool StorePreamblesInMemory,
+  getOrCreateFile(PathRef File, bool StorePreamblesInMemory,
                   std::shared_ptr<PCHContainerOperations> PCHs) {
     std::lock_guard<std::mutex> Lock(Mutex);
     auto It = OpenedFiles.find(File);




More information about the cfe-commits mailing list