[clang-tools-extra] 0250b05 - [clangd] Add a Filesystem that overlays Dirty files.
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 9 06:35:29 PST 2021
Author: Nathan James
Date: 2021-03-09T14:35:21Z
New Revision: 0250b053b5aa7fc16408e6c037e5e71db5e9b012
URL: https://github.com/llvm/llvm-project/commit/0250b053b5aa7fc16408e6c037e5e71db5e9b012
DIFF: https://github.com/llvm/llvm-project/commit/0250b053b5aa7fc16408e6c037e5e71db5e9b012.diff
LOG: [clangd] Add a Filesystem that overlays Dirty files.
Create a `ThreadsafeFS` in the `DraftStore` that overlays the dirty file contents over another `ThreadsafeFS`.
This provides a nice thread-safe interface for using dirty file contents throughout the codebase, for example cross file refactoring.
Creating a Filesystem view will overlay a snapshot of the current contents, so if the draft store is updated while the view is being used, it will contain stale contents.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D94554
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index df5377d77f2b..8875f85c8b70 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -666,8 +666,9 @@ void ClangdLSPServer::onDocumentDidChange(
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 @@ void ClangdLSPServer::onDocumentDidChange(
return;
}
}
- Server->addDocument(File, *Code, encodeVersion(Params.textDocument.version),
+ Server->addDocument(File, NewCode, encodeVersion(Params.textDocument.version),
WantDiags, Params.forceRebuild);
}
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index cbd9677f340c..9a5f4bce2e21 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -106,6 +106,23 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
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() {
@@ -132,7 +149,8 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
: FeatureModules(Opts.FeatureModules), 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.
@@ -220,14 +238,14 @@ void ClangdServer::reparseOpenFilesIfNeeded(
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);
}
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 5254bfe78b71..152aa5a49e8b 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -348,7 +348,9 @@ class ClangdServer {
/// 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;
+ /// Gets the contents of a currently tracked file. Returns nullptr if the file
+ /// isn't being tracked.
+ 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.
@@ -394,6 +396,8 @@ class ClangdServer {
// Only written from the main thread (despite being threadsafe).
// FIXME: TUScheduler also keeps these, unify?
DraftStore DraftMgr;
+
+ std::unique_ptr<ThreadsafeFS> DirtyFS;
};
} // namespace clangd
diff --git a/clang-tools-extra/clangd/DraftStore.cpp b/clang-tools-extra/clangd/DraftStore.cpp
index 97ac9eeaf89f..b640f88f5773 100644
--- a/clang-tools-extra/clangd/DraftStore.cpp
+++ b/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 @@ llvm::Optional<DraftStore::Draft> DraftStore::getDraft(PathRef File) const {
if (It == Drafts.end())
return None;
- return It->second;
+ return It->second.Draft;
}
std::vector<Path> DraftStore::getActiveFiles() const {
@@ -75,10 +77,11 @@ std::string DraftStore::addDraft(PathRef File, llvm::StringRef 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 &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 @@ void DraftStore::removeDraft(PathRef File) {
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
diff --git a/clang-tools-extra/clangd/DraftStore.h b/clang-tools-extra/clangd/DraftStore.h
index e997272bd6b4..ff3056e41c29 100644
--- a/clang-tools-extra/clangd/DraftStore.h
+++ b/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 @@ namespace clangd {
class DraftStore {
public:
struct Draft {
- std::string Contents;
+ std::shared_ptr<const std::string> Contents;
std::string Version;
};
@@ -47,9 +48,15 @@ class DraftStore {
/// 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
diff --git a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
index a6ed5d4436a0..5375ea4273af 100644
--- a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -24,20 +24,20 @@ TEST(DraftStore, Versions) {
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
More information about the cfe-commits
mailing list