Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -44,10 +44,8 @@ /// Add macros to code completion results. bool IncludeMacros = true; - /// Add brief comments to completion items, if available. - /// FIXME(ibiryukov): it looks like turning this option on significantly slows - /// down completion, investigate if it can be made faster. - bool IncludeBriefComments = true; + /// Add comments to code completion results, if available. + bool IncludeComments = true; /// Include results that are not legal completions in the current context. /// For example, private members are usually inaccessible. Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -258,7 +258,8 @@ CompletionItem build(llvm::StringRef FileName, const CompletionItemScores &Scores, const CodeCompleteOptions &Opts, - CodeCompletionString *SemaCCS) const { + CodeCompletionString *SemaCCS, + llvm::StringRef SemaDocComment) const { assert(bool(SemaResult) == bool(SemaCCS)); CompletionItem I; if (SemaResult) { @@ -266,7 +267,7 @@ getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText, Opts.EnableSnippets); I.filterText = getFilterText(*SemaCCS); - I.documentation = getDocumentation(*SemaCCS); + I.documentation = formatDocumentation(*SemaCCS, SemaDocComment); I.detail = getDetail(*SemaCCS); } if (IndexResult) { @@ -505,17 +506,17 @@ case CodeCompletionResult::RK_Pattern: return Result.Pattern->getTypedText(); } - auto *CCS = codeCompletionString(Result, /*IncludeBriefComments=*/false); + auto *CCS = codeCompletionString(Result); return CCS->getTypedText(); } // Build a CodeCompletion string for R, which must be from Results. // The CCS will be owned by this recorder. - CodeCompletionString *codeCompletionString(const CodeCompletionResult &R, - bool IncludeBriefComments) { + CodeCompletionString *codeCompletionString(const CodeCompletionResult &R) { // CodeCompletionResult doesn't seem to be const-correct. We own it, anyway. return const_cast(R).CreateCodeCompletionString( - *CCSema, CCContext, *CCAllocator, CCTUInfo, IncludeBriefComments); + *CCSema, CCContext, *CCAllocator, CCTUInfo, + /*IncludeBriefComments=*/false); } private: @@ -597,7 +598,9 @@ const auto *CCS = Candidate.CreateSignatureString( CurrentArg, S, *Allocator, CCTUInfo, true); assert(CCS && "Expected the CodeCompletionString to be non-null"); - SigHelp.signatures.push_back(ProcessOverloadCandidate(Candidate, *CCS)); + SigHelp.signatures.push_back(ProcessOverloadCandidate( + Candidate, *CCS, + getParameterDocComment(S.getASTContext(), Candidate, CurrentArg))); } } @@ -610,11 +613,12 @@ // CompletionString.h. SignatureInformation ProcessOverloadCandidate(const OverloadCandidate &Candidate, - const CodeCompletionString &CCS) const { + const CodeCompletionString &CCS, + llvm::StringRef DocComment) const { SignatureInformation Result; const char *ReturnType = nullptr; - Result.documentation = getDocumentation(CCS); + Result.documentation = formatDocumentation(CCS, DocComment); for (const auto &Chunk : CCS) { switch (Chunk.Kind) { @@ -838,7 +842,11 @@ Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns; Result.IncludeMacros = IncludeMacros; Result.IncludeGlobals = true; - Result.IncludeBriefComments = IncludeBriefComments; + // We choose to include full comments and not do doxygen parsing in + // completion. + // FIXME: ideally, we should support doxygen in some form, e.g. do markdown + // formatting of the comments. + Result.IncludeBriefComments = false; // When an is used, Sema is responsible for completing the main file, // the index can provide results from the preamble. @@ -1030,9 +1038,15 @@ CompletionItem toCompletionItem(const CompletionCandidate &Candidate, const CompletionItemScores &Scores) { CodeCompletionString *SemaCCS = nullptr; - if (auto *SR = Candidate.SemaResult) - SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments); - return Candidate.build(FileName, Scores, Opts, SemaCCS); + std::string DocComment; + if (auto *SR = Candidate.SemaResult) { + SemaCCS = Recorder->codeCompletionString(*SR); + if (Opts.IncludeComments) { + assert(Recorder->CCSema); + DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR); + } + } + return Candidate.build(FileName, Scores, Opts, SemaCCS, DocComment); } }; @@ -1058,7 +1072,7 @@ Options.IncludeGlobals = false; Options.IncludeMacros = false; Options.IncludeCodePatterns = false; - Options.IncludeBriefComments = true; + Options.IncludeBriefComments = false; semaCodeComplete(llvm::make_unique(Options, Result), Options, {FileName, Command, Preamble, Contents, Pos, std::move(VFS), Index: clangd/CodeCompletionStrings.h =================================================================== --- clangd/CodeCompletionStrings.h +++ clangd/CodeCompletionStrings.h @@ -17,8 +17,27 @@ #include "clang/Sema/CodeCompleteConsumer.h" namespace clang { +class ASTContext; + namespace clangd { +/// Gets a minimally formatted documentation comment of \p Result, with comment +/// markers stripped. See clang::RawComment::getFormattedText() for the detailed +/// explanation of how the comment text is transformed. +/// Returns empty string when no comment is available. +std::string getDocComment(const ASTContext &Ctx, + const CodeCompletionResult &Result); + +/// Gets a minimally formatted documentation for parameter of \p Result, +/// corresponding to argument number \p ArgIndex. +/// This currently looks for comments attached to the parameter itself, and +/// doesn't extract them from function documentation. +/// Returns empty string when no comment is available. +std::string +getParameterDocComment(const ASTContext &Ctx, + const CodeCompleteConsumer::OverloadCandidate &Result, + unsigned ArgIndex); + /// Gets label and insert text for a completion item. For example, for function /// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled. /// @@ -27,9 +46,13 @@ void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label, std::string *InsertText, bool EnableSnippets); -/// Gets the documentation for a completion item. For example, comment for the -/// a class declaration. -std::string getDocumentation(const CodeCompletionString &CCS); +/// Assembles formatted documentation for a completion result. This includes +/// documentation comments and other relevant information like annotations. +/// +/// \param DocComment is a documentation comment for the original declaration, +/// it should be obtained via getDocComment or getParameterDocComment. +std::string formatDocumentation(const CodeCompletionString &CCS, + llvm::StringRef DocComment); /// Gets detail to be used as the detail field in an LSP completion item. This /// is usually the return type of a function. Index: clangd/CodeCompletionStrings.cpp =================================================================== --- clangd/CodeCompletionStrings.cpp +++ clangd/CodeCompletionStrings.cpp @@ -8,6 +8,8 @@ //===---------------------------------------------------------------------===// #include "CodeCompletionStrings.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RawCommentList.h" #include namespace clang { @@ -122,13 +124,43 @@ } // namespace +std::string getDocComment(const ASTContext &Ctx, + const CodeCompletionResult &Result) { + // FIXME: clang's completion also returns documentation for RK_Pattern if they + // contain a pattern for ObjC properties. Unfortunately, there is no API to + // get this declaration, so we don't show documentation in that case. + if (Result.Kind != CodeCompletionResult::RK_Declaration) + return ""; + auto Decl = Result.getDeclaration(); + if (!Decl) + return ""; + const RawComment *RC = getCompletionComment(Ctx, Decl); + if (!RC) + return ""; + return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics()); +} + +std::string +getParameterDocComment(const ASTContext &Ctx, + const CodeCompleteConsumer::OverloadCandidate &Result, + unsigned ArgIndex) { + auto Func = Result.getFunction(); + if (!Func) + return ""; + const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex); + if (!RC) + return ""; + return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics()); +} + void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label, std::string *InsertText, bool EnableSnippets) { return EnableSnippets ? processSnippetChunks(CCS, Label, InsertText) : processPlainTextChunks(CCS, Label, InsertText); } -std::string getDocumentation(const CodeCompletionString &CCS) { +std::string formatDocumentation(const CodeCompletionString &CCS, + llvm::StringRef DocComment) { // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this // information in the documentation field. std::string Result; @@ -146,13 +178,13 @@ } } // Add brief documentation (if there is any). - if (CCS.getBriefComment() != nullptr) { + if (!DocComment.empty()) { if (!Result.empty()) { // This means we previously added annotations. Add an extra newline // character to make the annotations stand out. Result.push_back('\n'); } - Result += CCS.getBriefComment(); + Result += DocComment; } return Result; } Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -296,7 +296,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID) { - auto &SM = ND.getASTContext().getSourceManager(); + auto &Ctx = ND.getASTContext(); + auto &SM = Ctx.getSourceManager(); std::string QName; llvm::raw_string_ostream OS(QName); @@ -327,7 +328,7 @@ const auto *CCS = SymbolCompletion.CreateCodeCompletionString( *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator, *CompletionTUInfo, - /*IncludeBriefComments*/ true); + /*IncludeBriefComments*/ false); std::string Label; std::string SnippetInsertText; std::string IgnoredLabel; @@ -337,7 +338,8 @@ getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText, /*EnableSnippets=*/false); std::string FilterText = getFilterText(*CCS); - std::string Documentation = getDocumentation(*CCS); + std::string Documentation = + formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion)); std::string CompletionDetail = getDetail(*CCS); std::string Include; Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -243,8 +243,7 @@ // completion. At least there aren't any we're aware of. EXPECT_THAT(Results.items, Not(Contains(Kind(CompletionItemKind::Snippet)))); // Check documentation. - EXPECT_IFF(Opts.IncludeBriefComments, Results.items, - Contains(IsDocumented())); + EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented())); } void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) { @@ -289,8 +288,7 @@ AllOf(Has("local_var"), Has("LocalClass"), Contains(Kind(CompletionItemKind::Snippet)))); // Check documentation. - EXPECT_IFF(Opts.IncludeBriefComments, Results.items, - Contains(IsDocumented())); + EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented())); } TEST(CompletionTest, CompletionOptions) { @@ -301,7 +299,7 @@ // We used to test every combination of options, but that got too slow (2^N). auto Flags = { &clangd::CodeCompleteOptions::IncludeMacros, - &clangd::CodeCompleteOptions::IncludeBriefComments, + &clangd::CodeCompleteOptions::IncludeComments, &clangd::CodeCompleteOptions::EnableSnippets, &clangd::CodeCompleteOptions::IncludeCodePatterns, &clangd::CodeCompleteOptions::IncludeIneligibleResults, Index: unittests/clangd/CodeCompletionStringsTests.cpp =================================================================== --- unittests/clangd/CodeCompletionStringsTests.cpp +++ unittests/clangd/CodeCompletionStringsTests.cpp @@ -51,14 +51,15 @@ } TEST_F(CompletionStringTest, Documentation) { - Builder.addBriefComment("Is this brief?"); - EXPECT_EQ(getDocumentation(*Builder.TakeString()), "Is this brief?"); + Builder.addBriefComment("This is ignored"); + EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"), + "Is this brief?"); } TEST_F(CompletionStringTest, DocumentationWithAnnotation) { - Builder.addBriefComment("Is this brief?"); + Builder.addBriefComment("This is ignored"); Builder.AddAnnotation("Ano"); - EXPECT_EQ(getDocumentation(*Builder.TakeString()), + EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"), "Annotation: Ano\n\nIs this brief?"); } @@ -67,7 +68,7 @@ Builder.AddAnnotation("Ano2"); Builder.AddAnnotation("Ano3"); - EXPECT_EQ(getDocumentation(*Builder.TakeString()), + EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""), "Annotations: Ano1 Ano2 Ano3\n"); } @@ -97,7 +98,7 @@ TEST_F(CompletionStringTest, FunctionSnippet) { Builder.AddResultTypeChunk("result no no"); - Builder.addBriefComment("Foo's comment"); + Builder.addBriefComment("This comment is ignored"); Builder.AddTypedTextChunk("Foo"); Builder.AddChunk(CodeCompletionString::CK_LeftParen); Builder.AddPlaceholderChunk("p1"); @@ -113,7 +114,7 @@ labelAndInsertText(*CCS, /*EnableSnippets=*/true); EXPECT_EQ(Label, "Foo(p1, p2)"); EXPECT_EQ(InsertText, "Foo(${1:p1}, ${2:p2})"); - EXPECT_EQ(getDocumentation(*CCS), "Foo's comment"); + EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment"); EXPECT_EQ(getFilterText(*CCS), "Foo"); } Index: unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -490,11 +490,11 @@ } )"; runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre(QName("nx"), - AllOf(QName("nx::ff"), - Labeled("ff(int x, double y)"), - Detail("int"), Doc("Foo comment.")))); + EXPECT_THAT( + Symbols, + UnorderedElementsAre( + QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), + Detail("int"), Doc("Foo comment.")))); } TEST_F(SymbolCollectorTest, PlainAndSnippet) {