Move DraftMgr from ClangdServer to ClangdLSPServer

[simark]

Summary

This patch moves the draft manager closer to the edge of Clangd, from
ClangdServer to ClangdLSPServer. This will make it easier to implement
incremental document sync, by making ClangdServer only deal with
complete documents.

As a result, DraftStore doesn't have to deal with versioning, and thus
its API can be simplified. It is replaced by a StringMap in
ClangdServer holding a current version number for each file.

The current implementation of reparseOpenedFiles is racy. It calls
getActiveDrafts and calls forceReparse for each of them. forceReparse
gets the draft fromt he DraftStore. In theory, it's possible for a file
to have been closed in the mean time. I replaced getActiveDrafts by
forEachActiveDraft. It has the advantage that the DraftStore lock is
held while we iterate on the drafts.

Signed-off-by: Simon Marchi <[e-mail]>

[simark]

Rebase

Non-trivial rebase on today's master.

[ilya-biryukov]

We shouldn't add Contents parameter to every method, it would defeat the purpose of caching ASTs and won't allow to properly manage lifetimes of the indexes.

The most tricky part is getting rid of DraftMgr in forceReparse. Here's a change that removes usages of it there: D44462`, it should be much easier to make it work when it lands.
Any other reason why we need to add them?

clangd/ClangdLSPServer.h

87 ↗

(On Diff #138119)

We can remove this function now, it is equivalent to a one-liner DraftMgr.getDraft(File).Draft

[simark]

In D44408#1036846, @ilya-biryukov wrote:

We shouldn't add Contents parameter to every method, it would defeat the purpose of caching ASTs and won't allow to properly manage lifetimes of the indexes.

Makes sense.

The most tricky part is getting rid of DraftMgr in forceReparse. Here's a change that removes usages of it there: D44462`, it should be much easier to make it work when it lands.

I'll take a look, thanks!

Any other reason why we need to add them?

No, just ignorance on my part.

[simark]

Rebase on D44462

[simark]

I don't see how to avoid adding the Contents parameter to the codeComplete and signatureHelp methods, since they needed to fetch the document from the draft manager and pass it to the backend.

[ilya-biryukov]

In D44408#1036941, @simark wrote:

I don't see how to avoid adding the Contents parameter to the codeComplete and signatureHelp methods, since they needed to fetch the document from the draft manager and pass it to the backend.

You can get the contents inside the runWithPreamble callback:

llvm::Expected<InputsAndPreamble> IP;
auto &Contents = IP->Inputs.Contents;

[simark]

In D44408#1037327, @ilya-biryukov wrote:

In D44408#1036941, @simark wrote:

I don't see how to avoid adding the Contents parameter to the codeComplete and signatureHelp methods, since they needed to fetch the document from the draft manager and pass it to the backend.

You can get the contents inside the runWithPreamble callback:

llvm::Expected<InputsAndPreamble> IP;
auto &Contents = IP->Inputs.Contents;

I see, I made this as a separate patch:

https://reviews.llvm.org/D44484

[ilya-biryukov]

In D44408#1037520, @simark wrote:

I see, I made this as a separate patch:

https://reviews.llvm.org/D44484

I LGTMed it, so feel free to submit it.
However, if you do it in this patch it's also fine.

[simark]

Rebase on current master, addressing previous comments

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

495 ↗

(On Diff #138490)

This function is equivalent to return DraftMgr.getDraft(). Remove and replace all usages with DraftMgr.getDraft()?

clangd/ClangdServer.cpp

154 ↗

(On Diff #138490)

Please replace this assert with:

if (!IP)
  return CB(IP.takeError());

(Otherwise the assert will fire when completion is called for non-tracked files, we really want this to be an error instead)

clangd/ClangdServer.h

121 ↗

(On Diff #138490)

Let's keep it a StringRef. It means an extra copy for now, DraftMgr return StringRefs instead if we remove mutexes from its interface.

225 ↗

(On Diff #138490)

We don't use VersionedContents anywhere. Maybe remove this struct?

236 ↗

(On Diff #138490)

This is probably a good place for a small comment:

/// Used to synchronize diagnostic responses for added and removed files.

clangd/DraftStore.h

36 ↗

(On Diff #138490)

Let's avoid calling Callback while holding a lock, it is very easy to come up with a code that deadlocks.

I suggest either keeping the getActiveFiles() function or removing the mutex and the locks. IIRC, the DraftStore is never actually used concurrently, so removing mutexes should be no-op change.

[simark]

Changes based on review comments

[ilya-biryukov]

clangd/ClangdLSPServer.h

78 ↗

(On Diff #138569)

NIT: there is no forceReparse() anymore, maybe remove its mention from the comment?
I.e.

/// Reparses all tracked documents, invalidating compile commands cache.
/// As a result, this method may be very expensive.
/// This method is normally called when the compilation database is changed.

111 ↗

(On Diff #138569)

ClangdServer should be the last member! (see the comment above)

clangd/DraftStore.h

48 ↗

(On Diff #138569)

No need for Optionals here anymore. Maybe store std::string and erase on removeDraft?

[simark]

Address latest review comments.

Also, update the comments in DraftStore.{h,cpp} that were outdated.

[ilya-biryukov]

LGTM

clang-tools-extra/trunk/clangd/ClangdLSPServer.h

 //===--- ClangdLSPServer.h - LSP server --------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
 
 #include "ClangdServer.h"
+#include "DraftStore.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"
 #include "Protocol.h"
 #include "ProtocolHandlers.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 
 namespace clang {
private:
   void onFileEvent(DidChangeWatchedFilesParams &Params) override;
   void onCommand(ExecuteCommandParams &Params) override;
   void onRename(RenameParams &Parames) override;
   void onHover(TextDocumentPositionParams &Params) override;
   void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
 
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 
+  /// Forces a reparse of all currently opened files.  As a result, this method
+  /// may be very expensive.  This method is normally called when the
+  /// compilation database is changed.
+  void reparseOpenedFiles();
+
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
   bool ShutdownRequestReceived = false;
 
   /// Used to indicate that the 'exit' notification was received from the
   /// Language Server client.
   /// It's used to break out of the LSP parsing loop.
   bool IsDone = false;
 
   std::mutex FixItsMutex;
   typedef std::map<clangd::Diagnostic, std::vector<Fix>, LSPDiagnosticCompare>
       DiagnosticToReplacementMap;
   /// Caches FixIts per file and diagnostics
   llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
 
   // Various ClangdServer parameters go here. It's important they're created
   // before ClangdServer.
   DirectoryBasedGlobalCompilationDatabase CDB;
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
+
+  // Store of the current versions of the open documents.
+  DraftStore DraftMgr;
+
   // Server must be the last member of the class to allow its destructor to exit
   // the worker thread that may otherwise run an async callback on partially
   // destructed instance of ClangdLSPServer.
   ClangdServer Server;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include "ClangdLSPServer.h"
 #include "Diagnostics.h"
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
 using namespace clang::clangd;
 using namespace clang;
 
 namespace {
 

 }
 
 void ClangdLSPServer::onExit(ExitParams &Params) { IsDone = true; }
 
 void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
   if (Params.metadata && !Params.metadata->extraFlags.empty())
     CDB.setExtraFlagsForFile(Params.textDocument.uri.file(),
                              std::move(Params.metadata->extraFlags));
-  Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text,
-                     WantDiagnostics::Yes);
+
+  PathRef File = Params.textDocument.uri.file();
+  std::string &Contents = Params.textDocument.text;
+
+  DraftMgr.updateDraft(File, Contents);
+  Server.addDocument(File, Contents, WantDiagnostics::Yes);
 }
 
 void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
   if (Params.contentChanges.size() != 1)
     return replyError(ErrorCode::InvalidParams,
                       "can only apply one change at a time");
   auto WantDiags = WantDiagnostics::Auto;
   if (Params.wantDiagnostics.hasValue())
     WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes
                                                   : WantDiagnostics::No;
+
+  PathRef File = Params.textDocument.uri.file();
+  std::string &Contents = Params.contentChanges[0].text;
+
   // We only support full syncing right now.
-  Server.addDocument(Params.textDocument.uri.file(),
-                     Params.contentChanges[0].text, WantDiags);
+  DraftMgr.updateDraft(File, Contents);
+  Server.addDocument(File, Contents, WantDiags);
 }
 
 void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
   Server.onFileEvent(Params);
 }
 
 void ClangdLSPServer::onCommand(ExecuteCommandParams &Params) {
   auto ApplyEdit = [](WorkspaceEdit WE) {
if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
     // 6. The editor applies the changes (applyEdit), and sends us a reply (but
     // we ignore it)
 
     reply("Fix applied.");
     ApplyEdit(*Params.workspaceEdit);
   } else if (Params.command ==
              ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
     auto &FileURI = Params.includeInsertion->textDocument.uri;
-    auto Code = Server.getDocument(FileURI.file());
+    auto Code = DraftMgr.getDraft(FileURI.file());
     if (!Code)
       return replyError(ErrorCode::InvalidParams,
                         ("command " +
                          ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE +
                          " called on non-added file " + FileURI.file())
                             .str());
     llvm::StringRef DeclaringHeader = Params.includeInsertion->declaringHeader;
     if (DeclaringHeader.empty())
if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
     replyError(
         ErrorCode::InvalidParams,
         llvm::formatv("Unsupported command \"{0}\".", Params.command).str());
   }
 }
 
 void ClangdLSPServer::onRename(RenameParams &Params) {
   Path File = Params.textDocument.uri.file();
-  llvm::Optional<std::string> Code = Server.getDocument(File);
+  llvm::Optional<std::string> Code = DraftMgr.getDraft(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onRename called for non-added file");
 
   Server.rename(
       File, Params.position, Params.newName,
       [File, Code,
        Params](llvm::Expected<std::vector<tooling::Replacement>> Replacements) {
         if (!Replacements)
           return replyError(ErrorCode::InternalError,
                             llvm::toString(Replacements.takeError()));
 
         std::vector<TextEdit> Edits = replacementsToEdits(*Code, *Replacements);
         WorkspaceEdit WE;
         WE.changes = {{Params.textDocument.uri.uri(), Edits}};
         reply(WE);
       });
 }
 
 void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams &Params) {
-  Server.removeDocument(Params.textDocument.uri.file());
+  PathRef File = Params.textDocument.uri.file();
+  DraftMgr.removeDraft(File);
+  Server.removeDocument(File);
 }
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
     DocumentOnTypeFormattingParams &Params) {
   auto File = Params.textDocument.uri.file();
-  auto Code = Server.getDocument(File);
+  auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onDocumentOnTypeFormatting called for non-added file");
 
   auto ReplacementsOrError = Server.formatOnType(*Code, File, Params.position);
   if (ReplacementsOrError)
     reply(json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
   else
     replyError(ErrorCode::UnknownErrorCode,
                llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
     DocumentRangeFormattingParams &Params) {
   auto File = Params.textDocument.uri.file();
-  auto Code = Server.getDocument(File);
+  auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onDocumentRangeFormatting called for non-added file");
 
   auto ReplacementsOrError = Server.formatRange(*Code, File, Params.range);
   if (ReplacementsOrError)
     reply(json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
   else
     replyError(ErrorCode::UnknownErrorCode,
                llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
   auto File = Params.textDocument.uri.file();
-  auto Code = Server.getDocument(File);
+  auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onDocumentFormatting called for non-added file");
 
   auto ReplacementsOrError = Server.formatFile(*Code, File);
   if (ReplacementsOrError)
     reply(json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
   else
     replyError(ErrorCode::UnknownErrorCode,
                llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
   // We provide a code action for each diagnostic at the requested location
   // which has FixIts available.
-  auto Code = Server.getDocument(Params.textDocument.uri.file());
+  auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onCodeAction called for non-added file");
 
   json::ary Commands;
   for (Diagnostic &D : Params.context.diagnostics) {
     for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
       WorkspaceEdit WE;

 // FIXME: This function needs to be properly tested.
 void ClangdLSPServer::onChangeConfiguration(
     DidChangeConfigurationParams &Params) {
   ClangdConfigurationParamsChange &Settings = Params.settings;
 
   // Compilation database change.
   if (Settings.compilationDatabasePath.hasValue()) {
     CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
-    Server.reparseOpenedFiles();
+    reparseOpenedFiles();
   }
 }
 
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
                                  llvm::Optional<Path> CompileCommandsDir,
                                  const ClangdServer::Options &Opts)
     : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
Out.writeMessage(json::obj{
       {"method", "textDocument/publishDiagnostics"},
       {"params",
        json::obj{
            {"uri", URIForFile{File}},
            {"diagnostics", std::move(DiagnosticsJSON)},
        }},
   });
 }
+
+void ClangdLSPServer::reparseOpenedFiles() {
+  for (const Path &FilePath : DraftMgr.getActiveFiles())
+    Server.addDocument(FilePath, *DraftMgr.getDraft(FilePath),
+                       WantDiagnostics::Auto,
+                       /*SkipCache=*/true);
+}

clang-tools-extra/trunk/clangd/ClangdServer.h

 //===--- ClangdServer.h - Main clangd server code ----------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDSERVER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDSERVER_H
 
 #include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "CompileArgsCache.h"
-#include "DraftStore.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "TUScheduler.h"
 #include "index/FileIndex.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
public:
   void addDocument(PathRef File, StringRef Contents,
                    WantDiagnostics WD = WantDiagnostics::Auto,
                    bool SkipCache = false);
 
   /// Remove \p File from list of tracked files, schedule a request to free
   /// resources associated with it.
   void removeDocument(PathRef File);
 
-  /// Calls forceReparse() on all currently opened files.
-  /// As a result, this method may be very expensive.
-  /// This method is normally called when the compilation database is changed.
-  /// FIXME: this method must be moved to ClangdLSPServer along with DraftMgr.
-  void reparseOpenedFiles();
-
   /// Run code completion for \p File at \p Pos.
   /// Request is processed asynchronously.
   ///
-  /// The current draft for \p File will be used. If \p UsedFS is non-null, it
-  /// will be overwritten by vfs::FileSystem used for completion.
-  ///
   /// This method should only be called for currently tracked files. However, it
   /// is safe to call removeDocument for \p File after this method returns, even
   /// while returned future is not yet ready.
   /// A version of `codeComplete` that runs \p Callback on the processing thread
   /// when codeComplete results become available.
   void codeComplete(PathRef File, Position Pos,
                     const clangd::CodeCompleteOptions &Opts,
                     Callback<CompletionList> CB);
 
-  /// Provide signature help for \p File at \p Pos. If \p OverridenContents is
-  /// not None, they will used only for signature help, i.e. no diagnostics
-  /// update will be scheduled and a draft for \p File will not be updated. If
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for signature help. This method should only be called for tracked files.
+  /// Provide signature help for \p File at \p Pos.  This method should only be
+  /// called for tracked files.
   void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
 
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   void findDefinitions(PathRef File, Position Pos,
                        Callback<std::vector<Location>> CB);
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
public:
   /// with <> or "" that can be #included directly.
   ///
   /// Both OriginalHeader and InsertedHeader will be considered to determine
   /// whether an include needs to be added.
   Expected<tooling::Replacements> insertInclude(PathRef File, StringRef Code,
                                                 StringRef DeclaringHeader,
                                                 StringRef InsertedHeader);
 
-  /// Gets current document contents for \p File. Returns None if \p File is not
-  /// currently tracked.
-  /// FIXME(ibiryukov): This function is here to allow offset-to-Position
-  /// conversions in outside code, maybe there's a way to get rid of it.
-  llvm::Optional<std::string> getDocument(PathRef File);
-
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
   /// \p File. \p File must be in the list of added documents.
   void dumpAST(PathRef File, UniqueFunction<void(std::string)> Callback);
   /// Called when an event occurs for a watched file in the workspace.
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
   /// Returns estimated memory usage for each of the currently open files.

 
 private:
   /// FIXME: This stats several files to find a .clang-format file. I/O can be
   /// slow. Think of a way to cache this.
   llvm::Expected<tooling::Replacements>
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
+  typedef uint64_t DocVersion;
+
   void consumeDiagnostics(PathRef File, DocVersion Version,
                           std::vector<Diag> Diags);
 
   CompileArgsCache CompileArgs;
   DiagnosticsConsumer &DiagConsumer;
   FileSystemProvider &FSProvider;
-  DraftStore DraftMgr;
+
+  /// Used to synchronize diagnostic responses for added and removed files.
+  llvm::StringMap<DocVersion> InternalVersion;
+
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
   //   - the dynamic index owned by ClangdServer (FileIdx)
   //   - the static index passed to the constructor
   //   - a merged view of a static and dynamic index (MergedIndex)
   SymbolIndex *Index;
   // If present, an up-to-date of symbols in open files. Read via Index.
   std::unique_ptr<FileIndex> FileIdx;

clang-tools-extra/trunk/clangd/ClangdServer.cpp

if (llvm::sys::fs::is_directory(NewRootPath))
     this->RootPath = NewRootPath;
 }
 
 void ClangdServer::addDocument(PathRef File, StringRef Contents,
                                WantDiagnostics WantDiags, bool SkipCache) {
   if (SkipCache)
     CompileArgs.invalidate(File);
 
-  DocVersion Version = DraftMgr.updateDraft(File, Contents);
+  DocVersion Version = ++InternalVersion[File];
   ParseInputs Inputs = {CompileArgs.getCompileCommand(File),
                         FSProvider.getFileSystem(), Contents.str()};
 
   Path FileStr = File.str();
   WorkScheduler.update(File, std::move(Inputs), WantDiags,
                        [this, FileStr, Version](std::vector<Diag> Diags) {
                          consumeDiagnostics(FileStr, Version, std::move(Diags));
                        });
 }
 
 void ClangdServer::removeDocument(PathRef File) {
-  DraftMgr.removeDraft(File);
+  ++InternalVersion[File];
   CompileArgs.invalidate(File);
   WorkScheduler.remove(File);
 }
 
 void ClangdServer::codeComplete(PathRef File, Position Pos,
                                 const clangd::CodeCompleteOptions &Opts,
                                 Callback<CompletionList> CB) {
   // Copy completion options for passing them to async task handler.
   auto CodeCompleteOpts = Opts;
   if (!CodeCompleteOpts.Index) // Respect overridden index.
     CodeCompleteOpts.Index = Index;
 
-  VersionedDraft Latest = DraftMgr.getDraft(File);
-  if (!Latest.Draft)
-    return CB(llvm::make_error<llvm::StringError>(
-        "codeComplete called for non-added document",
-        llvm::errc::invalid_argument));
-
   // Copy PCHs to avoid accessing this->PCHs concurrently
   std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
   auto FS = FSProvider.getFileSystem();
   auto Task = [PCHs, Pos, FS,
                CodeCompleteOpts](Path File, Callback<CompletionList> CB,
                                  llvm::Expected<InputsAndPreamble> IP) {
-    assert(IP && "error when trying to read preamble for codeComplete");
+    if (!IP)
+      return CB(IP.takeError());
+
     auto PreambleData = IP->Preamble;
 
     // 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(
         File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr,
         IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
     CB(std::move(Result));
   };
 
   WorkScheduler.runWithPreamble("CodeComplete", File,
                                 Bind(Task, File.str(), std::move(CB)));
 }
 
 void ClangdServer::signatureHelp(PathRef File, Position Pos,
                                  Callback<SignatureHelp> CB) {
-  VersionedDraft Latest = DraftMgr.getDraft(File);
-  if (!Latest.Draft)
-    return CB(llvm::make_error<llvm::StringError>(
-        "signatureHelp is called for non-added document",
-        llvm::errc::invalid_argument));
 
   auto PCHs = this->PCHs;
   auto FS = FSProvider.getFileSystem();
   auto Action = [Pos, FS, PCHs](Path File, Callback<SignatureHelp> CB,
                                 llvm::Expected<InputsAndPreamble> IP) {
     if (!IP)
       return CB(IP.takeError());
 
ClangdServer::insertInclude(PathRef File, StringRef Code,
   tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude);
   auto Replaces =
       format::cleanupAroundReplacements(Code, tooling::Replacements(R), *Style);
   if (!Replaces)
     return Replaces;
   return formatReplacements(Code, *Replaces, *Style);
 }
 
-llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
-  auto Latest = DraftMgr.getDraft(File);
-  if (!Latest.Draft)
-    return llvm::None;
-  return std::move(*Latest.Draft);
-}
-
 void ClangdServer::dumpAST(PathRef File,
                            UniqueFunction<void(std::string)> Callback) {
   auto Action = [](decltype(Callback) Callback,
                    llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST) {
       ignoreError(InpAST.takeError());
       return Callback("<no-ast>");
     }
ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
   return IncludeReplaces.merge(format::reformat(
       Style.get(), *Changed,
       tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
       File));
 }
 
 void ClangdServer::findDocumentHighlights(
     PathRef File, Position Pos, Callback<std::vector<DocumentHighlight>> CB) {
-  auto FileContents = DraftMgr.getDraft(File);
-  if (!FileContents.Draft)
-    return CB(llvm::make_error<llvm::StringError>(
-        "findDocumentHighlights called on non-added file",
-        llvm::errc::invalid_argument));
 
   auto FS = FSProvider.getFileSystem();
   auto Action = [FS, Pos](Callback<std::vector<DocumentHighlight>> CB,
                           llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::findDocumentHighlights(InpAST->AST, Pos));
   };
 
   WorkScheduler.runWithAST("Highlights", File, Bind(Action, std::move(CB)));
 }
 
 void ClangdServer::findHover(PathRef File, Position Pos, Callback<Hover> CB) {
-  Hover FinalHover;
-  auto FileContents = DraftMgr.getDraft(File);
-  if (!FileContents.Draft)
-    return CB(llvm::make_error<llvm::StringError>(
-        "findHover called on non-added file", llvm::errc::invalid_argument));
-
   auto FS = FSProvider.getFileSystem();
   auto Action = [Pos, FS](Callback<Hover> CB,
                           llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::getHover(InpAST->AST, Pos));
   };
 
void ClangdServer::consumeDiagnostics(PathRef File, DocVersion Version,
   // 64-bit unsigned integer.
   if (Version < LastReportedDiagsVersion)
     return;
   LastReportedDiagsVersion = Version;
 
   DiagConsumer.onDiagnosticsReady(File, std::move(Diags));
 }
 
-void ClangdServer::reparseOpenedFiles() {
-  for (const Path &FilePath : DraftMgr.getActiveFiles())
-    addDocument(FilePath, *DraftMgr.getDraft(FilePath).Draft,
-                WantDiagnostics::Auto, /*SkipCache=*/true);
-}
-
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
 }
 
 std::vector<std::pair<Path, std::size_t>>
 ClangdServer::getUsedBytesPerFile() const {
   return WorkScheduler.getUsedBytesPerFile();
 }
 
 LLVM_NODISCARD bool
 ClangdServer::blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds) {
   return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds));
 }

clang-tools-extra/trunk/clangd/DraftStore.h

 //===--- DraftStore.h - File contents container -----------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_DRAFTSTORE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DRAFTSTORE_H
 
 #include "Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
-#include <cstdint>
 #include <mutex>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
-/// Using unsigned int type here to avoid undefined behaviour on overflow.
-typedef uint64_t DocVersion;
-
-/// Document draft with a version of this draft.
-struct VersionedDraft {
-  DocVersion Version;
-  /// If the value of the field is None, draft is now deleted
-  llvm::Optional<std::string> Draft;
-};
-
 /// A thread-safe container for files opened in a workspace, addressed by
-/// filenames. The contents are owned by the DraftStore. Versions are mantained
-/// for the all added documents, including removed ones. The document version is
-/// incremented on each update and removal of the document.
+/// filenames. The contents are owned by the DraftStore.
 class DraftStore {
 public:
-  /// \return version and contents of the stored document.
-  /// For untracked files, a (0, None) pair is returned.
-  VersionedDraft getDraft(PathRef File) const;
-
-  /// \return List of names of active drafts in this store.  Drafts that were
-  /// removed (which still have an entry in the Drafts map) are not returned by
-  /// this function.
-  std::vector<Path> getActiveFiles() const;
+  /// \return Contents of the stored document.
+  /// For untracked files, a llvm::None is returned.
+  llvm::Optional<std::string> getDraft(PathRef File) const;
 
-  /// \return version of the tracked document.
-  /// For untracked files, 0 is returned.
-  DocVersion getVersion(PathRef File) const;
+  /// \return List of names of the drafts in this store.
+  std::vector<Path> getActiveFiles() const;
 
   /// Replace contents of the draft for \p File with \p Contents.
-  /// \return The new version of the draft for \p File.
-  DocVersion updateDraft(PathRef File, StringRef Contents);
-  /// Remove the contents of the draft
-  /// \return The new version of the draft for \p File.
-  DocVersion removeDraft(PathRef File);
+  void updateDraft(PathRef File, StringRef Contents);
+
+  /// Remove the draft from the store.
+  void removeDraft(PathRef File);
 
 private:
   mutable std::mutex Mutex;
-  llvm::StringMap<VersionedDraft> Drafts;
+  llvm::StringMap<std::string> Drafts;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/trunk/clangd/DraftStore.cpp

 //===--- DraftStore.cpp - File contents container ---------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DraftStore.h"
 
 using namespace clang;
 using namespace clang::clangd;
 
-VersionedDraft DraftStore::getDraft(PathRef File) const {
+llvm::Optional<std::string> DraftStore::getDraft(PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 
   auto It = Drafts.find(File);
   if (It == Drafts.end())
-    return {0, llvm::None};
+    return llvm::None;
+
   return It->second;
 }
 
 std::vector<Path> DraftStore::getActiveFiles() const {
   std::lock_guard<std::mutex> Lock(Mutex);
   std::vector<Path> ResultVector;
 
   for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
-    if (DraftIt->second.Draft)
-      ResultVector.push_back(DraftIt->getKey());
 
   return ResultVector;
 }
 
-DocVersion DraftStore::getVersion(PathRef File) const {
-  std::lock_guard<std::mutex> Lock(Mutex);
-
-  auto It = Drafts.find(File);
-  if (It == Drafts.end())
-    return 0;
-  return It->second.Version;
-}
-
-DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::updateDraft(PathRef File, StringRef Contents) {
   std::lock_guard<std::mutex> Lock(Mutex);
 
-  auto &Entry = Drafts[File];
-  DocVersion NewVersion = ++Entry.Version;
-  Entry.Draft = Contents;
-  return NewVersion;
+  Drafts[File] = Contents;
 }
 
-DocVersion DraftStore::removeDraft(PathRef File) {
+void DraftStore::removeDraft(PathRef File) {
   std::lock_guard<std::mutex> Lock(Mutex);
 
-  auto &Entry = Drafts[File];
-  DocVersion NewVersion = ++Entry.Version;
-  Entry.Draft = llvm::None;
-  return NewVersion;
+  Drafts.erase(File);
 }

clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

)cpp");
   EXPECT_TRUE(bool(Locations));
   EXPECT_THAT(*Locations, ElementsAre(Location{URIForFile{FooCpp},
                                                FooSource.range("one")}));
 
   // Undefine MACRO, close baz.cpp.
   CDB.ExtraClangFlags.clear();
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
-  Server.reparseOpenedFiles();
+  Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto,
+                     /*SkipCache=*/true);
+  Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto,
+                     /*SkipCache=*/true);
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
 
   Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
   EXPECT_TRUE(bool(Locations));
   EXPECT_THAT(*Locations, ElementsAre(Location{URIForFile{FooCpp},