simark created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov,
mgorny, klimek.
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 <simon.mar...@ericsson.com>
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44408
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/DraftStore.cpp
clangd/DraftStore.h
unittests/clangd/CMakeLists.txt
unittests/clangd/ClangdTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DraftStoreTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===================================================================
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -22,11 +22,13 @@
void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents);
Tagged<CompletionList> runCodeComplete(ClangdServer &Server, PathRef File,
- Position Pos,
+ StringRef Contents, Position Pos,
clangd::CodeCompleteOptions Opts);
-llvm::Expected<Tagged<SignatureHelp>>
-runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos);
+llvm::Expected<Tagged<SignatureHelp>> runSignatureHelp(ClangdServer &Server,
+ PathRef File,
+ StringRef Contents,
+ Position Pos);
llvm::Expected<Tagged<std::vector<Location>>>
runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos);
Index: unittests/clangd/SyncAPI.cpp
===================================================================
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -68,17 +68,19 @@
} // namespace
Tagged<CompletionList> runCodeComplete(ClangdServer &Server, PathRef File,
- Position Pos,
+ StringRef Contents, Position Pos,
clangd::CodeCompleteOptions Opts) {
llvm::Optional<Tagged<CompletionList>> Result;
- Server.codeComplete(File, Pos, Opts, capture(Result));
+ Server.codeComplete(File, Contents, Pos, Opts, capture(Result));
return std::move(*Result);
}
-llvm::Expected<Tagged<SignatureHelp>>
-runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos) {
+llvm::Expected<Tagged<SignatureHelp>> runSignatureHelp(ClangdServer &Server,
+ PathRef File,
+ StringRef Contents,
+ Position Pos) {
llvm::Optional<llvm::Expected<Tagged<SignatureHelp>>> Result;
- Server.signatureHelp(File, Pos, capture(Result));
+ Server.signatureHelp(File, Contents, Pos, capture(Result));
return std::move(*Result);
}
Index: unittests/clangd/DraftStoreTests.cpp
===================================================================
--- /dev/null
+++ unittests/clangd/DraftStoreTests.cpp
@@ -0,0 +1,56 @@
+//===-- DraftStoreTests.cpp - Clangd unit tests -----------------*- 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"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using namespace llvm;
+
+using ::testing::UnorderedElementsAre;
+
+/// Get the active drafts from \p DS as a vector.
+static std::vector<PathRef> getActiveDrafts(const DraftStore &DS) {
+ std::vector<PathRef> ActiveDrafts;
+
+ DS.forEachActiveDraft([&ActiveDrafts](PathRef File, StringRef Contents) {
+ ActiveDrafts.push_back(File);
+ });
+
+ return ActiveDrafts;
+}
+
+TEST(DraftStoreTest, forEachActiveDraft) {
+ DraftStore DS;
+
+ DS.updateDraft("/foo.cpp", "Foo");
+ DS.updateDraft("/bar.cpp", "Bar");
+ DS.updateDraft("/baz.cpp", "Baz");
+
+ std::vector<PathRef> Drafts = getActiveDrafts(DS);
+ EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/bar.cpp", "/baz.cpp"));
+
+ DS.removeDraft("/bar.cpp");
+
+ Drafts = getActiveDrafts(DS);
+ EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/baz.cpp"));
+
+ DS.removeDraft("/another.cpp");
+
+ Drafts = getActiveDrafts(DS);
+ EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/baz.cpp"));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -121,7 +121,8 @@
auto File = testPath("foo.cpp");
Annotations Test(Text);
runAddDocument(Server, File, Test.code());
- auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value;
+ auto CompletionList =
+ runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value;
// Sanity-check that filterText is valid.
EXPECT_THAT(CompletionList.items, Each(NameContainsFilter()));
return CompletionList;
@@ -536,15 +537,17 @@
auto I = memIndex({var("ns::index")});
Opts.Index = I.get();
- auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
+ auto WithIndex =
+ runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value;
EXPECT_THAT(WithIndex.items,
UnorderedElementsAre(Named("local"), Named("index")));
auto ClassFromPreamble =
- runCodeComplete(Server, File, Test.point("2"), Opts).Value;
+ runCodeComplete(Server, File, Test.code(), Test.point("2"), Opts).Value;
EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
Opts.Index = nullptr;
- auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
+ auto WithoutIndex =
+ runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value;
EXPECT_THAT(WithoutIndex.items,
UnorderedElementsAre(Named("local"), Named("preamble")));
}
@@ -575,7 +578,8 @@
)cpp");
runAddDocument(Server, File, Test.code());
- auto Results = runCodeComplete(Server, File, Test.point(), {}).Value;
+ auto Results =
+ runCodeComplete(Server, File, Test.code(), Test.point(), {}).Value;
// "XYZ" and "foo" are not included in the file being completed but are still
// visible through the index.
EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
@@ -614,7 +618,7 @@
auto File = testPath("foo.cpp");
Annotations Test(Text);
runAddDocument(Server, File, Test.code());
- auto R = runSignatureHelp(Server, File, Test.point());
+ auto R = runSignatureHelp(Server, File, Test.code(), Test.point());
assert(R);
return R.get().Value;
}
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -253,13 +253,13 @@
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "";
- Server.forceReparse(FooCpp);
+ Server.forceReparse(FooCpp, SourceContents);
auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "int a;";
- Server.forceReparse(FooCpp);
+ Server.forceReparse(FooCpp, SourceContents);
auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
@@ -284,13 +284,17 @@
FS.Tag = "123";
runAddDocument(Server, FooCpp, SourceContents);
- EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag);
+ EXPECT_EQ(
+ runCodeComplete(Server, FooCpp, SourceContents, Position(), CCOpts).Tag,
+ FS.Tag);
EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
FS.Tag = "321";
runAddDocument(Server, FooCpp, SourceContents);
EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
- EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag);
+ EXPECT_EQ(
+ runCodeComplete(Server, FooCpp, SourceContents, Position(), CCOpts).Tag,
+ FS.Tag);
}
// Only enable this test on Unix
@@ -380,7 +384,7 @@
runAddDocument(Server, FooCpp, SourceContents2);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
// But forceReparse should reparse the file with proper flags.
- Server.forceReparse(FooCpp);
+ Server.forceReparse(FooCpp, SourceContents2);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument calls should finish without errors too.
@@ -419,7 +423,7 @@
runAddDocument(Server, FooCpp, SourceContents);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
// But forceReparse should reparse the file with proper flags.
- Server.forceReparse(FooCpp);
+ Server.forceReparse(FooCpp, SourceContents);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument call should finish without errors too.
@@ -481,7 +485,8 @@
CDB.ExtraClangFlags.clear();
DiagConsumer.clear();
Server.removeDocument(BazCpp);
- Server.reparseOpenedFiles();
+ Server.forceReparse(FooCpp, FooSource.code());
+ Server.forceReparse(BarCpp, BarSource.code());
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_THAT(DiagConsumer.filesWithDiags(),
@@ -536,25 +541,26 @@
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
auto FooCpp = testPath("foo.cpp");
+ StringRef SourceContents = "int main() {}";
// clang cannot create CompilerInvocation if we pass two files in the
// CompileCommand. We pass the file in ExtraFlags once and CDB adds another
// one in getCompileCommand().
CDB.ExtraClangFlags.push_back(FooCpp);
// Clang can't parse command args in that case, but we shouldn't crash.
- runAddDocument(Server, FooCpp, "int main() {}");
+ runAddDocument(Server, FooCpp, SourceContents);
EXPECT_EQ(runDumpAST(Server, FooCpp), "<no-ast>");
EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position()));
EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name"));
// FIXME: codeComplete and signatureHelp should also return errors when they
// can't parse the file.
- EXPECT_THAT(
- runCodeComplete(Server, FooCpp, Position(), clangd::CodeCompleteOptions())
- .Value.items,
- IsEmpty());
- auto SigHelp = runSignatureHelp(Server, FooCpp, Position());
+ EXPECT_THAT(runCodeComplete(Server, FooCpp, SourceContents, Position(),
+ clangd::CodeCompleteOptions())
+ .Value.items,
+ IsEmpty());
+ auto SigHelp = runSignatureHelp(Server, FooCpp, SourceContents, Position());
ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
EXPECT_THAT(SigHelp->Value.signatures, IsEmpty());
}
@@ -706,11 +712,16 @@
auto ForceReparseRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
+ const RequestStats &Stats = ReqStats[FileIndex];
+
// Make sure we don't violate the ClangdServer's contract.
- if (ReqStats[FileIndex].FileIsRemoved)
+ if (Stats.FileIsRemoved)
AddDocument(FileIndex);
- Server.forceReparse(FilePaths[FileIndex]);
+ Server.forceReparse(FilePaths[FileIndex],
+ Stats.LastContentsHadErrors
+ ? SourceContentsWithErrors
+ : SourceContentsWithoutErrors);
UpdateStatsOnForceReparse(FileIndex);
};
@@ -726,8 +737,10 @@
auto CodeCompletionRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
+ const RequestStats &Stats = ReqStats[FileIndex];
+
// Make sure we don't violate the ClangdServer's contract.
- if (ReqStats[FileIndex].FileIsRemoved)
+ if (Stats.FileIsRemoved)
AddDocument(FileIndex);
Position Pos;
@@ -739,8 +752,10 @@
// requests as opposed to AddDocument/RemoveDocument, which are implicitly
// cancelled by any subsequent AddDocument/RemoveDocument request to the
// same file.
- runCodeComplete(Server, FilePaths[FileIndex], Pos,
- clangd::CodeCompleteOptions());
+ runCodeComplete(Server, FilePaths[FileIndex],
+ Stats.LastContentsHadErrors ? SourceContentsWithErrors
+ : SourceContentsWithoutErrors,
+ Pos, clangd::CodeCompleteOptions());
};
auto FindDefinitionsRequest = [&]() {
Index: unittests/clangd/CMakeLists.txt
===================================================================
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -15,6 +15,7 @@
CodeCompleteTests.cpp
CodeCompletionStringsTests.cpp
ContextTests.cpp
+ DraftStoreTests.cpp
FileIndexTests.cpp
FuzzyMatchTests.cpp
HeadersTests.cpp
Index: clangd/DraftStore.h
===================================================================
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -21,45 +21,30 @@
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.
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;
+ 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;
+ /// Call \p Callback for each active draft, while holding the store lock.
+ void forEachActiveDraft(
+ llvm::function_ref<void(PathRef, StringRef)> Callback) 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);
+ void 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 removeDraft(PathRef File);
private:
mutable std::mutex Mutex;
- llvm::StringMap<VersionedDraft> Drafts;
+ llvm::StringMap<llvm::Optional<std::string>> Drafts;
};
} // namespace clangd
Index: clangd/DraftStore.cpp
===================================================================
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -12,49 +12,33 @@
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 {
+void DraftStore::forEachActiveDraft(
+ llvm::function_ref<void(PathRef, StringRef)> Callback) 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;
+ if (DraftIt->second)
+ Callback(DraftIt->getKey(), *DraftIt->second);
}
-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[File] = llvm::None;
}
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -155,47 +155,41 @@
/// \p File is already tracked. Also schedules parsing of the AST for it on a
/// separate thread. When the parsing is complete, DiagConsumer passed in
/// constructor will receive onDiagnosticsReady callback.
- void addDocument(PathRef File, StringRef Contents,
+ void addDocument(PathRef File, std::string Contents,
WantDiagnostics WD = WantDiagnostics::Auto);
/// Remove \p File from list of tracked files, schedule a request to free
/// resources associated with it.
void removeDocument(PathRef File);
- /// Force \p File to be reparsed using the latest contents.
+ /// Force \p File to be reparsed using the \p Contents as the contents.
/// Will also check if CompileCommand, provided by GlobalCompilationDatabase
/// for \p File has changed. If it has, will remove currently stored Preamble
/// and AST and rebuild them from scratch.
- void forceReparse(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.
- void reparseOpenedFiles();
+ void forceReparse(PathRef File, std::string Contents);
/// 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.
+ /// \p Contents is the current content of the file. 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,
+ void codeComplete(PathRef File, std::string Contents, Position Pos,
const clangd::CodeCompleteOptions &Opts,
UniqueFunction<void(Tagged<CompletionList>)> Callback,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
- /// 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. \p Contents is the current
+ /// of the file. 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.
void signatureHelp(
- PathRef File, Position Pos,
+ PathRef File, std::string Contents, Position Pos,
UniqueFunction<void(llvm::Expected<Tagged<SignatureHelp>>)> Callback,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
@@ -255,12 +249,6 @@
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.
@@ -289,15 +277,27 @@
formatCode(llvm::StringRef Code, PathRef File,
ArrayRef<tooling::Range> Ranges);
+ typedef uint64_t DocVersion;
+
+ /// Document contents with a version of this content.
+ struct VersionedContents {
+ DocVersion Version;
+ std::string Contents;
+ };
+
void
- scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
+ scheduleReparseAndDiags(PathRef File, VersionedContents Contents,
WantDiagnostics WD,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
CompileArgsCache CompileArgs;
DiagnosticsConsumer &DiagConsumer;
FileSystemProvider &FSProvider;
- DraftStore DraftMgr;
+
+ // For each file, an internal number representing the latest version we know
+ // about it.
+ 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)
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -116,36 +116,31 @@
this->RootPath = NewRootPath;
}
-void ClangdServer::addDocument(PathRef File, StringRef Contents,
+void ClangdServer::addDocument(PathRef File, std::string Contents,
WantDiagnostics WantDiags) {
- DocVersion Version = DraftMgr.updateDraft(File, Contents);
+ DocVersion Version = ++InternalVersion[File];
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
- scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
+ scheduleReparseAndDiags(File, VersionedContents{Version, std::move(Contents)},
WantDiags, std::move(TaggedFS));
}
void ClangdServer::removeDocument(PathRef File) {
- DraftMgr.removeDraft(File);
+ ++InternalVersion[File];
CompileArgs.invalidate(File);
WorkScheduler.remove(File);
}
-void ClangdServer::forceReparse(PathRef File) {
- auto FileContents = DraftMgr.getDraft(File);
- assert(FileContents.Draft &&
- "forceReparse() was called for non-added document");
-
+void ClangdServer::forceReparse(PathRef File, std::string Contents) {
// forceReparse promises to request new compilation flags from CDB, so we
- // remove any cahced flags.
+ // remove any cached flags.
CompileArgs.invalidate(File);
- auto TaggedFS = FSProvider.getTaggedFileSystem(File);
- scheduleReparseAndDiags(File, std::move(FileContents), WantDiagnostics::Yes,
- std::move(TaggedFS));
+ addDocument(File, std::move(Contents), WantDiagnostics::Yes);
}
void ClangdServer::codeComplete(
- PathRef File, Position Pos, const clangd::CodeCompleteOptions &Opts,
+ PathRef File, std::string Contents, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
UniqueFunction<void(Tagged<CompletionList>)> Callback,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
using CallbackType = UniqueFunction<void(Tagged<CompletionList>)>;
@@ -159,10 +154,6 @@
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- VersionedDraft Latest = DraftMgr.getDraft(File);
- // FIXME(sammccall): return error for consistency?
- assert(Latest.Draft && "codeComplete called for non-added document");
-
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
auto Task = [PCHs, Pos, TaggedFS, CodeCompleteOpts](
@@ -183,23 +174,17 @@
WorkScheduler.runWithPreamble(
"CodeComplete", File,
- Bind(Task, std::move(*Latest.Draft), File.str(), std::move(Callback)));
+ Bind(Task, std::move(Contents), File.str(), std::move(Callback)));
}
void ClangdServer::signatureHelp(
- PathRef File, Position Pos,
+ PathRef File, std::string Contents, Position Pos,
UniqueFunction<void(llvm::Expected<Tagged<SignatureHelp>>)> Callback,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
if (UsedFS)
*UsedFS = TaggedFS.Value;
- VersionedDraft Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return Callback(llvm::make_error<llvm::StringError>(
- "signatureHelp is called for non-added document",
- llvm::errc::invalid_argument));
-
auto PCHs = this->PCHs;
auto Action = [Pos, TaggedFS, PCHs](std::string Contents, Path File,
decltype(Callback) Callback,
@@ -218,7 +203,7 @@
WorkScheduler.runWithPreamble(
"SignatureHelp", File,
- Bind(Action, std::move(*Latest.Draft), File.str(), std::move(Callback)));
+ Bind(Action, std::move(Contents), File.str(), std::move(Callback)));
}
llvm::Expected<tooling::Replacements>
@@ -360,13 +345,6 @@
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,
@@ -489,11 +467,6 @@
PathRef File, Position Pos,
UniqueFunction<void(llvm::Expected<Tagged<std::vector<DocumentHighlight>>>)>
Callback) {
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return Callback(llvm::make_error<llvm::StringError>(
- "findDocumentHighlights called on non-added file",
- llvm::errc::invalid_argument));
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
@@ -512,12 +485,6 @@
void ClangdServer::findHover(
PathRef File, Position Pos,
UniqueFunction<void(llvm::Expected<Tagged<Hover>>)> Callback) {
- Hover FinalHover;
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return Callback(llvm::make_error<llvm::StringError>(
- "findHover called on non-added file", llvm::errc::invalid_argument));
-
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
auto Action = [Pos, TaggedFS](decltype(Callback) Callback,
@@ -533,7 +500,7 @@
}
void ClangdServer::scheduleReparseAndDiags(
- PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags,
+ PathRef File, VersionedContents Contents, WantDiagnostics WantDiags,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
@@ -561,15 +528,10 @@
WorkScheduler.update(File,
ParseInputs{std::move(Command),
std::move(TaggedFS.Value),
- std::move(*Contents.Draft)},
+ std::move(Contents.Contents)},
WantDiags, std::move(Callback));
}
-void ClangdServer::reparseOpenedFiles() {
- for (const Path &FilePath : DraftMgr.getActiveFiles())
- forceReparse(FilePath);
-}
-
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
// FIXME: Do nothing for now. This will be used for indexing and potentially
// invalidating other caches.
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -75,6 +75,17 @@
std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
+ /// Forces a reparse 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.
+ void reparseOpenedFiles();
+
+ /// 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);
+
JSONOutput &Out;
/// Used to indicate that the 'shutdown' request was received from the
/// Language Server client.
@@ -101,6 +112,9 @@
// the worker thread that may otherwise run an async callback on partially
// destructed instance of ClangdLSPServer.
ClangdServer Server;
+
+ // Store of the current versions of the open documents.
+ DraftStore DraftMgr;
};
} // namespace clangd
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
#include "URI.h"
+#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
@@ -142,8 +143,12 @@
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, std::move(Contents), WantDiagnostics::Yes);
}
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
@@ -154,9 +159,13 @@
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, std::move(Contents), WantDiags);
}
void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
@@ -188,7 +197,7 @@
} else if (Params.command ==
ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
auto &FileURI = Params.includeInsertion->textDocument.uri;
- auto Code = Server.getDocument(FileURI.file());
+ auto Code = getDocument(FileURI.file());
if (!Code)
return replyError(ErrorCode::InvalidParams,
("command " +
@@ -233,7 +242,7 @@
void ClangdLSPServer::onRename(RenameParams &Params) {
Path File = Params.textDocument.uri.file();
- llvm::Optional<std::string> Code = Server.getDocument(File);
+ llvm::Optional<std::string> Code = getDocument(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onRename called for non-added file");
@@ -254,13 +263,15 @@
}
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 = getDocument(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentOnTypeFormatting called for non-added file");
@@ -276,7 +287,7 @@
void ClangdLSPServer::onDocumentRangeFormatting(
DocumentRangeFormattingParams &Params) {
auto File = Params.textDocument.uri.file();
- auto Code = Server.getDocument(File);
+ auto Code = getDocument(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentRangeFormatting called for non-added file");
@@ -291,7 +302,7 @@
void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
auto File = Params.textDocument.uri.file();
- auto Code = Server.getDocument(File);
+ auto Code = getDocument(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentFormatting called for non-added file");
@@ -307,7 +318,7 @@
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 = getDocument(Params.textDocument.uri.file());
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onCodeAction called for non-added file");
@@ -329,19 +340,32 @@
}
void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {
- Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
+ PathRef File = Params.textDocument.uri.file();
+
+ llvm::Optional<std::string> Contents = DraftMgr.getDraft(File);
+ // FIXME(sammccall): return error for consistency?
+ assert(Contents && "codeComplete called for non-added document");
+
+ Server.codeComplete(File, std::move(*Contents), Params.position, CCOpts,
[](Tagged<CompletionList> List) { reply(List.Value); });
}
void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
- Server.signatureHelp(Params.textDocument.uri.file(), Params.position,
- [](llvm::Expected<Tagged<SignatureHelp>> SignatureHelp) {
- if (!SignatureHelp)
- return replyError(
- ErrorCode::InvalidParams,
- llvm::toString(SignatureHelp.takeError()));
- reply(SignatureHelp->Value);
- });
+ PathRef File = Params.textDocument.uri.file();
+ auto Callback = [](llvm::Expected<Tagged<SignatureHelp>> SignatureHelp) {
+ if (!SignatureHelp)
+ return replyError(ErrorCode::InvalidParams,
+ llvm::toString(SignatureHelp.takeError()));
+ reply(SignatureHelp->Value);
+ };
+
+ llvm::Optional<std::string> Contents = DraftMgr.getDraft(File);
+ if (!Contents)
+ return Callback(llvm::make_error<llvm::StringError>(
+ "signatureHelp is called for non-added document",
+ llvm::errc::invalid_argument));
+
+ Server.signatureHelp(File, std::move(*Contents), Params.position, Callback);
}
void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) {
@@ -361,27 +385,30 @@
}
void ClangdLSPServer::onDocumentHighlight(TextDocumentPositionParams &Params) {
- Server.findDocumentHighlights(
- Params.textDocument.uri.file(), Params.position,
+ PathRef File = Params.textDocument.uri.file();
+ auto Callback =
[](llvm::Expected<Tagged<std::vector<DocumentHighlight>>> Highlights) {
if (!Highlights)
return replyError(ErrorCode::InternalError,
llvm::toString(Highlights.takeError()));
reply(json::ary(Highlights->Value));
- });
+ };
+
+ Server.findDocumentHighlights(File, Params.position, Callback);
}
void ClangdLSPServer::onHover(TextDocumentPositionParams &Params) {
- Server.findHover(Params.textDocument.uri.file(), Params.position,
- [](llvm::Expected<Tagged<Hover>> H) {
- if (!H) {
- replyError(ErrorCode::InternalError,
- llvm::toString(H.takeError()));
- return;
- }
-
- reply(H->Value);
- });
+ PathRef File = Params.textDocument.uri.file();
+ auto Callback = [](llvm::Expected<Tagged<Hover>> H) {
+ if (!H) {
+ replyError(ErrorCode::InternalError, llvm::toString(H.takeError()));
+ return;
+ }
+
+ reply(H->Value);
+ };
+
+ Server.findHover(File, Params.position, Callback);
}
// FIXME: This function needs to be properly tested.
@@ -392,7 +419,7 @@
// Compilation database change.
if (Settings.compilationDatabasePath.hasValue()) {
CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
- Server.reparseOpenedFiles();
+ reparseOpenedFiles();
}
}
@@ -474,3 +501,17 @@
}},
});
}
+
+void ClangdLSPServer::reparseOpenedFiles() {
+ DraftMgr.forEachActiveDraft([this](StringRef File, StringRef Contents) {
+ Server.forceReparse(File, Contents);
+ });
+}
+
+llvm::Optional<std::string> ClangdLSPServer::getDocument(PathRef File) {
+ llvm::Optional<std::string> Contents = DraftMgr.getDraft(File);
+ if (!Contents)
+ return llvm::None;
+
+ return std::move(*Contents);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
