Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -177,15 +177,16 @@ auto PCHs = this->PCHs; auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, PCHs](Path File, Callback CB, - llvm::Expected IP) { + auto *Index = this->Index; + auto Action = [Pos, FS, PCHs, Index](Path File, Callback CB, + llvm::Expected IP) { if (!IP) return CB(IP.takeError()); auto PreambleData = IP->Preamble; CB(clangd::signatureHelp(File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, - IP->Contents, Pos, FS, PCHs)); + IP->Contents, Pos, FS, PCHs, Index)); }; WorkScheduler.runWithPreamble("SignatureHelp", File, Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -171,12 +171,11 @@ CodeCompleteOptions Opts); /// Get signature help at a specified \p Pos in \p FileName. -SignatureHelp signatureHelp(PathRef FileName, - const tooling::CompileCommand &Command, - PrecompiledPreamble const *Preamble, - StringRef Contents, Position Pos, - IntrusiveRefCntPtr VFS, - std::shared_ptr PCHs); +SignatureHelp +signatureHelp(PathRef FileName, const tooling::CompileCommand &Command, + PrecompiledPreamble const *Preamble, StringRef Contents, + Position Pos, IntrusiveRefCntPtr VFS, + std::shared_ptr PCHs, SymbolIndex *Index); // For index-based completion, we only consider: // * symbols in namespaces or translation unit scopes (e.g. no class Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -687,18 +687,23 @@ llvm::unique_function ResultsCallback; }; -using ScoredSignature = - std::pair; +struct ScoredSignature { + // When set, requires documentation to be requested from the index with this + // ID. + llvm::Optional IDForDoc; + SignatureInformation Signature; + SignatureQualitySignals Quality; +}; class SignatureHelpCollector final : public CodeCompleteConsumer { - public: SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, - SignatureHelp &SigHelp) - : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false), + SymbolIndex *Index, SignatureHelp &SigHelp) + : CodeCompleteConsumer(CodeCompleteOpts, + /*OutputIsBinary=*/false), SigHelp(SigHelp), Allocator(std::make_shared()), - CCTUInfo(Allocator) {} + CCTUInfo(Allocator), Index(Index) {} void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg, OverloadCandidate *Candidates, @@ -726,46 +731,73 @@ const auto *CCS = Candidate.CreateSignatureString( CurrentArg, S, *Allocator, CCTUInfo, true); assert(CCS && "Expected the CodeCompletionString to be non-null"); - // FIXME: for headers, we need to get a comment from the index. ScoredSignatures.push_back(processOverloadCandidate( Candidate, *CCS, getParameterDocComment(S.getASTContext(), Candidate, CurrentArg, /*CommentsFromHeaders=*/false))); } - std::sort(ScoredSignatures.begin(), ScoredSignatures.end(), - [](const ScoredSignature &L, const ScoredSignature &R) { - // Ordering follows: - // - Less number of parameters is better. - // - Function is better than FunctionType which is better than - // Function Template. - // - High score is better. - // - Shorter signature is better. - // - Alphebatically smaller is better. - if (L.first.NumberOfParameters != R.first.NumberOfParameters) - return L.first.NumberOfParameters < - R.first.NumberOfParameters; - if (L.first.NumberOfOptionalParameters != - R.first.NumberOfOptionalParameters) - return L.first.NumberOfOptionalParameters < - R.first.NumberOfOptionalParameters; - if (L.first.Kind != R.first.Kind) { - using OC = CodeCompleteConsumer::OverloadCandidate; - switch (L.first.Kind) { - case OC::CK_Function: - return true; - case OC::CK_FunctionType: - return R.first.Kind != OC::CK_Function; - case OC::CK_FunctionTemplate: - return false; - } - llvm_unreachable("Unknown overload candidate type."); - } - if (L.second.label.size() != R.second.label.size()) - return L.second.label.size() < R.second.label.size(); - return L.second.label < R.second.label; - }); - for (const auto &SS : ScoredSignatures) - SigHelp.signatures.push_back(SS.second); + + // Sema does not load the docs from the preamble, so we need to fetch extra + // docs from the index instead. + llvm::DenseMap FetchedDocs; + if (Index) { + LookupRequest IndexRequest; + for (const auto &S : ScoredSignatures) { + if (!S.IDForDoc) + continue; + IndexRequest.IDs.insert(*S.IDForDoc); + } + Index->lookup(IndexRequest, [&](const Symbol &S) { + if (!S.Detail || S.Detail->Documentation.empty()) + return; + FetchedDocs[S.ID] = S.Detail->Documentation; + }); + log("SigHelp: requested docs for {0} symbols from the index, got {1} " + "symbols with non-empty docs in the response", + IndexRequest.IDs.size(), FetchedDocs.size()); + } + + std::sort( + ScoredSignatures.begin(), ScoredSignatures.end(), + [](const ScoredSignature &L, const ScoredSignature &R) { + // Ordering follows: + // - Less number of parameters is better. + // - Function is better than FunctionType which is better than + // Function Template. + // - High score is better. + // - Shorter signature is better. + // - Alphebatically smaller is better. + if (L.Quality.NumberOfParameters != R.Quality.NumberOfParameters) + return L.Quality.NumberOfParameters < R.Quality.NumberOfParameters; + if (L.Quality.NumberOfOptionalParameters != + R.Quality.NumberOfOptionalParameters) + return L.Quality.NumberOfOptionalParameters < + R.Quality.NumberOfOptionalParameters; + if (L.Quality.Kind != R.Quality.Kind) { + using OC = CodeCompleteConsumer::OverloadCandidate; + switch (L.Quality.Kind) { + case OC::CK_Function: + return true; + case OC::CK_FunctionType: + return R.Quality.Kind != OC::CK_Function; + case OC::CK_FunctionTemplate: + return false; + } + llvm_unreachable("Unknown overload candidate type."); + } + if (L.Signature.label.size() != R.Signature.label.size()) + return L.Signature.label.size() < R.Signature.label.size(); + return L.Signature.label < R.Signature.label; + }); + + for (auto &SS : ScoredSignatures) { + auto IndexDocIt = + SS.IDForDoc ? FetchedDocs.find(*SS.IDForDoc) : FetchedDocs.end(); + if (IndexDocIt != FetchedDocs.end()) + SS.Signature.documentation = IndexDocIt->second; + + SigHelp.signatures.push_back(std::move(SS.Signature)); + } } GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; } @@ -778,11 +810,11 @@ ScoredSignature processOverloadCandidate(const OverloadCandidate &Candidate, const CodeCompletionString &CCS, llvm::StringRef DocComment) const { - SignatureInformation Result; + SignatureInformation Signature; SignatureQualitySignals Signal; const char *ReturnType = nullptr; - Result.documentation = formatDocumentation(CCS, DocComment); + Signature.documentation = formatDocumentation(CCS, DocComment); Signal.Kind = Candidate.getKind(); for (const auto &Chunk : CCS) { @@ -801,10 +833,10 @@ // A piece of text that describes the parameter that corresponds to // the code-completion location within a function call, message send, // macro invocation, etc. - Result.label += Chunk.Text; + Signature.label += Chunk.Text; ParameterInformation Info; Info.label = Chunk.Text; - Result.parameters.push_back(std::move(Info)); + Signature.parameters.push_back(std::move(Info)); Signal.NumberOfParameters++; Signal.ContainsActiveParameter = true; break; @@ -813,29 +845,36 @@ // The rest of the parameters are defaulted/optional. assert(Chunk.Optional && "Expected the optional code completion string to be non-null."); - Result.label += - getOptionalParameters(*Chunk.Optional, Result.parameters, Signal); + Signature.label += getOptionalParameters(*Chunk.Optional, + Signature.parameters, Signal); break; } case CodeCompletionString::CK_VerticalSpace: break; default: - Result.label += Chunk.Text; + Signature.label += Chunk.Text; break; } } if (ReturnType) { - Result.label += " -> "; - Result.label += ReturnType; + Signature.label += " -> "; + Signature.label += ReturnType; } - dlog("Signal for {0}: {1}", Result, Signal); - return {Signal, Result}; + dlog("Signal for {0}: {1}", Signature, Signal); + ScoredSignature Result; + Result.Signature = std::move(Signature); + Result.Quality = Signal; + Result.IDForDoc = + Result.Signature.documentation.empty() && Candidate.getFunction() + ? clangd::getSymbolID(Candidate.getFunction()) + : llvm::None; + return Result; } SignatureHelp &SigHelp; std::shared_ptr Allocator; CodeCompletionTUInfo CCTUInfo; - + const SymbolIndex *Index; }; // SignatureHelpCollector struct SemaCompleteInput { @@ -1314,7 +1353,8 @@ PrecompiledPreamble const *Preamble, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, - std::shared_ptr PCHs) { + std::shared_ptr PCHs, + SymbolIndex *Index) { SignatureHelp Result; clang::CodeCompleteOptions Options; Options.IncludeGlobals = false; @@ -1322,10 +1362,11 @@ Options.IncludeCodePatterns = false; Options.IncludeBriefComments = false; IncludeStructure PreambleInclusions; // Unused for signatureHelp - semaCodeComplete(llvm::make_unique(Options, Result), - Options, - {FileName, Command, Preamble, Contents, Pos, std::move(VFS), - std::move(PCHs)}); + semaCodeComplete( + llvm::make_unique(Options, Index, Result), + Options, + {FileName, Command, Preamble, Contents, Pos, std::move(VFS), + std::move(PCHs)}); return Result; } Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -826,11 +826,19 @@ EXPECT_TRUE(Results.Completions.empty()); } -SignatureHelp signatures(StringRef Text) { +SignatureHelp signatures(StringRef Text, + std::vector IndexSymbols = {}) { + std::unique_ptr Index; + if (!IndexSymbols.empty()) + Index = memIndex(IndexSymbols); + MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdServer::Options Opts = ClangdServer::optsForTest(); + Opts.StaticIndex = Index.get(); + + ClangdServer Server(CDB, FS, DiagConsumer, Opts); auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); @@ -845,6 +853,7 @@ return false; return true; } +MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; } Matcher Sig(std::string Label, std::vector Params) { @@ -1590,6 +1599,51 @@ ElementsAre(Sig("foo(T, U) -> void", {"T", "U"}))); } +TEST(SignatureHelpTest, IndexDocumentation) { + Symbol::Details DocDetails; + DocDetails.Documentation = "Doc from the index"; + + Symbol Foo0 = sym("foo", index::SymbolKind::Function, "@F@\\0#"); + Foo0.Detail = &DocDetails; + Symbol Foo1 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#"); + Foo1.Detail = &DocDetails; + Symbol Foo2 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#I#"); + + EXPECT_THAT( + signatures(R"cpp( + int foo(); + int foo(double); + + void test() { + foo(^); + } + )cpp", + {Foo0}) + .signatures, + ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")), + AllOf(Sig("foo(double) -> int", {"double"}), SigDoc("")))); + + EXPECT_THAT( + signatures(R"cpp( + int foo(); + // Overriden doc from sema + int foo(int); + // Doc from sema + int foo(int, int); + + void test() { + foo(^); + } + )cpp", + {Foo0, Foo1, Foo2}) + .signatures, + ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")), + AllOf(Sig("foo(int) -> int", {"int"}), + SigDoc("Overriden doc from sema")), + AllOf(Sig("foo(int, int) -> int", {"int", "int"}), + SigDoc("Doc from sema")))); +} + } // namespace } // namespace clangd } // namespace clang