Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -180,23 +180,25 @@ } void ClangdLSPServer::onRename(RenameParams &Params) { - auto File = Params.textDocument.uri.file; - auto Code = Server.getDocument(File); + Path File = Params.textDocument.uri.file; + llvm::Optional Code = Server.getDocument(File); if (!Code) return replyError(ErrorCode::InvalidParams, "onRename called for non-added file"); - auto Replacements = Server.rename(File, Params.position, Params.newName); - if (!Replacements) { - replyError(ErrorCode::InternalError, - llvm::toString(Replacements.takeError())); - return; - } - - std::vector Edits = replacementsToEdits(*Code, *Replacements); - WorkspaceEdit WE; - WE.changes = {{Params.textDocument.uri.uri(), Edits}}; - reply(WE); + Server.rename( + File, Params.position, Params.newName, + [File, Code, + Params](llvm::Expected> Replacements) { + if (!Replacements) + return replyError(ErrorCode::InternalError, + llvm::toString(Replacements.takeError())); + + std::vector Edits = replacementsToEdits(*Code, *Replacements); + WorkspaceEdit WE; + WE.changes = {{Params.textDocument.uri.uri(), Edits}}; + reply(WE); + }); } void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams &Params) { @@ -280,21 +282,25 @@ } void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) { - auto SignatureHelp = - Server.signatureHelp(Params.textDocument.uri.file, Params.position); - if (!SignatureHelp) - return replyError(ErrorCode::InvalidParams, - llvm::toString(SignatureHelp.takeError())); - reply(SignatureHelp->Value); + Server.signatureHelp(Params.textDocument.uri.file, Params.position, + [](llvm::Expected> SignatureHelp) { + if (!SignatureHelp) + return replyError( + ErrorCode::InvalidParams, + llvm::toString(SignatureHelp.takeError())); + reply(SignatureHelp->Value); + }); } void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) { - auto Items = - Server.findDefinitions(Params.textDocument.uri.file, Params.position); - if (!Items) - return replyError(ErrorCode::InvalidParams, - llvm::toString(Items.takeError())); - reply(json::ary(Items->Value)); + Server.findDefinitions( + Params.textDocument.uri.file, Params.position, + [](llvm::Expected>> Items) { + if (!Items) + return replyError(ErrorCode::InvalidParams, + llvm::toString(Items.takeError())); + reply(json::ary(Items->Value)); + }); } void ClangdLSPServer::onSwitchSourceHeader(TextDocumentIdentifier &Params) { @@ -303,16 +309,14 @@ } void ClangdLSPServer::onDocumentHighlight(TextDocumentPositionParams &Params) { - auto Highlights = Server.findDocumentHighlights(Params.textDocument.uri.file, - Params.position); - - if (!Highlights) { - replyError(ErrorCode::InternalError, - llvm::toString(Highlights.takeError())); - return; - } - - reply(json::ary(Highlights->Value)); + Server.findDocumentHighlights( + Params.textDocument.uri.file, Params.position, + [](llvm::Expected>> Highlights) { + if (!Highlights) + return replyError(ErrorCode::InternalError, + llvm::toString(Highlights.takeError())); + reply(json::ary(Highlights->Value)); + }); } ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount, Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -189,22 +189,28 @@ /// will be used. If \p UsedFS is non-null, it will be overwritten by /// vfs::FileSystem used for signature help. This method should only be called /// for currently tracked files. - llvm::Expected> - signatureHelp(PathRef File, Position Pos, - llvm::Optional OverridenContents = llvm::None, - IntrusiveRefCntPtr *UsedFS = nullptr); + void signatureHelp( + PathRef File, Position Pos, + UniqueFunction>)> Callback, + llvm::Optional OverridenContents = llvm::None, + IntrusiveRefCntPtr *UsedFS = nullptr); /// Get definition of symbol at a specified \p Line and \p Column in \p File. - llvm::Expected>> findDefinitions(PathRef File, - Position Pos); + void findDefinitions( + PathRef File, Position Pos, + UniqueFunction>>)> + Callback); /// Helper function that returns a path to the corresponding source file when /// given a header file and vice versa. llvm::Optional switchSourceHeader(PathRef Path); /// Get document highlights for a given position. - llvm::Expected>> - findDocumentHighlights(PathRef File, Position Pos); + void findDocumentHighlights( + PathRef File, Position Pos, + UniqueFunction< + void(llvm::Expected>>)> + Callback); /// Run formatting for \p Rng inside \p File with content \p Code. llvm::Expected formatRange(StringRef Code, @@ -221,8 +227,9 @@ /// Rename all occurrences of the symbol at the \p Pos in \p File to /// \p NewName. - Expected> rename(PathRef File, Position Pos, - llvm::StringRef NewName); + void rename(PathRef File, Position Pos, llvm::StringRef NewName, + UniqueFunction>)> + Callback); /// Gets current document contents for \p File. Returns None if \p File is not /// currently tracked. @@ -233,7 +240,7 @@ /// 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. - std::string dumpAST(PathRef File); + void dumpAST(PathRef File, UniqueFunction Callback); /// Called when an event occurs for a watched file in the workspace. void onFileEvent(const DidChangeWatchedFilesParams &Params); Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -31,36 +31,6 @@ namespace { -// Issues an async read of AST and waits for results. -template -Ret blockingRunWithAST(TUScheduler &S, PathRef File, Func &&F) { - // Using shared_ptr to workaround MSVC bug. It requires future<> arguments to - // have default and copy ctor. - auto SharedPtrFunc = [&](llvm::Expected Arg) { - return std::make_shared(F(std::move(Arg))); - }; - std::packaged_task(llvm::Expected)> Task( - SharedPtrFunc); - auto Future = Task.get_future(); - S.runWithAST(File, std::move(Task)); - return std::move(*Future.get()); -} - -// Issues an async read of preamble and waits for results. -template -Ret blockingRunWithPreamble(TUScheduler &S, PathRef File, Func &&F) { - // Using shared_ptr to workaround MSVC bug. It requires future<> arguments to - // have default and copy ctor. - auto SharedPtrFunc = [&](llvm::Expected Arg) { - return std::make_shared(F(std::move(Arg))); - }; - std::packaged_task(llvm::Expected)> - Task(SharedPtrFunc); - auto Future = Task.get_future(); - S.runWithPreamble(File, std::move(Task)); - return std::move(*Future.get()); -} - void ignoreError(llvm::Error Err) { handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {}); } @@ -213,10 +183,11 @@ std::move(Callback))); } -llvm::Expected> -ClangdServer::signatureHelp(PathRef File, Position Pos, - llvm::Optional OverridenContents, - IntrusiveRefCntPtr *UsedFS) { +void ClangdServer::signatureHelp( + PathRef File, Position Pos, + UniqueFunction>)> Callback, + llvm::Optional OverridenContents, + IntrusiveRefCntPtr *UsedFS) { auto TaggedFS = FSProvider.getTaggedFileSystem(File); if (UsedFS) *UsedFS = TaggedFS.Value; @@ -227,27 +198,30 @@ } else { VersionedDraft Latest = DraftMgr.getDraft(File); if (!Latest.Draft) - return llvm::make_error( + return Callback(llvm::make_error( "signatureHelp is called for non-added document", - llvm::errc::invalid_argument); + llvm::errc::invalid_argument)); Contents = std::move(*Latest.Draft); } - auto Action = [=](llvm::Expected IP) - -> Expected> { + auto PCHs = this->PCHs; + auto Action = [Contents, Pos, TaggedFS, + PCHs](Path File, decltype(Callback) Callback, + llvm::Expected IP) { if (!IP) - return IP.takeError(); + return Callback(IP.takeError()); + auto PreambleData = IP->Preamble; auto &Command = IP->Inputs.CompileCommand; - - return make_tagged( + Callback(make_tagged( clangd::signatureHelp(File, Command, PreambleData ? &PreambleData->Preamble : nullptr, Contents, Pos, TaggedFS.Value, PCHs), - TaggedFS.Tag); + TaggedFS.Tag)); }; - return blockingRunWithPreamble>>(WorkScheduler, - File, Action); + + WorkScheduler.runWithPreamble( + File, BindWithForward(Action, File.str(), std::move(Callback))); } llvm::Expected @@ -276,12 +250,15 @@ return formatCode(Code, File, {tooling::Range(PreviousLBracePos, Len)}); } -Expected> -ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName) { - using RetType = Expected>; - auto Action = [=](Expected InpAST) -> RetType { +void ClangdServer::rename( + PathRef File, Position Pos, llvm::StringRef NewName, + UniqueFunction>)> + Callback) { + auto Action = [Pos](Path File, std::string NewName, + decltype(Callback) Callback, + Expected InpAST) { if (!InpAST) - return InpAST.takeError(); + return Callback(InpAST.takeError()); auto &AST = InpAST->AST; RefactoringResultCollector ResultCollector; @@ -289,23 +266,24 @@ const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); if (!FE) - return llvm::make_error( - "rename called for non-added document", llvm::errc::invalid_argument); + return Callback(llvm::make_error( + "rename called for non-added document", + llvm::errc::invalid_argument)); SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier(AST, Pos, FE); tooling::RefactoringRuleContext Context( AST.getASTContext().getSourceManager()); Context.setASTContext(AST.getASTContext()); auto Rename = clang::tooling::RenameOccurrences::initiate( - Context, SourceRange(SourceLocationBeg), NewName.str()); + Context, SourceRange(SourceLocationBeg), NewName); if (!Rename) - return Rename.takeError(); + return Callback(Rename.takeError()); Rename->invoke(ResultCollector, Context); assert(ResultCollector.Result.hasValue()); if (!ResultCollector.Result.getValue()) - return ResultCollector.Result->takeError(); + return Callback(ResultCollector.Result->takeError()); std::vector Replacements; for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) { @@ -324,9 +302,12 @@ Replacements.push_back(Rep); } } - return Replacements; + return Callback(Replacements); }; - return blockingRunWithAST(WorkScheduler, File, std::move(Action)); + + WorkScheduler.runWithAST( + File, + BindWithForward(Action, File.str(), NewName.str(), std::move(Callback))); } llvm::Optional ClangdServer::getDocument(PathRef File) { @@ -336,37 +317,40 @@ return std::move(*Latest.Draft); } -std::string ClangdServer::dumpAST(PathRef File) { - auto Action = [](llvm::Expected InpAST) -> std::string { +void ClangdServer::dumpAST(PathRef File, + UniqueFunction Callback) { + auto Action = [](decltype(Callback) Callback, + llvm::Expected InpAST) { if (!InpAST) { ignoreError(InpAST.takeError()); - return ""; + return Callback(""); } - std::string Result; llvm::raw_string_ostream ResultOS(Result); clangd::dumpAST(InpAST->AST, ResultOS); ResultOS.flush(); - return Result; + Callback(Result); }; - return blockingRunWithAST(WorkScheduler, File, - std::move(Action)); + + WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback))); } -llvm::Expected>> -ClangdServer::findDefinitions(PathRef File, Position Pos) { +void ClangdServer::findDefinitions( + PathRef File, Position Pos, + UniqueFunction>>)> + Callback) { auto TaggedFS = FSProvider.getTaggedFileSystem(File); - - using RetType = llvm::Expected>>; - auto Action = [=](llvm::Expected InpAST) -> RetType { + auto Action = [Pos, TaggedFS](decltype(Callback) Callback, + llvm::Expected InpAST) { if (!InpAST) - return InpAST.takeError(); + return Callback(InpAST.takeError()); auto Result = clangd::findDefinitions(InpAST->AST, Pos); - return make_tagged(std::move(Result), TaggedFS.Tag); + Callback(make_tagged(std::move(Result), TaggedFS.Tag)); }; - return blockingRunWithAST(WorkScheduler, File, Action); + + WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback))); } llvm::Optional ClangdServer::switchSourceHeader(PathRef Path) { @@ -442,24 +426,27 @@ } } -llvm::Expected>> -ClangdServer::findDocumentHighlights(PathRef File, Position Pos) { +void ClangdServer::findDocumentHighlights( + PathRef File, Position Pos, + UniqueFunction>>)> + Callback) { auto FileContents = DraftMgr.getDraft(File); if (!FileContents.Draft) - return llvm::make_error( + return Callback(llvm::make_error( "findDocumentHighlights called on non-added file", - llvm::errc::invalid_argument); + llvm::errc::invalid_argument)); auto TaggedFS = FSProvider.getTaggedFileSystem(File); - using RetType = llvm::Expected>>; - auto Action = [=](llvm::Expected InpAST) -> RetType { + auto Action = [TaggedFS, Pos](decltype(Callback) Callback, + llvm::Expected InpAST) { if (!InpAST) - return InpAST.takeError(); + return Callback(InpAST.takeError()); auto Result = clangd::findDocumentHighlights(InpAST->AST, Pos); - return make_tagged(std::move(Result), TaggedFS.Tag); + Callback(make_tagged(std::move(Result), TaggedFS.Tag)); }; - return blockingRunWithAST(WorkScheduler, File, Action); + + WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback))); } void ClangdServer::scheduleReparseAndDiags( Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -96,7 +96,7 @@ } std::string dumpASTWithoutMemoryLocs(ClangdServer &Server, PathRef File) { - auto DumpWithMemLocs = Server.dumpAST(File); + auto DumpWithMemLocs = runDumpAST(Server, File); return replacePtrsInDump(DumpWithMemLocs); } @@ -432,17 +432,17 @@ // Clang can't parse command args in that case, but we shouldn't crash. Server.addDocument(FooCpp, "int main() {}"); - EXPECT_EQ(Server.dumpAST(FooCpp), ""); - EXPECT_ERROR(Server.findDefinitions(FooCpp, Position())); - EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position())); - EXPECT_ERROR(Server.rename(FooCpp, Position(), "new_name")); + EXPECT_EQ(runDumpAST(Server, FooCpp), ""); + 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 = Server.signatureHelp(FooCpp, Position()); + auto SigHelp = runSignatureHelp(Server, FooCpp, Position()); ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error"; EXPECT_THAT(SigHelp->Value.signatures, IsEmpty()); } @@ -646,7 +646,7 @@ Pos.line = LineDist(RandGen); Pos.character = ColumnDist(RandGen); - ASSERT_TRUE(!!Server.findDefinitions(FilePaths[FileIndex], Pos)); + ASSERT_TRUE(!!runFindDefinitions(Server, FilePaths[FileIndex], Pos)); }; std::vector> AsyncRequests = { @@ -768,8 +768,8 @@ public: std::atomic Count = {0}; - NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) - : StartSecondReparse(std::move(StartSecondReparse)) {} + NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) + : StartSecondReparse(std::move(StartSecondReparse)) {} void onDiagnosticsReady(PathRef, Tagged>) override { Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -626,7 +626,7 @@ Annotations Test(Text); Server.addDocument(File, Test.code()); EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; - auto R = Server.signatureHelp(File, Test.point()); + auto R = runSignatureHelp(Server, File, Test.point()); assert(R); return R.get().Value; } Index: clang-tools-extra/trunk/unittests/clangd/SyncAPI.h =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SyncAPI.h +++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.h @@ -22,6 +22,22 @@ runCodeComplete(ClangdServer &Server, PathRef File, Position Pos, clangd::CodeCompleteOptions Opts, llvm::Optional OverridenContents = llvm::None); + +llvm::Expected> +runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos, + llvm::Optional OverridenContents = llvm::None); + +llvm::Expected>> +runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos); + +llvm::Expected>> +runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos); + +llvm::Expected> +runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName); + +std::string runDumpAST(ClangdServer &Server, PathRef File); + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp +++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp @@ -17,7 +17,9 @@ /// T Result; /// someAsyncFunc(Param1, Param2, /*Callback=*/capture(Result)); template struct CaptureProxy { - CaptureProxy(T &Target) : Target(&Target) {} + CaptureProxy(llvm::Optional &Target) : Target(&Target) { + assert(!Target.hasValue()); + } CaptureProxy(const CaptureProxy &) = delete; CaptureProxy &operator=(const CaptureProxy &) = delete; @@ -39,16 +41,17 @@ if (!Target) return; assert(Future.valid() && "conversion to callback was not called"); - *Target = Future.get(); + assert(!Target->hasValue()); + Target->emplace(Future.get()); } private: - T *Target; + llvm::Optional *Target; std::promise Promise; std::future Future; }; -template CaptureProxy capture(T &Target) { +template CaptureProxy capture(llvm::Optional &Target) { return CaptureProxy(Target); } } // namespace @@ -57,9 +60,44 @@ runCodeComplete(ClangdServer &Server, PathRef File, Position Pos, clangd::CodeCompleteOptions Opts, llvm::Optional OverridenContents) { - Tagged Result; + llvm::Optional> Result; Server.codeComplete(File, Pos, Opts, capture(Result), OverridenContents); - return Result; + return std::move(*Result); +} + +llvm::Expected> +runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos, + llvm::Optional OverridenContents) { + llvm::Optional>> Result; + Server.signatureHelp(File, Pos, capture(Result), OverridenContents); + return std::move(*Result); +} + +llvm::Expected>> +runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos) { + llvm::Optional>>> Result; + Server.findDefinitions(File, Pos, capture(Result)); + return std::move(*Result); +} + +llvm::Expected>> +runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos) { + llvm::Optional>>> Result; + Server.findDocumentHighlights(File, Pos, capture(Result)); + return std::move(*Result); +} + +llvm::Expected> +runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName) { + llvm::Optional>> Result; + Server.rename(File, Pos, NewName, capture(Result)); + return std::move(*Result); +} + +std::string runDumpAST(ClangdServer &Server, PathRef File) { + llvm::Optional Result; + Server.dumpAST(File, capture(Result)); + return std::move(*Result); } } // namespace clangd Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp @@ -10,6 +10,7 @@ #include "ClangdUnit.h" #include "Compiler.h" #include "Matchers.h" +#include "SyncAPI.h" #include "TestFS.h" #include "XRefs.h" #include "clang/Frontend/CompilerInvocation.h" @@ -249,7 +250,8 @@ FS.Files[FooCpp] = ""; Server.addDocument(FooCpp, SourceAnnotations.code()); - auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point()); + auto Locations = + runFindDefinitions(Server, FooCpp, SourceAnnotations.point()); EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error"; EXPECT_THAT(Locations->Value,