njames93 updated this revision to Diff 327280.
njames93 marked an inline comment as done.
njames93 added a comment.

Address some comments, though I have a feeling @Sammccall, I may have to wait 
until DraftStore has been refactored first before continuing on with this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94554/new/

https://reviews.llvm.org/D94554

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/DraftStore.cpp
  clang-tools-extra/clangd/DraftStore.h
  clang-tools-extra/clangd/unittests/DraftStoreTests.cpp

Index: clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -52,8 +52,8 @@
     llvm::Expected<DraftStore::Draft> Result =
         DS.updateDraft(Path, llvm::None, {Event});
     ASSERT_TRUE(!!Result);
-    EXPECT_EQ(Result->Contents, SrcAfter.code());
-    EXPECT_EQ(DS.getDraft(Path)->Contents, SrcAfter.code());
+    EXPECT_EQ(*Result->Contents, SrcAfter.code());
+    EXPECT_EQ(*DS.getDraft(Path)->Contents, SrcAfter.code());
     EXPECT_EQ(Result->Version, static_cast<int64_t>(i));
   }
 }
@@ -84,8 +84,8 @@
       DS.updateDraft(Path, llvm::None, Changes);
 
   ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());
-  EXPECT_EQ(Result->Contents, FinalSrc.code());
-  EXPECT_EQ(DS.getDraft(Path)->Contents, FinalSrc.code());
+  EXPECT_EQ(*Result->Contents, FinalSrc.code());
+  EXPECT_EQ(*DS.getDraft(Path)->Contents, FinalSrc.code());
   EXPECT_EQ(Result->Version, 1);
 }
 
@@ -345,7 +345,7 @@
 
   Optional<DraftStore::Draft> Contents = DS.getDraft(File);
   EXPECT_TRUE(Contents);
-  EXPECT_EQ(Contents->Contents, OriginalContents);
+  EXPECT_EQ(*Contents->Contents, OriginalContents);
   EXPECT_EQ(Contents->Version, 0);
 }
 
Index: clang-tools-extra/clangd/DraftStore.h
===================================================================
--- clang-tools-extra/clangd/DraftStore.h
+++ clang-tools-extra/clangd/DraftStore.h
@@ -11,8 +11,10 @@
 
 #include "Protocol.h"
 #include "support/Path.h"
+#include "support/ThreadsafeFS.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
 #include <mutex>
 #include <string>
 #include <vector>
@@ -28,10 +30,14 @@
 class DraftStore {
 public:
   struct Draft {
-    std::string Contents;
+    std::shared_ptr<const std::string> Contents;
     int64_t Version = -1;
   };
 
+  DraftStore(const ThreadsafeFS &BaseFS);
+
+  DraftStore() = default;
+
   /// \return Contents of the stored document.
   /// For untracked files, a llvm::None is returned.
   llvm::Optional<Draft> getDraft(PathRef File) const;
@@ -59,9 +65,18 @@
   /// Remove the draft from the store.
   void removeDraft(PathRef File);
 
+  const ThreadsafeFS &getDraftFS() const;
+
 private:
+  struct MetadataDraft {
+    Draft Draft;
+    llvm::sys::TimePoint<> MTime;
+    llvm::sys::fs::UniqueID UID;
+  };
+  class DraftstoreFS;
+  std::unique_ptr<ThreadsafeFS> DraftFS;
   mutable std::mutex Mutex;
-  llvm::StringMap<Draft> Drafts;
+  llvm::StringMap<MetadataDraft> Drafts;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/DraftStore.cpp
===================================================================
--- clang-tools-extra/clangd/DraftStore.cpp
+++ clang-tools-extra/clangd/DraftStore.cpp
@@ -9,7 +9,13 @@
 #include "DraftStore.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
+#include "llvm/Support/Chrono.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <cstdint>
+#include <memory>
+#include <string>
 
 namespace clang {
 namespace clangd {
@@ -21,7 +27,7 @@
   if (It == Drafts.end())
     return None;
 
-  return It->second;
+  return It->second.Draft;
 }
 
 std::vector<Path> DraftStore::getActiveFiles() const {
@@ -47,14 +53,22 @@
   }
 }
 
+static void updateTime(llvm::sys::TimePoint<> &Time) {
+  Time = llvm::sys::TimePoint<>::clock::now();
+}
+
 int64_t DraftStore::addDraft(PathRef File, llvm::Optional<int64_t> Version,
                          llvm::StringRef Contents) {
   std::lock_guard<std::mutex> Lock(Mutex);
 
-  Draft &D = Drafts[File];
-  updateVersion(D, Version);
-  D.Contents = Contents.str();
-  return D.Version;
+  auto Res = Drafts.try_emplace(File);
+  auto &D = Res.first->getValue();
+  if (Res.second)
+    D.UID = llvm::vfs::getNextVirtualUniqueID();
+  updateVersion(D.Draft, Version);
+  updateTime(D.MTime);
+  D.Draft.Contents = std::make_shared<std::string>(Contents);
+  return D.Draft.Version;
 }
 
 llvm::Expected<DraftStore::Draft> DraftStore::updateDraft(
@@ -68,8 +82,8 @@
                  "Trying to do incremental update on non-added document: {0}",
                  File);
   }
-  Draft &D = EntryIt->second;
-  std::string Contents = EntryIt->second.Contents;
+  MetadataDraft &D = EntryIt->second;
+  std::string Contents = *D.Draft.Contents;
 
   for (const TextDocumentContentChangeEvent &Change : Changes) {
     if (!Change.range) {
@@ -120,9 +134,10 @@
     Contents = std::move(NewContents);
   }
 
-  updateVersion(D, Version);
-  D.Contents = std::move(Contents);
-  return D;
+  updateVersion(D.Draft, Version);
+  updateTime(D.MTime);
+  D.Draft.Contents = std::make_shared<std::string>(std::move(Contents));
+  return D.Draft;
 }
 
 void DraftStore::removeDraft(PathRef File) {
@@ -131,5 +146,178 @@
   Drafts.erase(File);
 }
 
+namespace {
+
+/// A read only MemoryBuffer shares ownership of a ref counted string. The
+/// shared string object must not be modified while an owned by this buffer.
+class SharedStringBuffer : public llvm::MemoryBuffer {
+public:
+  static std::unique_ptr<SharedStringBuffer>
+  create(std::shared_ptr<const std::string> Data, StringRef Name) {
+    // Allocate space for the FileContentMemBuffer and its name with null
+    // terminator.
+    static_assert(alignof(SharedStringBuffer) <= sizeof(void *),
+                  "Alignment requirements can't be greater than pointer");
+    auto MemSize = sizeof(SharedStringBuffer) + Name.size() + 1;
+    return std::unique_ptr<SharedStringBuffer>(
+        new (operator new(MemSize)) SharedStringBuffer(std::move(Data), Name));
+  }
+
+  BufferKind getBufferKind() const override {
+    return MemoryBuffer::MemoryBuffer_Malloc;
+  }
+
+  StringRef getBufferIdentifier() const override {
+    return StringRef(nameBegin(), NameSize);
+  }
+
+private:
+  SharedStringBuffer(std::shared_ptr<const std::string> Data, StringRef Name)
+      : BufferContents(std::move(Data)), NameSize(Name.size()) {
+    assert(BufferContents && "Can't create from empty shared_ptr");
+    MemoryBuffer::init(BufferContents->c_str(),
+                       BufferContents->c_str() + BufferContents->size(), true);
+    memcpy(nameBegin(), Name.begin(), Name.size());
+    nameBegin()[Name.size()] = '\0';
+  }
+
+  char *nameBegin() { return (char *)&this[1]; }
+  const char *nameBegin() const { return (const char *)&this[1]; }
+
+  const std::shared_ptr<const std::string> BufferContents;
+  const size_t NameSize;
+};
+
+/// Adapter for \ref llvm::vfs::File that uses a ref counted string.
+class DraftStoreFile : public llvm::vfs::File {
+public:
+  struct FileData {
+    std::shared_ptr<const std::string> Contents;
+    llvm::sys::fs::UniqueID UID;
+    llvm::sys::TimePoint<> MTime;
+  };
+  DraftStoreFile(std::string RequestedName, FileData Data)
+      : RequestedName(std::move(RequestedName)), Data(std::move(Data)) {}
+
+  llvm::ErrorOr<llvm::vfs::Status> status() override {
+    return llvm::vfs::Status(
+        RequestedName, Data.UID, Data.MTime, 0, 0, Data.Contents->size(),
+        llvm::sys::fs::file_type::regular_file, llvm::sys::fs::all_all);
+  }
+
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+  getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+            bool IsVolatile) override {
+    return SharedStringBuffer::create(Data.Contents, RequestedName);
+  }
+
+  std::error_code close() override { return {}; }
+
+  ~DraftStoreFile() override { close(); }
+
+private:
+  std::string RequestedName;
+  FileData Data;
+};
+
+class DraftStoreFileSystem : public llvm::vfs::FileSystem {
+public:
+  DraftStoreFileSystem(llvm::StringMap<DraftStoreFile::FileData> Contents)
+      : Contents(std::move(Contents)) {}
+
+  llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override {
+    SmallString<256> PathStorage;
+    Path.toVector(PathStorage);
+    auto EC = makeAbsolute(PathStorage);
+    assert(!EC);
+    (void)EC;
+    auto Iter = Contents.find(PathStorage);
+    if (Iter == Contents.end())
+      return llvm::errc::no_such_file_or_directory;
+    return DraftStoreFile(std::string(PathStorage), Iter->getValue()).status();
+  }
+
+  llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
+  openFileForRead(const Twine &Path) override {
+    SmallString<256> PathStorage;
+    Path.toVector(PathStorage);
+    auto EC = makeAbsolute(PathStorage);
+    assert(!EC);
+    (void)EC;
+    auto Iter = Contents.find(PathStorage);
+    if (Iter == Contents.end())
+      return llvm::errc::no_such_file_or_directory;
+    return std::make_unique<DraftStoreFile>(std::string(PathStorage),
+                                            Iter->getValue());
+  }
+
+  llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+    return WorkingDirectory;
+  }
+
+  std::error_code setCurrentWorkingDirectory(const Twine &P) override {
+    SmallString<128> Path;
+    P.toVector(Path);
+
+    // Fix up relative paths. This just prepends the current working
+    // directory.
+    std::error_code EC = makeAbsolute(Path);
+    assert(!EC);
+    (void)EC;
+
+    llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
+
+    if (!Path.empty())
+      WorkingDirectory = std::string(Path);
+    return {};
+  }
+  llvm::vfs::directory_iterator dir_begin(const Twine &Dir,
+                                          std::error_code &EC) override {
+    EC = llvm::errc::no_such_file_or_directory;
+    return llvm::vfs::directory_iterator();
+  }
+
+private:
+  std::string WorkingDirectory;
+  llvm::StringMap<DraftStoreFile::FileData> Contents;
+};
+
+} // namespace
+
+class DraftStore::DraftstoreFS : public ThreadsafeFS {
+public:
+  DraftstoreFS(const ThreadsafeFS &Base, const DraftStore &DS)
+      : Base(Base), DS(DS) {}
+
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
+    llvm::StringMap<DraftStoreFile::FileData> Contents;
+    {
+      std::lock_guard<std::mutex> Guard(DS.Mutex);
+      for (const auto &KV : DS.Drafts) {
+        assert(KV.getValue().Draft.Contents && "Draft can't be empty");
+        Contents.insert(std::make_pair(
+            KV.getKey(),
+            DraftStoreFile::FileData{KV.getValue().Draft.Contents,
+                                     KV.getValue().UID, KV.getValue().MTime}));
+      }
+    }
+    auto OFS = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(
+        Base.view(llvm::None));
+    OFS->pushOverlay(
+        llvm::makeIntrusiveRefCnt<DraftStoreFileSystem>(std::move(Contents)));
+    return OFS;
+  }
+
+private:
+  const ThreadsafeFS &Base;
+  const DraftStore &DS;
+};
+
+DraftStore::DraftStore(const ThreadsafeFS &BaseFS)
+    : DraftFS(std::make_unique<DraftstoreFS>(BaseFS, *this)) {}
+const ThreadsafeFS &DraftStore::getDraftFS() const {
+  assert(DraftFS && "No draft fs has been set up");
+  return *DraftFS;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -159,7 +159,13 @@
   /// those arguments for subsequent reparses. However, ClangdServer will check
   /// if compilation arguments changed on calls to forceReparse().
   ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS,
-               const Options &Opts, Callbacks *Callbacks = nullptr);
+               const ThreadsafeFS &DirtyFS, const Options &Opts,
+               Callbacks *Callbacks = nullptr);
+
+  ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS,
+               const Options &Opts, Callbacks *Callbacks = nullptr)
+      : ClangdServer(CDB, TFS, TFS, Opts, Callbacks) {}
+
   ~ClangdServer();
 
   /// Gets the installed module of a given type, if any.
@@ -352,6 +358,8 @@
   ModuleSet *Modules;
   const GlobalCompilationDatabase &CDB;
   const ThreadsafeFS &TFS;
+  /// A File system that overlays open documents over the underlying filesystem.
+  const ThreadsafeFS &DirtyFS;
 
   Path ResourceDir;
   // The index used to look up symbols. This could be:
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -125,9 +125,9 @@
 }
 
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
-                           const ThreadsafeFS &TFS, const Options &Opts,
-                           Callbacks *Callbacks)
-    : Modules(Opts.Modules), CDB(CDB), TFS(TFS),
+                           const ThreadsafeFS &TFS, const ThreadsafeFS &DirtyFS,
+                           const Options &Opts, Callbacks *Callbacks)
+    : Modules(Opts.Modules), CDB(CDB), TFS(TFS), DirtyFS(DirtyFS),
       DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
       ClangTidyProvider(Opts.ClangTidyProvider),
       WorkspaceRoot(Opts.WorkspaceRoot) {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -132,7 +132,7 @@
       // If the file is open in user's editor, make sure the version we
       // saw and current version are compatible as this is the text that
       // will be replaced by editors.
-      if (!It.second.canApplyTo(Draft->Contents)) {
+      if (!It.second.canApplyTo(*Draft->Contents)) {
         ++InvalidFileCount;
         LastInvalidFile = It.first();
       }
@@ -491,7 +491,7 @@
     llvm::Optional<WithContextValue> WithOffsetEncoding;
     if (Opts.Encoding)
       WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
-    Server.emplace(*CDB, TFS, Opts,
+    Server.emplace(*CDB, TFS, DraftMgr.getDraftFS(), Opts,
                    static_cast<ClangdServer::Callbacks *>(this));
   }
   applyConfiguration(Params.initializationOptions.ConfigSettings);
@@ -673,7 +673,7 @@
     return;
   }
 
-  Server->addDocument(File, Draft->Contents, encodeVersion(Draft->Version),
+  Server->addDocument(File, *Draft->Contents, encodeVersion(Draft->Version),
                       WantDiags, Params.forceRebuild);
 }
 
@@ -863,7 +863,7 @@
         "onDocumentOnTypeFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  Server->formatOnType(File, Code->Contents, Params.position, Params.ch,
+  Server->formatOnType(File, *Code->Contents, Params.position, Params.ch,
                        std::move(Reply));
 }
 
@@ -878,11 +878,11 @@
         ErrorCode::InvalidParams));
 
   Server->formatRange(
-      File, Code->Contents, Params.range,
+      File, *Code->Contents, Params.range,
       [Code = Code->Contents, Reply = std::move(Reply)](
           llvm::Expected<tooling::Replacements> Result) mutable {
         if (Result)
-          Reply(replacementsToEdits(Code, Result.get()));
+          Reply(replacementsToEdits(*Code, Result.get()));
         else
           Reply(Result.takeError());
       });
@@ -898,11 +898,11 @@
         "onDocumentFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  Server->formatFile(File, Code->Contents,
+  Server->formatFile(File, *Code->Contents,
                      [Code = Code->Contents, Reply = std::move(Reply)](
                          llvm::Expected<tooling::Replacements> Result) mutable {
                        if (Result)
-                         Reply(replacementsToEdits(Code, Result.get()));
+                         Reply(replacementsToEdits(*Code, Result.get()));
                        else
                          Reply(Result.takeError());
                      });
@@ -1431,8 +1431,7 @@
   Server->getAST(Params.textDocument.uri.file(), Params.range, std::move(CB));
 }
 
-ClangdLSPServer::ClangdLSPServer(Transport &Transp,
-                                 const ThreadsafeFS &TFS,
+ClangdLSPServer::ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
                                  const ClangdLSPServer::Options &Opts)
     : ShouldProfile(/*Period=*/std::chrono::minutes(5),
                     /*Delay=*/std::chrono::minutes(1)),
@@ -1441,7 +1440,8 @@
       BackgroundContext(Context::current().clone()), Transp(Transp),
       MsgHandler(new MessageHandler(*this)), TFS(TFS),
       SupportedSymbolKinds(defaultSymbolKinds()),
-      SupportedCompletionItemKinds(defaultCompletionItemKinds()), Opts(Opts) {
+      SupportedCompletionItemKinds(defaultCompletionItemKinds()), DraftMgr(TFS),
+      Opts(Opts) {
   if (Opts.ConfigProvider) {
     assert(!Opts.ContextProvider &&
            "Only one of ConfigProvider and ContextProvider allowed!");
@@ -1560,14 +1560,14 @@
   auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
   if (!Code)
     return true; // completion code will log the error for untracked doc.
-  auto Offset = positionToOffset(Code->Contents, Params.position,
+  auto Offset = positionToOffset(*Code->Contents, Params.position,
                                  /*AllowColumnsBeyondLineLength=*/false);
   if (!Offset) {
     vlog("could not convert position '{0}' to offset for file '{1}'",
          Params.position, Params.textDocument.uri.file());
     return true;
   }
-  return allowImplicitCompletion(Code->Contents, *Offset);
+  return allowImplicitCompletion(*Code->Contents, *Offset);
 }
 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
@@ -1687,7 +1687,7 @@
   for (const Path &FilePath : DraftMgr.getActiveFiles())
     if (Filter(FilePath))
       if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
-        Server->addDocument(FilePath, std::move(Draft->Contents),
+        Server->addDocument(FilePath, *Draft->Contents,
                             encodeVersion(Draft->Version),
                             WantDiagnostics::Auto);
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to