njames93 updated this revision to Diff 327646.
njames93 added a comment.
Getting there.
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
@@ -24,20 +24,20 @@
EXPECT_EQ("25", DS.addDraft(File, "25", ""));
EXPECT_EQ("25", DS.getDraft(File)->Version);
- EXPECT_EQ("", DS.getDraft(File)->Contents);
+ EXPECT_EQ("", *DS.getDraft(File)->Contents);
EXPECT_EQ("26", DS.addDraft(File, "", "x"));
EXPECT_EQ("26", DS.getDraft(File)->Version);
- EXPECT_EQ("x", DS.getDraft(File)->Contents);
+ EXPECT_EQ("x", *DS.getDraft(File)->Contents);
EXPECT_EQ("27", DS.addDraft(File, "", "x")) << "no-op change";
EXPECT_EQ("27", DS.getDraft(File)->Version);
- EXPECT_EQ("x", DS.getDraft(File)->Contents);
+ EXPECT_EQ("x", *DS.getDraft(File)->Contents);
// We allow versions to go backwards.
EXPECT_EQ("7", DS.addDraft(File, "7", "y"));
EXPECT_EQ("7", DS.getDraft(File)->Version);
- EXPECT_EQ("y", DS.getDraft(File)->Contents);
+ EXPECT_EQ("y", *DS.getDraft(File)->Contents);
}
} // namespace
Index: clang-tools-extra/clangd/DraftStore.h
===================================================================
--- clang-tools-extra/clangd/DraftStore.h
+++ clang-tools-extra/clangd/DraftStore.h
@@ -13,6 +13,7 @@
#include "support/Path.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include <mutex>
#include <string>
#include <vector>
@@ -27,7 +28,7 @@
class DraftStore {
public:
struct Draft {
- std::string Contents;
+ std::shared_ptr<const std::string> Contents;
std::string Version;
};
@@ -47,9 +48,15 @@
/// Remove the draft from the store.
void removeDraft(PathRef File);
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> asVFS() const;
+
private:
+ struct DraftAndTime {
+ Draft Draft;
+ time_t MTime;
+ };
mutable std::mutex Mutex;
- llvm::StringMap<Draft> Drafts;
+ llvm::StringMap<DraftAndTime> Drafts;
};
} // namespace clangd
Index: clang-tools-extra/clangd/DraftStore.cpp
===================================================================
--- clang-tools-extra/clangd/DraftStore.cpp
+++ clang-tools-extra/clangd/DraftStore.cpp
@@ -11,6 +11,8 @@
#include "support/Logger.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Errc.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
namespace clang {
namespace clangd {
@@ -22,7 +24,7 @@
if (It == Drafts.end())
return None;
- return It->second;
+ return It->second.Draft;
}
std::vector<Path> DraftStore::getActiveFiles() const {
@@ -75,10 +77,12 @@
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();
+ updateVersion(D.Draft, Version);
+ time(&D.MTime);
+ D.Draft.Contents = std::make_shared<std::string>(Contents);
+ return D.Draft.Version;
}
void DraftStore::removeDraft(PathRef File) {
@@ -87,5 +91,38 @@
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 {
+ const std::shared_ptr<const std::string> BufferContents;
+ const std::string Name;
+
+public:
+ BufferKind getBufferKind() const override {
+ return MemoryBuffer::MemoryBuffer_Malloc;
+ }
+
+ StringRef getBufferIdentifier() const override { return Name; }
+
+ SharedStringBuffer(std::shared_ptr<const std::string> Data, StringRef Name)
+ : BufferContents(std::move(Data)), Name(Name) {
+ assert(BufferContents && "Can't create from empty shared_ptr");
+ MemoryBuffer::init(BufferContents->c_str(),
+ BufferContents->c_str() + BufferContents->size(), true);
+ }
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> DraftStore::asVFS() const {
+ auto MemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ std::lock_guard<std::mutex> Guard(Mutex);
+ for (const auto &Draft : Drafts)
+ MemFS->addFile(Draft.getKey(), Draft.getValue().MTime,
+ std::make_unique<SharedStringBuffer>(
+ Draft.getValue().Draft.Contents, Draft.getKey()));
+ return MemFS;
+}
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -339,7 +339,7 @@
/// FIXME: those metrics might be useful too, we should add them.
llvm::StringMap<TUScheduler::FileStats> fileStats() const;
- llvm::Optional<std::string> getDraft(PathRef File) const;
+ std::shared_ptr<const std::string> getDraft(PathRef File) const;
// Blocks the main thread until the server is idle. Only for use in tests.
// Returns false if the timeout expires.
@@ -385,6 +385,25 @@
// Only written from the main thread (despite being threadsafe).
// FIXME: TUScheduler also keeps these, unify?
DraftStore DraftMgr;
+
+ class DirtyFS : public ThreadsafeFS {
+ public:
+ DirtyFS(const ThreadsafeFS &Base, const DraftStore &Drafts)
+ : Base(Base), DirtyFiles(Drafts) {}
+
+ private:
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
+ auto OFS = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(
+ Base.view(llvm::None));
+ OFS->pushOverlay(DirtyFiles.asVFS());
+ return OFS;
+ }
+
+ const ThreadsafeFS &Base;
+ const DraftStore &DirtyFiles;
+ };
+
+ DirtyFS DirtyFS{TFS, DraftMgr};
};
} // namespace clangd
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -218,14 +218,14 @@
for (const Path &FilePath : DraftMgr.getActiveFiles())
if (Filter(FilePath))
if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
- addDocument(FilePath, std::move(Draft->Contents), Draft->Version,
+ addDocument(FilePath, *Draft->Contents, Draft->Version,
WantDiagnostics::Auto);
}
-llvm::Optional<std::string> ClangdServer::getDraft(PathRef File) const {
+std::shared_ptr<const std::string> ClangdServer::getDraft(PathRef File) const {
auto Draft = DraftMgr.getDraft(File);
if (!Draft)
- return llvm::None;
+ return nullptr;
return std::move(Draft->Contents);
}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -666,8 +666,9 @@
log("Trying to incrementally change non-added document: {0}", File);
return;
}
+ std::string NewCode(*Code);
for (const auto &Change : Params.contentChanges) {
- if (auto Err = applyChange(*Code, Change)) {
+ if (auto Err = applyChange(NewCode, Change)) {
// If this fails, we are most likely going to be not in sync anymore with
// the client. It is better to remove the draft and let further
// operations fail rather than giving wrong results.
@@ -676,7 +677,7 @@
return;
}
}
- Server->addDocument(File, *Code, encodeVersion(Params.textDocument.version),
+ Server->addDocument(File, NewCode, encodeVersion(Params.textDocument.version),
WantDiags, Params.forceRebuild);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
