Skip to content

[clangd] Collect comments from function definitions into the index #67802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clang-tools-extra/clangd/index/Symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -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?
};

Expand Down
57 changes: 47 additions & 10 deletions clang-tools-extra/clangd/index/SymbolCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -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);
}

Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/index/SymbolCollector.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
52 changes: 52 additions & 0 deletions clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading