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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
