[clangd] Simplify code handling compile commands

[ilya-biryukov]

Summary

CppFile can now change compilation arguments during rebuild. This allows
simplifying code that manages CppFiles.

[sammccall]

This looks better overall to me.

I'm not sure about clangdserver rather than clangdunit computing the commands. You're probably right but I'd like you to explain it to me :-)
Rest is just readability nits.

clangd/ClangdServer.cpp

35

This seems like something of an odd fit: the clangdserver both produces and consumes the compile commands, but ClangdUnit is responsible for storing it?

Are we sure it wouldn't be clearer to keep the "get compile command" action in clangd unit (and propagate the "should rescan" flag), and just encapsulate the resource-dir/fallback logic a bit better?

336

what's inconsistent about this state? (and above)

337

this seems better off inlined?

577

FWIW this might be easier to parse, and avoids dead copies, as

Optional<CompileCommand> ReusedCommand;
if (AllowCachedFlags)
  ReusedCommand = Resources->getLatestCommand();
CompileCommand Cmd = ReusedCommand ? std::move(*ReusedCommand) : getCompileCommand();
ParseInput Inputs(std::move(Cmd), ...)

clangd/ClangdServer.h

335

The name UpdateCompileCommand is confusing in the case that this file hasn't been seen: it's not obvious whether you have to pass true, or whether it doesn't matter.

Consider AllowCachedFlags, and inverting the sense?
At least for me, it's more obvious that this flag is ignored if the cache is empty.
(or AllowCachedCompileCommand, which is a bit long for my taste, or AllowCachedCommand, which is a bit vague)

clangd/ClangdUnit.cpp

479

nit: alias doesn't seem needed

clangd/ClangdUnit.h

62

unwrap?

67

these comments just echo the type/name, remove?

185–186

should ParseInputs be &&? It's big, if callers are copying it something went wrong.

222

nit: consider getLastCommand which removes "latest"'s ambiguity between "previous command used" and "current command, bypassing any caches"

[ilya-biryukov]

Addressing review comments

[ilya-biryukov]

clangd/ClangdServer.cpp

35

I've put the command into ClangdUnit to not change the code consuming compile commands (this is where you currently get compile commands from).

The final goal is to remove compile command from ClangdUnit and store it beside the contents of the file (somewhere inside ClangdServer or its component that manages the resources), ClangdUnit will only manage the built ASTs/Preambles.
This is what ParseInputs is for, it captures all things necessary to build AST/Preamble. When we'll start dropping ASTs for non-accessed files, we could be storing ParseInputs instead to be able to recreate the ASTs when necessary.

336

There's an invariant that if a file is tracked, its compile command in CppFile should be set.
E.g., !!(Untis.getFile(File)) == !!(Untis.getFile(File))->getLatestCompileCommand.

We should probably spell it out explicitly in the assertion message. E.g. "getFile() must only return files with populated commands"
WDYT?

clangd/ClangdServer.h

335

I like CachedFlags, but it also seems a bit vague in the same sense that CachedCommand is vague.
I've opted for AllowCachedCompileFlags, it's long but shouldn't cause any confusion.

clangd/ClangdUnit.h

67

Sorry, I thought I removed them prior to doing the commit.
Thanks for spotting that.

[sammccall]

clangd/ClangdUnit.h

62

This is not done. Is clang-format doing this?

(It looks like exactly 80 cols, but in any case the second comma shouldn't be there :-))

64

remove the constructor and just use brace-init?

222

I think that this might actually be the best place to document the invariant you rely on elsewhere (despite it being a layering violation).

// In practice we always call rebuild() when adding a CppFile to the collection,
// and only `cancelRebuild()` after removing it. This means files in the CppFileCollection
// always have a compile command available.

[ilya-biryukov]

• Unwrap the comment
• Remove ParseInputs constructor
• Document the CppFile invariant

[ilya-biryukov]

clangd/ClangdUnit.h

62

Sorry, my mistake. Fixed now.

clangd/ClangdServer.h

private:
   /// slow. Think of a way to cache this.
   llvm::Expected<tooling::Replacements>
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
   std::future<Context>
   scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents,
                           std::shared_ptr<CppFile> Resources,
-                          Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
+                          Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS,
+                          bool AllowCachedCompileFlags);
 
   std::future<Context>
   scheduleCancelRebuild(Context Ctx, std::shared_ptr<CppFile> Resources);
 
   GlobalCompilationDatabase &CDB;
   DiagnosticsConsumer &DiagConsumer;
   FileSystemProvider &FSProvider;
   DraftStore DraftMgr;

clangd/ClangdServer.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <future>
 
 using namespace clang;
 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);
 }
 
 class RefactoringResultCollector final
     : public tooling::RefactoringResultConsumer {
 public:
if (llvm::sys::fs::is_directory(NewRootPath))
     this->RootPath = NewRootPath;
 }
 
 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, CDB, StorePreamblesInMemory, PCHs);
+  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));
+                                 std::move(Resources), std::move(TaggedFS),
+                                 /*AllowCachedCompileFlags=*/true);
 }
 
 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::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);
-  auto Recreated = Units.recreateFileIfCompileCommandChanged(
-      File, ResourceDir, CDB, StorePreamblesInMemory, PCHs);
-
-  // Note that std::future from this cleanup action is ignored.
-  scheduleCancelRebuild(Ctx.clone(), std::move(Recreated.RemovedFile));
-  // Schedule a reparse.
-  return scheduleReparseAndDiags(std::move(Ctx), File, std::move(FileContents),
-                                 std::move(Recreated.FileInCollection),
-                                 std::move(TaggedFS));
+  std::shared_ptr<CppFile> Resources =
+      Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs);
+  return scheduleReparseAndDiags(std::move(Ctx), File, FileContents,
+                                 std::move(Resources), std::move(TaggedFS),
+                                 /*AllowCachedCompileFlags=*/false);
 }
 
 std::future<std::pair<Context, Tagged<CompletionList>>>
 ClangdServer::codeComplete(Context Ctx, PathRef File, Position Pos,
                            const clangd::CodeCompleteOptions &Opts,
                            llvm::Optional<StringRef> OverridenContents,
                            IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
   using ResultType = std::pair<Context, Tagged<CompletionList>>;
void ClangdServer::codeComplete(
   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;
+
+  assert(Resources->getLastCommand() &&
+         "CppFile is in inconsistent state, missing CompileCommand");
+  tooling::CompileCommand CompileCommand = *Resources->getLastCommand();
+
   // A task that will be run asynchronously.
   auto Task =
       // 'mutable' to reassign Preamble variable.
       [FileStr, Preamble, Resources, Contents, Pos, CodeCompleteOpts, TaggedFS,
-       PCHs](Context Ctx, CallbackType Callback) mutable {
+       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, Resources->getCompileCommand(),
+            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),
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);
 
+  assert(Resources->getLastCommand() &&
+         "CppFile is in inconsistent state, missing CompileCommand");
+
   auto Preamble = Resources->getPossiblyStalePreamble();
   auto Result =
-      clangd::signatureHelp(Ctx, File, Resources->getCompileCommand(),
+      clangd::signatureHelp(Ctx, File, *Resources->getLastCommand(),
                             Preamble ? &Preamble->Preamble : nullptr,
                             *OverridenContents, Pos, TaggedFS.Value, PCHs);
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
 llvm::Expected<tooling::Replacements>
 ClangdServer::formatRange(StringRef Code, PathRef File, Range Rng) {
   size_t Begin = positionToOffset(Code, Rng.start);
ClangdServer::findDocumentHighlights(const Context &Ctx, PathRef File,
   if (Err)
     return std::move(*Err);
   return make_tagged(Result, TaggedFS.Tag);
 }
 
 std::future<Context> ClangdServer::scheduleReparseAndDiags(
     Context Ctx, PathRef File, VersionedDraft Contents,
     std::shared_ptr<CppFile> Resources,
-    Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
+    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)};
   assert(Contents.Draft && "Draft must have contents");
   UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(const Context &)>
-      DeferredRebuild =
-          Resources->deferRebuild(*Contents.Draft, TaggedFS.Value);
+      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,

clangd/ClangdUnit.h

PreambleData(PrecompiledPreamble Preamble,
                std::vector<serialization::DeclID> TopLevelDeclIDs,
                std::vector<DiagWithFixIts> Diags);
 
   PrecompiledPreamble Preamble;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<DiagWithFixIts> Diags;
 };
 
+/// Information required to run clang, e.g. to parse AST or do code completion.
+struct ParseInputs {
+  tooling::CompileCommand CompileCommand;
+  IntrusiveRefCntPtr<vfs::FileSystem> FS;
+  std::string Contents;
+};
+
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
   /// it is reused during parsing.
   static llvm::Optional<ParsedAST>
   Build(const Context &Ctx, std::unique_ptr<clang::CompilerInvocation> CI,
         std::shared_ptr<const PreambleData> Preamble,

 
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
 class CppFile : public std::enable_shared_from_this<CppFile> {
 public:
   // We only allow to create CppFile as shared_ptr, because a future returned by
   // deferRebuild will hold references to it.
   static std::shared_ptr<CppFile>
-  Create(PathRef FileName, tooling::CompileCommand Command,
-         bool StorePreamblesInMemory,
+  Create(PathRef FileName, bool StorePreamblesInMemory,
          std::shared_ptr<PCHContainerOperations> PCHs,
          ASTParsedCallback ASTCallback);
 
 private:
-  CppFile(PathRef FileName, tooling::CompileCommand Command,
-          bool StorePreamblesInMemory,
+  CppFile(PathRef FileName, bool StorePreamblesInMemory,
           std::shared_ptr<PCHContainerOperations> PCHs,
           ASTParsedCallback ASTCallback);
 
 public:
   CppFile(CppFile const &) = delete;
   CppFile(CppFile &&) = delete;
 
   /// Cancels a scheduled rebuild, if any, and sets AST and Preamble to nulls.
   /// If a rebuild is in progress, will wait for it to finish.
   void cancelRebuild();
 
   /// Similar to deferRebuild, but sets both Preamble and AST to nulls instead
   /// of doing an actual parsing. Returned function is a deferred computation
   /// that will wait for any ongoing rebuilds to finish and actually set the AST
   /// and Preamble to nulls. It can be run on a different thread. This function
   /// is useful to cancel ongoing rebuilds, if any, before removing CppFile.
   UniqueFunction<void()> deferCancelRebuild();
 
   /// Rebuild AST and Preamble synchronously on the calling thread.
   /// Returns a list of diagnostics or a llvm::None, if another rebuild was
   /// requested in parallel (effectively cancelling this rebuild) before
   /// diagnostics were produced.
-  llvm::Optional<std::vector<DiagWithFixIts>>
-  rebuild(const Context &Ctx, StringRef NewContents,
-          IntrusiveRefCntPtr<vfs::FileSystem> VFS);
+  llvm::Optional<std::vector<DiagWithFixIts>> rebuild(const Context &Ctx,
+                                                      ParseInputs &&Inputs);
 
   /// Schedule a rebuild and return a deferred computation that will finish the
   /// rebuild, that can be called on a different thread.
   /// After calling this method, resources, available via futures returned by
   /// getPreamble() and getAST(), will be waiting for rebuild to finish. A
   /// continuation fininshing rebuild, returned by this function, must be
   /// computed(i.e., operator() must be called on it) in order to make those
   /// resources ready. If deferRebuild is called again before the rebuild is
   /// finished (either because returned future had not been called or because it
   /// had not returned yet), the previous rebuild request is cancelled and the
   /// resource futures (returned by getPreamble() or getAST()) that were not
   /// ready will be waiting for the last rebuild to finish instead.
   /// The future to finish rebuild returns a list of diagnostics built during
   /// reparse, or None, if another deferRebuild was called before this
   /// rebuild was finished.
   UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(const Context &)>
-  deferRebuild(StringRef NewContents, IntrusiveRefCntPtr<vfs::FileSystem> VFS);
+  deferRebuild(ParseInputs &&Inputs);
 
   /// Returns a future to get the most fresh PreambleData for a file. The
   /// future will wait until the Preamble is rebuilt.
   std::shared_future<std::shared_ptr<const PreambleData>> getPreamble() const;
   /// Return some preamble for a file. It might be stale, but won't wait for
   /// rebuild to finish.
   std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
 
   /// Returns a future to get the most fresh AST for a file. Returned AST is
   /// wrapped to prevent concurrent accesses.
   /// We use std::shared_ptr here because MVSC fails to compile non-copyable
   /// classes as template arguments of promise/future. It is guaranteed to
   /// always be non-null.
   std::shared_future<std::shared_ptr<ParsedASTWrapper>> getAST() const;
 
-  /// Get CompileCommand used to build this CppFile.
-  tooling::CompileCommand const &getCompileCommand() 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 {
   public:
     RebuildGuard(CppFile &File, unsigned RequestRebuildCounter);
     ~RebuildGuard();
 
     bool wasCancelledBeforeConstruction() const;
 
   private:
     CppFile &File;
     unsigned RequestRebuildCounter;
     bool WasCancelledBeforeConstruction;
   };
 
   Path FileName;
-  tooling::CompileCommand Command;
   bool StorePreamblesInMemory;
 
   /// Mutex protects all fields, declared below it, FileName and Command are not
   /// mutated.
   mutable std::mutex Mutex;
   /// A counter to cancel old rebuilds.
   unsigned RebuildCounter;
   /// Used to wait when rebuild is finished before starting another one.
   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
   /// classes as template arguments of promise/future.
   std::promise<std::shared_ptr<ParsedASTWrapper>> ASTPromise;
   std::shared_future<std::shared_ptr<ParsedASTWrapper>> ASTFuture;
 
   /// Promise and future for the latests Preamble. Fulfilled during rebuild.

clangd/ClangdUnit.cpp

 
 SourceLocation getMacroArgExpandedLocation(const SourceManager &Mgr,
                                            const FileEntry *FE, Position Pos) {
   SourceLocation InputLoc =
       Mgr.translateFileLineCol(FE, Pos.line + 1, Pos.character + 1);
   return Mgr.getMacroArgExpandedLocation(InputLoc);
 }
 
-
 } // namespace
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
   if (PreambleDeclsDeserialized || !Preamble)
     return;
 
   std::vector<const Decl *> Resolved;
   Resolved.reserve(Preamble->TopLevelDeclIDs.size());

 
 ParsedASTWrapper::ParsedASTWrapper(ParsedASTWrapper &&Wrapper)
     : AST(std::move(Wrapper.AST)) {}
 
 ParsedASTWrapper::ParsedASTWrapper(llvm::Optional<ParsedAST> AST)
     : AST(std::move(AST)) {}
 
 std::shared_ptr<CppFile>
-CppFile::Create(PathRef FileName, tooling::CompileCommand Command,
-                bool StorePreamblesInMemory,
+CppFile::Create(PathRef FileName, bool StorePreamblesInMemory,
                 std::shared_ptr<PCHContainerOperations> PCHs,
                 ASTParsedCallback ASTCallback) {
-  return std::shared_ptr<CppFile>(
-      new CppFile(FileName, std::move(Command), StorePreamblesInMemory,
-                  std::move(PCHs), std::move(ASTCallback)));
+  return std::shared_ptr<CppFile>(new CppFile(FileName, StorePreamblesInMemory,
+                                              std::move(PCHs),
+                                              std::move(ASTCallback)));
 }
 
-CppFile::CppFile(PathRef FileName, tooling::CompileCommand Command,
-                 bool StorePreamblesInMemory,
+CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  ASTParsedCallback ASTCallback)
-    : FileName(FileName), Command(std::move(Command)),
-      StorePreamblesInMemory(StorePreamblesInMemory), RebuildCounter(0),
-      RebuildInProgress(false), PCHs(std::move(PCHs)),
+    : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
+      RebuildCounter(0), RebuildInProgress(false), PCHs(std::move(PCHs)),
       ASTCallback(std::move(ASTCallback)) {
   // FIXME(ibiryukov): we should pass a proper Context here.
-  log(Context::empty(), "Opened file " + FileName + " with command [" +
-                            this->Command.Directory + "] " +
-                            llvm::join(this->Command.CommandLine, " "));
+  log(Context::empty(), "Created CppFile for " + FileName);
 
   std::lock_guard<std::mutex> Lock(Mutex);
   LatestAvailablePreamble = nullptr;
   PreamblePromise.set_value(nullptr);
   PreambleFuture = PreamblePromise.get_future();
 
   ASTPromise.set_value(std::make_shared<ParsedASTWrapper>(llvm::None));
   ASTFuture = ASTPromise.get_future();
return [That, RequestRebuildCounter]() {
 
     // Set empty results for Promises.
     That->PreamblePromise.set_value(nullptr);
     That->ASTPromise.set_value(std::make_shared<ParsedASTWrapper>(llvm::None));
   };
 }
 
 llvm::Optional<std::vector<DiagWithFixIts>>
-CppFile::rebuild(const Context &Ctx, StringRef NewContents,
-                 IntrusiveRefCntPtr<vfs::FileSystem> VFS) {
-  return deferRebuild(NewContents, std::move(VFS))(Ctx);
+CppFile::rebuild(const Context &Ctx, ParseInputs &&Inputs) {
+  return deferRebuild(std::move(Inputs))(Ctx);
 }
 
 UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>(const Context &)>
-CppFile::deferRebuild(StringRef NewContents,
-                      IntrusiveRefCntPtr<vfs::FileSystem> VFS) {
+CppFile::deferRebuild(ParseInputs &&Inputs) {
   std::shared_ptr<const PreambleData> OldPreamble;
   std::shared_ptr<PCHContainerOperations> PCHs;
   unsigned RequestRebuildCounter;
   {
     std::unique_lock<std::mutex> Lock(Mutex);
     // Increase RebuildCounter to cancel all ongoing FinishRebuild operations.
     // They will try to exit as early as possible and won't call set_value on
     // our promises.
     RequestRebuildCounter = ++this->RebuildCounter;
     PCHs = this->PCHs;
 
     // Remember the preamble to be used during rebuild.
     OldPreamble = this->LatestAvailablePreamble;
     // Setup std::promises and std::futures for Preamble and AST. Corresponding
     // futures will wait until the rebuild process is finished.
     if (futureIsReady(this->PreambleFuture)) {
       this->PreamblePromise =
           std::promise<std::shared_ptr<const PreambleData>>();
       this->PreambleFuture = this->PreamblePromise.get_future();
     }
     if (futureIsReady(this->ASTFuture)) {
       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();
 
   // A helper to function to finish the rebuild. May be run on a different
   // thread.
 
   // Don't let this CppFile die before rebuild is finished.
   std::shared_ptr<CppFile> That = shared_from_this();
   auto FinishRebuild =
-      [OldPreamble, VFS, RequestRebuildCounter, PCHs,
-       That](std::string NewContents,
+      [OldPreamble, RequestRebuildCounter, PCHs,
+       That](ParseInputs Inputs,
              const Context &Ctx) mutable /* to allow changing OldPreamble. */
       -> llvm::Optional<std::vector<DiagWithFixIts>> {
+    log(Context::empty(),
+        "Rebuilding file " + That->FileName + " with command [" +
+            Inputs.CompileCommand.Directory + "] " +
+            llvm::join(Inputs.CompileCommand.CommandLine, " "));
+
     // Only one execution of this method is possible at a time.
     // RebuildGuard will wait for any ongoing rebuilds to finish and will put us
     // into a state for doing a rebuild.
     RebuildGuard Rebuild(*That, RequestRebuildCounter);
     if (Rebuild.wasCancelledBeforeConstruction())
       return llvm::None;
 
     std::vector<const char *> ArgStrs;
-    for (const auto &S : That->Command.CommandLine)
+    for (const auto &S : Inputs.CompileCommand.CommandLine)
       ArgStrs.push_back(S.c_str());
 
-    VFS->setCurrentWorkingDirectory(That->Command.Directory);
+    Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory);
 
     std::unique_ptr<CompilerInvocation> CI;
     {
       // FIXME(ibiryukov): store diagnostics from CommandLine when we start
       // reporting them.
       IgnoreDiagnostics IgnoreDiagnostics;
       IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
           CompilerInstance::createDiagnostics(new DiagnosticOptions,
                                               &IgnoreDiagnostics, false);
-      CI =
-          createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, VFS);
+      CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
+                                           Inputs.FS);
       // createInvocationFromCommandLine sets DisableFree.
       CI->getFrontendOpts().DisableFree = false;
     }
     assert(CI && "Couldn't create CompilerInvocation");
 
     std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
-        llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName);
+        llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, That->FileName);
 
     // A helper function to rebuild the preamble or reuse the existing one. Does
     // not mutate any fields of CppFile, only does the actual computation.
     // Lamdba is marked mutable to call reset() on OldPreamble.
     auto DoRebuildPreamble =
         [&]() mutable -> std::shared_ptr<const PreambleData> {
       auto Bounds =
           ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
-      if (OldPreamble && OldPreamble->Preamble.CanReuse(
-                             *CI, ContentsBuffer.get(), Bounds, VFS.get())) {
+      if (OldPreamble &&
+          OldPreamble->Preamble.CanReuse(*CI, ContentsBuffer.get(), Bounds,
+                                         Inputs.FS.get())) {
         log(Ctx, "Reusing preamble for file " + Twine(That->FileName));
         return OldPreamble;
       }
       log(Ctx, "Premble for file " + Twine(That->FileName) +
                    " cannot be reused. Attempting to rebuild it.");
       // We won't need the OldPreamble anymore, release it so it can be
       // deleted (if there are no other references to it).
       OldPreamble.reset();
 
       trace::Span Tracer(Ctx, "Preamble");
       SPAN_ATTACH(Tracer, "File", That->FileName);
       std::vector<DiagWithFixIts> PreambleDiags;
       StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
       IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
           CompilerInstance::createDiagnostics(
               &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
 
       // Skip function bodies when building the preamble to speed up building
       // the preamble and make it smaller.
       assert(!CI->getFrontendOpts().SkipFunctionBodies);
       CI->getFrontendOpts().SkipFunctionBodies = true;
 
       CppFilePreambleCallbacks SerializedDeclsCollector;
       auto BuiltPreamble = PrecompiledPreamble::Build(
-          *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
+          *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS,
+          PCHs,
           /*StoreInMemory=*/That->StorePreamblesInMemory,
           SerializedDeclsCollector);
 
       // When building the AST for the main file, we do want the function
       // bodies.
       CI->getFrontendOpts().SkipFunctionBodies = false;
 
       if (BuiltPreamble) {
auto FinishRebuild =
     }
 
     // Compute updated AST.
     llvm::Optional<ParsedAST> NewAST;
     {
       trace::Span Tracer(Ctx, "Build");
       SPAN_ATTACH(Tracer, "File", That->FileName);
       NewAST = ParsedAST::Build(Ctx, std::move(CI), std::move(NewPreamble),
-                                std::move(ContentsBuffer), PCHs, VFS);
+                                std::move(ContentsBuffer), PCHs, Inputs.FS);
     }
 
     if (NewAST) {
       Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(),
                          NewAST->getDiagnostics().end());
       if (That->ASTCallback)
         That->ASTCallback(Ctx, That->FileName, NewAST.getPointer());
     } else {
// Publish the new AST.
 
       That->ASTPromise.set_value(
           std::make_shared<ParsedASTWrapper>(std::move(NewAST)));
     } // unlock Mutex
 
     return Diagnostics;
   };
 
-  return BindWithForward(FinishRebuild, NewContents.str());
+  return BindWithForward(FinishRebuild, std::move(Inputs));
 }
 
 std::shared_future<std::shared_ptr<const PreambleData>>
 CppFile::getPreamble() const {
   std::lock_guard<std::mutex> Lock(Mutex);
   return PreambleFuture;
 }
 
 std::shared_ptr<const PreambleData> CppFile::getPossiblyStalePreamble() const {
   std::lock_guard<std::mutex> Lock(Mutex);
   return LatestAvailablePreamble;
 }
 
 std::shared_future<std::shared_ptr<ParsedASTWrapper>> CppFile::getAST() const {
   std::lock_guard<std::mutex> Lock(Mutex);
   return ASTFuture;
 }
 
-tooling::CompileCommand const &CppFile::getCompileCommand() const {
-  return Command;
+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) {
   std::unique_lock<std::mutex> Lock(File.Mutex);
   WasCancelledBeforeConstruction = File.RebuildCounter != RequestRebuildCounter;
   if (WasCancelledBeforeConstruction)

clangd/ClangdUnitStore.h

 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)) {}
 
   std::shared_ptr<CppFile>
   getOrCreateFile(PathRef File, PathRef ResourceDir,
-                  GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory,
+                  bool StorePreamblesInMemory,
                   std::shared_ptr<PCHContainerOperations> PCHs) {
     std::lock_guard<std::mutex> Lock(Mutex);
-
     auto It = OpenedFiles.find(File);
     if (It == OpenedFiles.end()) {
-      auto Command = getCompileCommand(CDB, File, ResourceDir);
-
       It = OpenedFiles
-               .try_emplace(File, CppFile::Create(File, std::move(Command),
-                                                  StorePreamblesInMemory,
+               .try_emplace(File, CppFile::Create(File, StorePreamblesInMemory,
                                                   std::move(PCHs), ASTCallback))
                .first;
     }
     return It->second;
   }
 
-  struct RecreateResult {
-    /// A CppFile, stored in this CppFileCollection for the corresponding
-    /// filepath after calling recreateFileIfCompileCommandChanged.
-    std::shared_ptr<CppFile> FileInCollection;
-    /// If a new CppFile had to be created to account for changed
-    /// CompileCommand, a previous CppFile instance will be returned in this
-    /// field.
-    std::shared_ptr<CppFile> RemovedFile;
-  };
-
-  /// Similar to getOrCreateFile, but will replace a current CppFile for \p File
-  /// with a new one if CompileCommand, provided by \p CDB has changed.
-  /// If a currently stored CppFile had to be replaced, the previous instance
-  /// will be returned in RecreateResult.RemovedFile.
-  RecreateResult recreateFileIfCompileCommandChanged(
-      PathRef File, PathRef ResourceDir, GlobalCompilationDatabase &CDB,
-      bool StorePreamblesInMemory,
-      std::shared_ptr<PCHContainerOperations> PCHs);
-
   std::shared_ptr<CppFile> getFile(PathRef File) {
     std::lock_guard<std::mutex> Lock(Mutex);
 
     auto It = OpenedFiles.find(File);
     if (It == OpenedFiles.end())
       return nullptr;
     return It->second;
   }
 
   /// Removes a CppFile, stored for \p File, if it's inside collection and
   /// returns it.
   std::shared_ptr<CppFile> removeIfPresent(PathRef File);
 
 private:
-  tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB,
-                                            PathRef File, PathRef ResourceDir);
-
-  bool compileCommandsAreEqual(tooling::CompileCommand const &LHS,
-                               tooling::CompileCommand const &RHS);
-
   std::mutex Mutex;
   llvm::StringMap<std::shared_ptr<CppFile>> OpenedFiles;
   ASTParsedCallback ASTCallback;
 };
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/ClangdUnitStore.cpp

std::shared_ptr<CppFile> CppFileCollection::removeIfPresent(PathRef File) {
   auto It = OpenedFiles.find(File);
   if (It == OpenedFiles.end())
     return nullptr;
 
   std::shared_ptr<CppFile> Result = It->second;
   OpenedFiles.erase(It);
   return Result;
 }
-
-CppFileCollection::RecreateResult
-CppFileCollection::recreateFileIfCompileCommandChanged(
-    PathRef File, PathRef ResourceDir, GlobalCompilationDatabase &CDB,
-    bool StorePreamblesInMemory, std::shared_ptr<PCHContainerOperations> PCHs) {
-  auto NewCommand = getCompileCommand(CDB, File, ResourceDir);
-
-  std::lock_guard<std::mutex> Lock(Mutex);
-
-  RecreateResult Result;
-
-  auto It = OpenedFiles.find(File);
-  if (It == OpenedFiles.end()) {
-    It = OpenedFiles
-             .try_emplace(File, CppFile::Create(File, std::move(NewCommand),
-                                                StorePreamblesInMemory,
-                                                std::move(PCHs), ASTCallback))
-             .first;
-  } else if (!compileCommandsAreEqual(It->second->getCompileCommand(),
-                                      NewCommand)) {
-    Result.RemovedFile = std::move(It->second);
-    It->second =
-        CppFile::Create(File, std::move(NewCommand), StorePreamblesInMemory,
-                        std::move(PCHs), ASTCallback);
-  }
-  Result.FileInCollection = It->second;
-  return Result;
-}
-
-tooling::CompileCommand
-CppFileCollection::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);
-}
-
-bool CppFileCollection::compileCommandsAreEqual(
-    tooling::CompileCommand const &LHS, tooling::CompileCommand const &RHS) {
-  // tooling::CompileCommand.Output is ignored, it's not relevant for clangd.
-  return LHS.Directory == RHS.Directory &&
-         LHS.CommandLine.size() == RHS.CommandLine.size() &&
-         std::equal(LHS.CommandLine.begin(), LHS.CommandLine.end(),
-                    RHS.CommandLine.begin());
-}