simark updated this revision to Diff 138253.
simark added a comment.
Rebase
Non-trivial rebase on today's master.
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);
llvm::Expected<CompletionList>
-runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
- clangd::CodeCompleteOptions Opts);
+runCodeComplete(ClangdServer &Server, PathRef File, std::string Contents,
+ Position Pos, clangd::CodeCompleteOptions Opts);
llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
- PathRef File, Position Pos);
+ PathRef File,
+ std::string Contents,
+ Position Pos);
llvm::Expected<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
llvm::Expected<CompletionList>
-runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
- clangd::CodeCompleteOptions Opts) {
+runCodeComplete(ClangdServer &Server, PathRef File, std::string Contents,
+ Position Pos, clangd::CodeCompleteOptions Opts) {
llvm::Optional<llvm::Expected<CompletionList>> Result;
- Server.codeComplete(File, Pos, Opts, capture(Result));
+ Server.codeComplete(File, std::move(Contents), Pos, Opts, capture(Result));
return std::move(*Result);
}
llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
- PathRef File, Position Pos) {
+ PathRef File,
+ std::string Contents,
+ Position Pos) {
llvm::Optional<llvm::Expected<SignatureHelp>> Result;
- Server.signatureHelp(File, Pos, capture(Result));
+ Server.signatureHelp(File, std::move(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
@@ -122,7 +122,7 @@
Annotations Test(Text);
runAddDocument(Server, File, Test.code());
auto CompletionList =
- cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+ cantFail(runCodeComplete(Server, File, Test.code(), Test.point(), Opts));
// Sanity-check that filterText is valid.
EXPECT_THAT(CompletionList.items, Each(NameContainsFilter()));
return CompletionList;
@@ -537,16 +537,17 @@
auto I = memIndex({var("ns::index")});
Opts.Index = I.get();
- auto WithIndex = cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+ auto WithIndex =
+ cantFail(runCodeComplete(Server, File, Test.code(), Test.point(), Opts));
EXPECT_THAT(WithIndex.items,
UnorderedElementsAre(Named("local"), Named("index")));
- auto ClassFromPreamble =
- cantFail(runCodeComplete(Server, File, Test.point("2"), Opts));
+ auto ClassFromPreamble = cantFail(
+ runCodeComplete(Server, File, Test.code(), Test.point("2"), Opts));
EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
Opts.Index = nullptr;
auto WithoutIndex =
- cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+ cantFail(runCodeComplete(Server, File, Test.code(), Test.point(), Opts));
EXPECT_THAT(WithoutIndex.items,
UnorderedElementsAre(Named("local"), Named("preamble")));
}
@@ -577,7 +578,8 @@
)cpp");
runAddDocument(Server, File, Test.code());
- auto Results = cantFail(runCodeComplete(Server, File, Test.point(), {}));
+ auto Results =
+ cantFail(runCodeComplete(Server, File, Test.code(), Test.point(), {}));
// "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));
@@ -616,7 +618,7 @@
auto File = testPath("foo.cpp");
Annotations Test(Text);
runAddDocument(Server, File, Test.code());
- return cantFail(runSignatureHelp(Server, File, Test.point()));
+ return cantFail(runSignatureHelp(Server, File, Test.code(), Test.point()));
}
MATCHER_P(ParamsAre, P, "") {
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -245,13 +245,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());
@@ -375,7 +375,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.
@@ -413,7 +413,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.
@@ -475,7 +475,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(),
@@ -530,25 +531,28 @@
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(cantFail(runCodeComplete(Server, FooCpp, Position(),
- clangd::CodeCompleteOptions()))
- .items,
- IsEmpty());
- auto SigHelp = runSignatureHelp(Server, FooCpp, Position());
+ EXPECT_THAT(
+ cantFail(runCodeComplete(Server, FooCpp, SourceContents.str(), Position(),
+ clangd::CodeCompleteOptions()))
+ .items,
+ IsEmpty());
+ auto SigHelp =
+ runSignatureHelp(Server, FooCpp, SourceContents.str(), Position());
ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
EXPECT_THAT(SigHelp->signatures, IsEmpty());
}
@@ -564,19 +568,23 @@
// BlockingRequestInterval-request will be a blocking one.
const unsigned BlockingRequestInterval = 40;
- const auto SourceContentsWithoutErrors = R"cpp(
-int a;
-int b;
-int c;
-int d;
-)cpp";
+ auto GetSourceContents = [](bool WithErrors) {
+ const auto SourceContentsWithoutErrors = R"cpp(
+ int a;
+ int b;
+ int c;
+ int d;
+ )cpp";
- const auto SourceContentsWithErrors = R"cpp(
-int a = x;
-int b;
-int c;
-int d;
-)cpp";
+ const auto SourceContentsWithErrors = R"cpp(
+ int a = x;
+ int b;
+ int c;
+ int d;
+ )cpp";
+
+ return WithErrors ? SourceContentsWithErrors : SourceContentsWithoutErrors;
+ };
// Giving invalid line and column number should not crash ClangdServer, but
// just to make sure we're sometimes hitting the bounds inside the file we
@@ -687,8 +695,7 @@
auto AddDocument = [&](unsigned FileIndex) {
bool ShouldHaveErrors = ShouldHaveErrorsDist(RandGen);
Server.addDocument(FilePaths[FileIndex],
- ShouldHaveErrors ? SourceContentsWithErrors
- : SourceContentsWithoutErrors);
+ GetSourceContents(ShouldHaveErrors));
UpdateStatsOnAddDocument(FileIndex, ShouldHaveErrors);
};
@@ -700,11 +707,14 @@
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],
+ GetSourceContents(Stats.LastContentsHadErrors));
UpdateStatsOnForceReparse(FileIndex);
};
@@ -720,8 +730,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;
@@ -733,8 +745,9 @@
// requests as opposed to AddDocument/RemoveDocument, which are implicitly
// cancelled by any subsequent AddDocument/RemoveDocument request to the
// same file.
- cantFail(runCodeComplete(Server, FilePaths[FileIndex], Pos,
- clangd::CodeCompleteOptions()));
+ cantFail(runCodeComplete(Server, FilePaths[FileIndex],
+ GetSourceContents(Stats.LastContentsHadErrors),
+ 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
@@ -13,53 +13,36 @@
#include "Path.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/StringMap.h"
-#include <cstdint>
#include <mutex>
#include <string>
-#include <vector>
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
@@ -117,45 +117,38 @@
/// \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,
Callback<CompletionList> CB);
- /// 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.
- void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
+ /// Provide signature help for \p File at \p Pos. \p Contents is the current
+ /// content of the file. This method should only be called for tracked files.
+ void signatureHelp(PathRef File, std::string Contents, Position Pos,
+ Callback<SignatureHelp> CB);
/// Get definition of symbol at a specified \p Line and \p Column in \p File.
void findDefinitions(PathRef File, Position Pos,
@@ -206,12 +199,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.
@@ -240,14 +227,26 @@
formatCode(llvm::StringRef Code, PathRef File,
ArrayRef<tooling::Range> Ranges);
- void scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
+ typedef uint64_t DocVersion;
+
+ /// Document contents with a version of this content.
+ struct VersionedContents {
+ DocVersion Version;
+ std::string Contents;
+ };
+
+ void scheduleReparseAndDiags(PathRef File, VersionedContents Contents,
WantDiagnostics WD,
IntrusiveRefCntPtr<vfs::FileSystem> FS);
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
@@ -115,51 +115,41 @@
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);
- scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
+ DocVersion Version = ++InternalVersion[File];
+ scheduleReparseAndDiags(File, VersionedContents{Version, std::move(Contents)},
WantDiags, FSProvider.getFileSystem());
}
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);
- scheduleReparseAndDiags(File, std::move(FileContents), WantDiagnostics::Yes,
- FSProvider.getFileSystem());
+ addDocument(File, std::move(Contents), WantDiagnostics::Yes);
}
-void ClangdServer::codeComplete(PathRef File, Position Pos,
+void ClangdServer::codeComplete(PathRef File, std::string Contents,
+ Position Pos,
const clangd::CodeCompleteOptions &Opts,
Callback<CompletionList> CB) {
// Copy completion options for passing them to async task handler.
auto CodeCompleteOpts = Opts;
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- VersionedDraft Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "codeComplete called for non-added document",
- llvm::errc::invalid_argument));
-
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
auto Task = [PCHs, Pos, FS, CodeCompleteOpts](
- std::string Contents, Path File, Callback<CompletionList> CB,
+ Path File, std::string Contents, Callback<CompletionList> CB,
llvm::Expected<InputsAndPreamble> IP) {
assert(IP && "error when trying to read preamble for codeComplete");
auto PreambleData = IP->Preamble;
@@ -175,20 +165,15 @@
WorkScheduler.runWithPreamble(
"CodeComplete", File,
- Bind(Task, std::move(*Latest.Draft), File.str(), std::move(CB)));
+ Bind(Task, File.str(), std::move(Contents), std::move(CB)));
}
-void ClangdServer::signatureHelp(PathRef File, Position Pos,
- Callback<SignatureHelp> CB) {
- VersionedDraft Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "signatureHelp is called for non-added document",
- llvm::errc::invalid_argument));
+void ClangdServer::signatureHelp(PathRef File, std::string Contents,
+ Position Pos, Callback<SignatureHelp> CB) {
auto PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
- auto Action = [Pos, FS, PCHs](std::string Contents, Path File,
+ auto Action = [Pos, FS, PCHs](Path File, std::string Contents,
Callback<SignatureHelp> CB,
llvm::Expected<InputsAndPreamble> IP) {
if (!IP)
@@ -203,7 +188,7 @@
WorkScheduler.runWithPreamble(
"SignatureHelp", File,
- Bind(Action, std::move(*Latest.Draft), File.str(), std::move(CB)));
+ Bind(Action, File.str(), std::move(Contents), std::move(CB)));
}
llvm::Expected<tooling::Replacements>
@@ -342,13 +327,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,
@@ -464,11 +442,6 @@
void ClangdServer::findDocumentHighlights(
PathRef File, Position Pos, Callback<std::vector<DocumentHighlight>> CB) {
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "findDocumentHighlights called on non-added file",
- llvm::errc::invalid_argument));
auto FS = FSProvider.getFileSystem();
auto Action = [FS, Pos](Callback<std::vector<DocumentHighlight>> CB,
@@ -482,12 +455,6 @@
}
void ClangdServer::findHover(PathRef File, Position Pos, Callback<Hover> CB) {
- Hover FinalHover;
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "findHover called on non-added file", llvm::errc::invalid_argument));
-
auto FS = FSProvider.getFileSystem();
auto Action = [Pos, FS](Callback<Hover> CB,
llvm::Expected<InputsAndAST> InpAST) {
@@ -500,7 +467,7 @@
}
void ClangdServer::scheduleReparseAndDiags(
- PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags,
+ PathRef File, VersionedContents Contents, WantDiagnostics WantDiags,
IntrusiveRefCntPtr<vfs::FileSystem> FS) {
tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
@@ -525,15 +492,10 @@
WorkScheduler.update(File,
ParseInputs{std::move(Command), std::move(FS),
- 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
@@ -74,6 +74,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.
@@ -100,6 +111,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,7 +340,13 @@
}
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,
[](llvm::Expected<CompletionList> List) {
if (!List)
return replyError(ErrorCode::InvalidParams,
@@ -339,14 +356,22 @@
}
void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
- Server.signatureHelp(Params.textDocument.uri.file(), Params.position,
- [](llvm::Expected<SignatureHelp> SignatureHelp) {
- if (!SignatureHelp)
- return replyError(
- ErrorCode::InvalidParams,
- llvm::toString(SignatureHelp.takeError()));
- reply(*SignatureHelp);
- });
+ PathRef File = Params.textDocument.uri.file();
+
+ auto Callback = [](llvm::Expected<SignatureHelp> SignatureHelp) {
+ if (!SignatureHelp)
+ return replyError(ErrorCode::InvalidParams,
+ llvm::toString(SignatureHelp.takeError()));
+ reply(*SignatureHelp);
+ };
+
+ 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) {
@@ -397,7 +422,7 @@
// Compilation database change.
if (Settings.compilationDatabasePath.hasValue()) {
CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
- Server.reparseOpenedFiles();
+ reparseOpenedFiles();
}
}
@@ -479,3 +504,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
