njames93 updated this revision to Diff 329070.
njames93 marked 2 inline comments as done.
njames93 added a comment.
Address comments.
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;
+ std::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,11 @@
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 &D = Drafts[File];
+ updateVersion(D.Draft, Version);
+ std::time(&D.MTime);
+ D.Draft.Contents = std::make_shared<std::string>(Contents);
+ return D.Draft.Version;
}
void DraftStore::removeDraft(PathRef File) {
@@ -87,5 +90,39 @@
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(),
+ /*RequiresNullTerminator=*/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,8 @@
// Only written from the main thread (despite being threadsafe).
// FIXME: TUScheduler also keeps these, unify?
DraftStore DraftMgr;
+
+ const std::unique_ptr<ThreadsafeFS> DirtyFS;
};
} // namespace clangd
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -104,6 +104,23 @@
ClangdServer::Callbacks *ServerCallbacks;
};
+class DraftStoreFS : public ThreadsafeFS {
+public:
+ DraftStoreFS(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;
+};
+
} // namespace
ClangdServer::Options ClangdServer::optsForTest() {
@@ -130,7 +147,8 @@
: Modules(Opts.Modules), CDB(CDB), TFS(TFS),
DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
ClangTidyProvider(Opts.ClangTidyProvider),
- WorkspaceRoot(Opts.WorkspaceRoot) {
+ WorkspaceRoot(Opts.WorkspaceRoot),
+ DirtyFS(std::make_unique<DraftStoreFS>(TFS, DraftMgr)) {
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
// is parsed.
@@ -218,14 +236,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
