[clang-tools-extra] r319753 - [clangd] Set completion options per-request.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 5 02:42:57 PST 2017
Author: ibiryukov
Date: Tue Dec 5 02:42:57 2017
New Revision: 319753
URL: http://llvm.org/viewvc/llvm-project?rev=319753&view=rev
Log:
[clangd] Set completion options per-request.
Summary:
Previously, completion options were set per ClangdServer instance.
It will allow to change completion preferences during the lifetime
of a single ClangdServer instance.
Also rewrote ClangdCompletionTest.CompletionOptions to reuse single
ClangdServer instance, the test now runs 2x faster on my machine.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall, ioeric
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D40654
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Dec 5 02:42:57 2017
@@ -194,14 +194,15 @@ void ClangdLSPServer::onCodeAction(Ctx C
}
void ClangdLSPServer::onCompletion(Ctx C, TextDocumentPositionParams &Params) {
- auto List = Server
- .codeComplete(
- Params.textDocument.uri.file,
- Position{Params.position.line, Params.position.character})
- .get() // FIXME(ibiryukov): This could be made async if we
- // had an API that would allow to attach callbacks to
- // futures returned by ClangdServer.
- .Value;
+ auto List =
+ Server
+ .codeComplete(
+ Params.textDocument.uri.file,
+ Position{Params.position.line, Params.position.character}, CCOpts)
+ .get() // FIXME(ibiryukov): This could be made async if we
+ // had an API that would allow to attach callbacks to
+ // futures returned by ClangdServer.
+ .Value;
C.reply(List);
}
@@ -240,9 +241,9 @@ ClangdLSPServer::ClangdLSPServer(JSONOut
llvm::Optional<StringRef> ResourceDir,
llvm::Optional<Path> CompileCommandsDir)
: Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)),
- Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
- StorePreamblesInMemory, CCOpts,
- /*Logger=*/Out, ResourceDir) {}
+ CCOpts(CCOpts), Server(CDB, /*DiagConsumer=*/*this, FSProvider,
+ AsyncThreadsCount, StorePreamblesInMemory,
+ /*Logger=*/Out, ResourceDir) {}
bool ClangdLSPServer::run(std::istream &In) {
assert(!IsDone && "Run was called before");
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue Dec 5 02:42:57 2017
@@ -96,7 +96,8 @@ private:
// before ClangdServer.
DirectoryBasedGlobalCompilationDatabase CDB;
RealFileSystemProvider FSProvider;
-
+ /// Options used for code completion
+ clangd::CodeCompleteOptions CCOpts;
// Server must be the last member of the class to allow its destructor to exit
// the worker thread that may otherwise run an async callback on partially
// destructed instance of ClangdLSPServer.
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 5 02:42:57 2017
@@ -173,16 +173,14 @@ ClangdServer::ClangdServer(GlobalCompila
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider,
unsigned AsyncThreadsCount,
- bool StorePreamblesInMemory,
- const clangd::CodeCompleteOptions &CodeCompleteOpts,
- clangd::Logger &Logger,
+ bool StorePreamblesInMemory, clangd::Logger &Logger,
llvm::Optional<StringRef> ResourceDir)
: Logger(Logger), CDB(CDB), DiagConsumer(DiagConsumer),
FSProvider(FSProvider),
ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
PCHs(std::make_shared<PCHContainerOperations>()),
StorePreamblesInMemory(StorePreamblesInMemory),
- CodeCompleteOpts(CodeCompleteOpts), WorkScheduler(AsyncThreadsCount) {}
+ WorkScheduler(AsyncThreadsCount) {}
void ClangdServer::setRootPath(PathRef RootPath) {
std::string NewRootPath = llvm::sys::path::convert_to_slash(
@@ -226,6 +224,7 @@ std::future<void> ClangdServer::forceRep
std::future<Tagged<CompletionList>>
ClangdServer::codeComplete(PathRef File, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
llvm::Optional<StringRef> OverridenContents,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
using ResultType = Tagged<CompletionList>;
@@ -239,13 +238,14 @@ ClangdServer::codeComplete(PathRef File,
std::future<ResultType> ResultFuture = ResultPromise.get_future();
codeComplete(BindWithForward(Callback, std::move(ResultPromise)), File, Pos,
- OverridenContents, UsedFS);
+ Opts, OverridenContents, UsedFS);
return ResultFuture;
}
void ClangdServer::codeComplete(
UniqueFunction<void(Tagged<CompletionList>)> Callback, PathRef File,
- Position Pos, llvm::Optional<StringRef> OverridenContents,
+ Position Pos, const clangd::CodeCompleteOptions &Opts,
+ llvm::Optional<StringRef> OverridenContents,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
using CallbackType = UniqueFunction<void(Tagged<CompletionList>)>;
@@ -273,6 +273,8 @@ void ClangdServer::codeComplete(
// is reusable in completion more often.
std::shared_ptr<const PreambleData> Preamble =
Resources->getPossiblyStalePreamble();
+ // Copy completion options for passing them to async task handler.
+ auto CodeCompleteOpts = Opts;
// A task that will be run asynchronously.
auto Task =
// 'mutable' to reassign Preamble variable.
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 5 02:42:57 2017
@@ -211,7 +211,6 @@ public:
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
- const clangd::CodeCompleteOptions &CodeCompleteOpts,
clangd::Logger &Logger,
llvm::Optional<StringRef> ResourceDir = llvm::None);
@@ -255,6 +254,7 @@ public:
/// while returned future is not yet ready.
std::future<Tagged<CompletionList>>
codeComplete(PathRef File, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
llvm::Optional<StringRef> OverridenContents = llvm::None,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
@@ -262,6 +262,7 @@ public:
/// when codeComplete results become available.
void codeComplete(UniqueFunction<void(Tagged<CompletionList>)> Callback,
PathRef File, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
llvm::Optional<StringRef> OverridenContents = llvm::None,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
@@ -328,7 +329,6 @@ private:
llvm::Optional<std::string> RootPath;
std::shared_ptr<PCHContainerOperations> PCHs;
bool StorePreamblesInMemory;
- clangd::CodeCompleteOptions CodeCompleteOpts;
/// Used to serialize diagnostic callbacks.
/// FIXME(ibiryukov): get rid of an extra map and put all version counters
/// into CppFile.
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Tue Dec 5 02:42:57 2017
@@ -124,7 +124,6 @@ protected:
MockCompilationDatabase CDB;
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
for (const auto &FileWithContents : ExtraFiles)
FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
@@ -189,7 +188,6 @@ TEST_F(ClangdVFSTest, Reparse) {
MockCompilationDatabase CDB;
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
const auto SourceContents = R"cpp(
@@ -236,7 +234,6 @@ TEST_F(ClangdVFSTest, ReparseOnHeaderCha
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
const auto SourceContents = R"cpp(
@@ -286,7 +283,6 @@ TEST_F(ClangdVFSTest, CheckVersions) {
ClangdServer Server(CDB, DiagConsumer, FS,
/*AsyncThreadsCount=*/0,
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -294,17 +290,22 @@ TEST_F(ClangdVFSTest, CheckVersions) {
FS.Files[FooCpp] = SourceContents;
FS.ExpectedFile = FooCpp;
+ // Use default completion options.
+ clangd::CodeCompleteOptions CCOpts;
+
// No need to sync reparses, because requests are processed on the calling
// thread.
FS.Tag = "123";
Server.addDocument(FooCpp, SourceContents);
- EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
+ EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}, CCOpts).get().Tag,
+ FS.Tag);
EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
FS.Tag = "321";
Server.addDocument(FooCpp, SourceContents);
EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
- EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
+ EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}, CCOpts).get().Tag,
+ FS.Tag);
}
// Only enable this test on Unix
@@ -322,7 +323,6 @@ TEST_F(ClangdVFSTest, SearchLibDir) {
ClangdServer Server(CDB, DiagConsumer, FS,
/*AsyncThreadsCount=*/0,
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
// Just a random gcc version string
@@ -373,7 +373,6 @@ TEST_F(ClangdVFSTest, ForceReparseCompil
ClangdServer Server(CDB, DiagConsumer, FS,
/*AsyncThreadsCount=*/0,
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
// No need to sync reparses, because reparses are performed on the calling
// thread to true.
@@ -515,7 +514,6 @@ int d;
MockCompilationDatabase CDB;
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
// Prepare some random distributions for the test.
@@ -609,7 +607,10 @@ int d;
// requests as opposed to AddDocument/RemoveDocument, which are implicitly
// cancelled by any subsequent AddDocument/RemoveDocument request to the
// same file.
- Server.codeComplete(FilePaths[FileIndex], Pos).wait();
+ Server
+ .codeComplete(FilePaths[FileIndex], Pos,
+ clangd::CodeCompleteOptions())
+ .wait();
};
auto FindDefinitionsRequest = [&]() {
@@ -676,7 +677,6 @@ TEST_F(ClangdVFSTest, CheckSourceHeaderS
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
auto SourceContents = R"cpp(
@@ -803,7 +803,6 @@ int d;
MockCompilationDatabase CDB;
ClangdServer Server(CDB, DiagConsumer, FS, 4, /*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
Server.addDocument(FooCpp, SourceContentsWithErrors);
StartSecondReparse.wait();
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=319753&r1=319752&r2=319753&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue Dec 5 02:42:57 2017
@@ -75,7 +75,6 @@ TEST_F(ClangdCompletionTest, CheckConten
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- clangd::CodeCompleteOptions(),
EmptyLogger::getInstance());
auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -88,6 +87,9 @@ int b = ;
int cbc;
int b = ;
)cpp";
+
+ // Use default options.
+ CodeCompleteOptions CCOpts;
// Complete after '=' sign. We need to be careful to keep the SourceContents'
// size the same.
// We complete on the 3rd line (2nd in zero-based numbering), because raw
@@ -103,7 +105,7 @@ int b = ;
{
auto CodeCompletionResults1 =
- Server.codeComplete(FooCpp, CompletePos, None).get().Value;
+ Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value;
EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
}
@@ -111,7 +113,7 @@ int b = ;
{
auto CodeCompletionResultsOverriden =
Server
- .codeComplete(FooCpp, CompletePos,
+ .codeComplete(FooCpp, CompletePos, CCOpts,
StringRef(OverridenSourceContents))
.get()
.Value;
@@ -121,7 +123,7 @@ int b = ;
{
auto CodeCompletionResults2 =
- Server.codeComplete(FooCpp, CompletePos, None).get().Value;
+ Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value;
EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
}
@@ -132,10 +134,8 @@ TEST_F(ClangdCompletionTest, Limit) {
MockCompilationDatabase CDB;
CDB.ExtraClangFlags.push_back("-xc++");
IgnoreDiagnostics DiagConsumer;
- clangd::CodeCompleteOptions Opts;
- Opts.Limit = 2;
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, Opts,
+ /*StorePreamblesInMemory=*/true,
EmptyLogger::getInstance());
auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -152,9 +152,12 @@ int main() { ClassWithMembers().{complet
"complete");
Server.addDocument(FooCpp, Completion.Text);
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 2;
+
/// For after-dot completion we must always get consistent results.
auto Results = Server
- .codeComplete(FooCpp, Completion.MarkerPos,
+ .codeComplete(FooCpp, Completion.MarkerPos, Opts,
StringRef(Completion.Text))
.get()
.Value;
@@ -171,9 +174,8 @@ TEST_F(ClangdCompletionTest, Filter) {
MockCompilationDatabase CDB;
CDB.ExtraClangFlags.push_back("-xc++");
IgnoreDiagnostics DiagConsumer;
- clangd::CodeCompleteOptions Opts;
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, Opts,
+ /*StorePreamblesInMemory=*/true,
EmptyLogger::getInstance());
auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -194,7 +196,8 @@ TEST_F(ClangdCompletionTest, Filter) {
"complete");
Server.addDocument(FooCpp, Completion.Text);
return Server
- .codeComplete(FooCpp, Completion.MarkerPos, StringRef(Completion.Text))
+ .codeComplete(FooCpp, Completion.MarkerPos,
+ clangd::CodeCompleteOptions(), StringRef(Completion.Text))
.get()
.Value;
};
@@ -288,7 +291,7 @@ int test() {
auto TestWithOpts = [&](clangd::CodeCompleteOptions Opts) {
ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, Opts,
+ /*StorePreamblesInMemory=*/true,
EmptyLogger::getInstance());
// No need to sync reparses here as there are no asserts on diagnostics (or
// other async operations).
@@ -301,7 +304,7 @@ int test() {
/// For after-dot completion we must always get consistent results.
{
auto Results = Server
- .codeComplete(FooCpp, MemberCompletion.MarkerPos,
+ .codeComplete(FooCpp, MemberCompletion.MarkerPos, Opts,
StringRef(MemberCompletion.Text))
.get()
.Value;
@@ -337,7 +340,7 @@ int test() {
// Global completion differs based on the Opts that were passed.
{
auto Results = Server
- .codeComplete(FooCpp, GlobalCompletion.MarkerPos,
+ .codeComplete(FooCpp, GlobalCompletion.MarkerPos, Opts,
StringRef(GlobalCompletion.Text))
.get()
.Value;
More information about the cfe-commits
mailing list