Skip to content

Commit 408259c

Browse files
[AST] Iterate redecls starting from the canonical one in getRawCommentsForAnyRedecl()
The intent of the `CommentlessRedeclChains` cache is that if new redecls have been parsed since the last time getRawCommentsForAnyRedecl() was called, only the new redecls are checked for comments. However, redecls are a circular list, and if iteration starts from the input decl `D`, that could be a new one, and we incorrectly skip it while traversing the list to `LastCheckedRedecl`. Starting the iteration from the first (canonical) decl makes the cache work as intended. The patch also plugs a hole in the modeling of explicit function template specializations where the FunctionDecl object created implicitly during template argument deduction for the explicit specialization does not store the specialization's template argument lists.
1 parent 99c43e3 commit 408259c

File tree

4 files changed

+106
-1
lines changed

4 files changed

+106
-1
lines changed

clang/lib/AST/ASTContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(
444444
return CommentlessRedeclChains.lookup(CanonicalD);
445445
}();
446446

447-
for (const auto Redecl : D->redecls()) {
447+
for (const auto Redecl : CanonicalD->redecls()) {
448448
assert(Redecl);
449449
// Skip all redeclarations that have been checked previously.
450450
if (LastCheckedRedecl) {

clang/lib/Sema/SemaTemplate.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10455,6 +10455,11 @@ bool Sema::CheckFunctionTemplateSpecialization(
1045510455
// Mark the prior declaration as an explicit specialization, so that later
1045610456
// clients know that this is an explicit specialization.
1045710457
if (!isFriend) {
10458+
SmallVector<TemplateParameterList *, 4> TPL;
10459+
for (unsigned I = 0; I < FD->getNumTemplateParameterLists(); ++I)
10460+
TPL.push_back(FD->getTemplateParameterList(I));
10461+
Specialization->setTemplateParameterListsInfo(Context, TPL);
10462+
1045810463
// Since explicit specializations do not inherit '=delete' from their
1045910464
// primary function template - check if the 'specialization' that was
1046010465
// implicitly generated (during template argument deduction for partial

clang/unittests/AST/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ add_clang_unittest(ASTTests
3333
NamedDeclPrinterTest.cpp
3434
ProfilingTest.cpp
3535
RandstructTest.cpp
36+
RawCommentForDeclTest.cpp
3637
RecursiveASTVisitorTest.cpp
3738
SizelessTypesTest.cpp
3839
SourceLocationTest.cpp
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
//===- unittests/AST/RawCommentForDeclTestTest.cpp
2+
//-------------------------===//
3+
//
4+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
5+
// See https://llvm.org/LICENSE.txt for license information.
6+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "clang/AST/ASTConsumer.h"
11+
#include "clang/AST/DeclGroup.h"
12+
#include "clang/Frontend/CompilerInstance.h"
13+
#include "clang/Frontend/FrontendAction.h"
14+
#include "clang/Tooling/Tooling.h"
15+
16+
#include "gmock/gmock-matchers.h"
17+
#include "gtest/gtest.h"
18+
19+
namespace clang {
20+
21+
struct FoundComment {
22+
std::string DeclName;
23+
bool IsDefinition;
24+
std::string Comment;
25+
26+
bool operator==(const FoundComment &RHS) const {
27+
return DeclName == RHS.DeclName && IsDefinition == RHS.IsDefinition &&
28+
Comment == RHS.Comment;
29+
}
30+
friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
31+
const FoundComment &C) {
32+
return Stream << "{Name: " << C.DeclName << ", Def: " << C.IsDefinition
33+
<< ", Comment: " << C.Comment << "}";
34+
}
35+
};
36+
37+
class CollectCommentsAction : public ASTFrontendAction {
38+
public:
39+
CollectCommentsAction(std::vector<FoundComment> &Comments)
40+
: Comments(Comments) {}
41+
42+
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
43+
llvm::StringRef) override {
44+
CI.getLangOpts().CommentOpts.ParseAllComments = true;
45+
return std::make_unique<Consumer>(*this);
46+
}
47+
48+
std::vector<FoundComment> &Comments;
49+
50+
private:
51+
class Consumer : public clang::ASTConsumer {
52+
private:
53+
CollectCommentsAction &Action;
54+
55+
public:
56+
Consumer(CollectCommentsAction &Action) : Action(Action) {}
57+
~Consumer() override {}
58+
59+
bool HandleTopLevelDecl(DeclGroupRef DG) override {
60+
for (Decl *D : DG) {
61+
if (NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
62+
auto &Ctx = D->getASTContext();
63+
const auto *RC = Ctx.getRawCommentForAnyRedecl(D);
64+
Action.Comments.push_back(FoundComment{
65+
ND->getNameAsString(), IsDefinition(D),
66+
RC ? RC->getRawText(Ctx.getSourceManager()).str() : ""});
67+
}
68+
}
69+
70+
return true;
71+
}
72+
73+
static bool IsDefinition(const Decl *D) {
74+
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
75+
return FD->isThisDeclarationADefinition();
76+
}
77+
if (const TagDecl *TD = dyn_cast<TagDecl>(D)) {
78+
return TD->isThisDeclarationADefinition();
79+
}
80+
return false;
81+
}
82+
};
83+
};
84+
85+
TEST(RawCommentForDecl, DefinitionComment) {
86+
std::vector<FoundComment> Comments;
87+
auto Action = std::make_unique<CollectCommentsAction>(Comments);
88+
ASSERT_TRUE(tooling::runToolOnCode(std::move(Action), R"cpp(
89+
void f();
90+
91+
// f is the best
92+
void f() {}
93+
)cpp"));
94+
EXPECT_THAT(Comments, testing::ElementsAre(
95+
FoundComment{"f", false, ""},
96+
FoundComment{"f", true, "// f is the best"}));
97+
}
98+
99+
} // namespace clang

0 commit comments

Comments
 (0)