https://github.com/ckandeler updated https://github.com/llvm/llvm-project/pull/67802
>From f441c0bd52add3d5b63bc0f19a3d38cb12477359 Mon Sep 17 00:00:00 2001 From: Christian Kandeler <christian.kande...@qt.io> Date: Fri, 29 Sep 2023 15:01:58 +0200 Subject: [PATCH] [clangd] Collect comments from function definitions into the index This is useful with projects that put their (doxygen) comments at the implementation site, rather than the header. --- clang-tools-extra/clangd/index/Symbol.h | 4 +- .../clangd/index/SymbolCollector.cpp | 57 +++++++++++++++---- .../clangd/index/SymbolCollector.h | 3 +- .../clangd/unittests/SymbolCollectorTests.cpp | 52 +++++++++++++++++ clang/lib/AST/ASTContext.cpp | 11 +++- 5 files changed, 114 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clangd/index/Symbol.h b/clang-tools-extra/clangd/index/Symbol.h index 1aa5265299231b..62c47ddfc5758d 100644 --- a/clang-tools-extra/clangd/index/Symbol.h +++ b/clang-tools-extra/clangd/index/Symbol.h @@ -145,9 +145,11 @@ struct Symbol { ImplementationDetail = 1 << 2, /// Symbol is visible to other files (not e.g. a static helper function). VisibleOutsideFile = 1 << 3, + /// Symbol has an attached documentation comment. + HasDocComment = 1 << 4 }; - SymbolFlag Flags = SymbolFlag::None; + /// FIXME: also add deprecation message and fixit? }; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 5c4e2150cf3123..a76894cf0855f3 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -635,17 +635,21 @@ bool SymbolCollector::handleDeclOccurrence( return true; const Symbol *BasicSymbol = Symbols.find(ID); - if (isPreferredDeclaration(*OriginalDecl, Roles)) + bool SkipDocCheckInDef = false; + if (isPreferredDeclaration(*OriginalDecl, Roles)) { // If OriginalDecl is preferred, replace/create the existing canonical // declaration (e.g. a class forward declaration). There should be at most // one duplicate as we expect to see only one preferred declaration per // TU, because in practice they are definitions. BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly); - else if (!BasicSymbol || DeclIsCanonical) + SkipDocCheckInDef = true; + } else if (!BasicSymbol || DeclIsCanonical) { BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly); + SkipDocCheckInDef = true; + } if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) - addDefinition(*OriginalDecl, *BasicSymbol); + addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef); return true; } @@ -1025,16 +1029,28 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, *ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator, *CompletionTUInfo, /*IncludeBriefComments*/ false); - std::string Documentation = - formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion, - /*CommentsFromHeaders=*/true)); + std::string DocComment; + std::string Documentation; + bool AlreadyHasDoc = S.Flags & Symbol::HasDocComment; + if (!AlreadyHasDoc) { + DocComment = getDocComment(Ctx, SymbolCompletion, + /*CommentsFromHeaders=*/true); + Documentation = formatDocumentation(*CCS, DocComment); + } + const auto UpdateDoc = [&] { + if (!AlreadyHasDoc) { + if (!DocComment.empty()) + S.Flags |= Symbol::HasDocComment; + S.Documentation = Documentation; + } + }; if (!(S.Flags & Symbol::IndexedForCodeCompletion)) { if (Opts.StoreAllDocumentation) - S.Documentation = Documentation; + UpdateDoc(); Symbols.insert(S); return Symbols.find(S.ID); } - S.Documentation = Documentation; + UpdateDoc(); std::string Signature; std::string SnippetSuffix; getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, @@ -1058,8 +1074,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, return Symbols.find(S.ID); } -void SymbolCollector::addDefinition(const NamedDecl &ND, - const Symbol &DeclSym) { +void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym, + bool SkipDocCheck) { if (DeclSym.Definition) return; const auto &SM = ND.getASTContext().getSourceManager(); @@ -1074,6 +1090,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND, Symbol S = DeclSym; // FIXME: use the result to filter out symbols. S.Definition = *DefLoc; + + std::string DocComment; + std::string Documentation; + if (!SkipDocCheck && !(S.Flags & Symbol::HasDocComment) && + (llvm::isa<FunctionDecl>(ND) || llvm::isa<CXXMethodDecl>(ND))) { + CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0); + const auto *CCS = SymbolCompletion.CreateCodeCompletionString( + *ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator, + *CompletionTUInfo, + /*IncludeBriefComments*/ false); + DocComment = getDocComment(ND.getASTContext(), SymbolCompletion, + /*CommentsFromHeaders=*/true); + if (!S.Documentation.empty()) + Documentation = S.Documentation.str() + '\n' + DocComment; + else + Documentation = formatDocumentation(*CCS, DocComment); + if (!DocComment.empty()) + S.Flags |= Symbol::HasDocComment; + S.Documentation = Documentation; + } + Symbols.insert(S); } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index 20116fca7c51e3..6ff7a0145ff874 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -161,7 +161,8 @@ class SymbolCollector : public index::IndexDataConsumer { private: const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); - void addDefinition(const NamedDecl &, const Symbol &DeclSymbol); + void addDefinition(const NamedDecl &, const Symbol &DeclSymbol, + bool SkipDocCheck); void processRelations(const NamedDecl &ND, const SymbolID &ID, ArrayRef<index::SymbolRelation> Relations); diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index 1e7a30e69fabe0..0666be95b6b9ee 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -1477,6 +1477,58 @@ TEST_F(SymbolCollectorTest, Documentation) { forCodeCompletion(false)))); } +TEST_F(SymbolCollectorTest, DocumentationInMain) { + const std::string Header = R"( + // doc Foo + class Foo { + void f(); + }; + )"; + const std::string Main = R"( + // doc f + void Foo::f() {} + )"; + CollectorOpts.StoreAllDocumentation = true; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(qName("Foo"), doc("doc Foo"), forCodeCompletion(true)), + AllOf(qName("Foo::f"), doc("doc f"), returnType(""), + forCodeCompletion(false)))); +} + +TEST_F(SymbolCollectorTest, DocumentationAtDeclThenDef) { + const std::string Header = R"( + class Foo { + // doc f decl + void f(); + }; + )"; + const std::string Main = R"( + // doc f def + void Foo::f() {} + )"; + CollectorOpts.StoreAllDocumentation = true; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(AllOf(qName("Foo")), + AllOf(qName("Foo::f"), doc("doc f decl")))); +} + +TEST_F(SymbolCollectorTest, DocumentationAtDefThenDecl) { + const std::string Header = R"( + // doc f def + void f() {} + + // doc f decl + void f(); + )"; + CollectorOpts.StoreAllDocumentation = true; + runSymbolCollector(Header, "" /*Main*/); + EXPECT_THAT(Symbols, + UnorderedElementsAre(AllOf(qName("f"), doc("doc f def")))); +} + TEST_F(SymbolCollectorTest, ClassMembers) { const std::string Header = R"( class Foo { diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index b201d201e1ea6a..7a7c64931a3c54 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -451,8 +451,17 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl( if (LastCheckedRedecl) { if (LastCheckedRedecl == Redecl) { LastCheckedRedecl = nullptr; + continue; + } + if (auto F = llvm::dyn_cast<FunctionDecl>(Redecl)) { + if (!F->isThisDeclarationADefinition()) + continue; + } else if (auto M = llvm::dyn_cast<CXXMethodDecl>(Redecl)) { + if (!M->isThisDeclarationADefinition()) + continue; + } else { + continue; } - continue; } const RawComment *RedeclComment = getRawCommentForDeclNoCache(Redecl); if (RedeclComment) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits