[clang] [AST] Iterate redecls starting from the canonical one in getRawCommentsForAnyRedecl() (PR #108475)

[Nathan Ridge via cfe-commits]

https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/108475

>From 408259c2e28e4664f0d0c47a6a897c6eb5660f93 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Tue, 10 Sep 2024 22:34:55 -0400
Subject: [PATCH] [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.
---
 clang/lib/AST/ASTContext.cpp                  |  2 +-
 clang/lib/Sema/SemaTemplate.cpp               |  5 +
 clang/unittests/AST/CMakeLists.txt            |  1 +
 clang/unittests/AST/RawCommentForDeclTest.cpp | 99 +++++++++++++++++++
 4 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 clang/unittests/AST/RawCommentForDeclTest.cpp

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a4e6d3b108c8a5..3735534ef3d3f1 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -444,7 +444,7 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(
     return CommentlessRedeclChains.lookup(CanonicalD);
   }();
 
-  for (const auto Redecl : D->redecls()) {
+  for (const auto Redecl : CanonicalD->redecls()) {
     assert(Redecl);
     // Skip all redeclarations that have been checked previously.
     if (LastCheckedRedecl) {
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a032e3ec6f6353..9b354052fab4b5 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10455,6 +10455,11 @@ bool Sema::CheckFunctionTemplateSpecialization(
   // Mark the prior declaration as an explicit specialization, so that later
   // clients know that this is an explicit specialization.
   if (!isFriend) {
+    SmallVector<TemplateParameterList *, 4> TPL;
+    for (unsigned I = 0; I < FD->getNumTemplateParameterLists(); ++I)
+      TPL.push_back(FD->getTemplateParameterList(I));
+    Specialization->setTemplateParameterListsInfo(Context, TPL);
+
     // Since explicit specializations do not inherit '=delete' from their
     // primary function template - check if the 'specialization' that was
     // implicitly generated (during template argument deduction for partial
diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index dcc9bc0f39ac2c..79ad8a28f2b33c 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -33,6 +33,7 @@ add_clang_unittest(ASTTests
   NamedDeclPrinterTest.cpp
   ProfilingTest.cpp
   RandstructTest.cpp
+  RawCommentForDeclTest.cpp
   RecursiveASTVisitorTest.cpp
   SizelessTypesTest.cpp
   SourceLocationTest.cpp
diff --git a/clang/unittests/AST/RawCommentForDeclTest.cpp b/clang/unittests/AST/RawCommentForDeclTest.cpp
new file mode 100644
index 00000000000000..b811df28127d43
--- /dev/null
+++ b/clang/unittests/AST/RawCommentForDeclTest.cpp
@@ -0,0 +1,99 @@
+//===- unittests/AST/RawCommentForDeclTestTest.cpp
+//-------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "gmock/gmock-matchers.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+
+struct FoundComment {
+  std::string DeclName;
+  bool IsDefinition;
+  std::string Comment;
+
+  bool operator==(const FoundComment &RHS) const {
+    return DeclName == RHS.DeclName && IsDefinition == RHS.IsDefinition &&
+           Comment == RHS.Comment;
+  }
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+                                       const FoundComment &C) {
+    return Stream << "{Name: " << C.DeclName << ", Def: " << C.IsDefinition
+                  << ", Comment: " << C.Comment << "}";
+  }
+};
+
+class CollectCommentsAction : public ASTFrontendAction {
+public:
+  CollectCommentsAction(std::vector<FoundComment> &Comments)
+      : Comments(Comments) {}
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 llvm::StringRef) override {
+    CI.getLangOpts().CommentOpts.ParseAllComments = true;
+    return std::make_unique<Consumer>(*this);
+  }
+
+  std::vector<FoundComment> &Comments;
+
+private:
+  class Consumer : public clang::ASTConsumer {
+  private:
+    CollectCommentsAction &Action;
+
+  public:
+    Consumer(CollectCommentsAction &Action) : Action(Action) {}
+    ~Consumer() override {}
+
+    bool HandleTopLevelDecl(DeclGroupRef DG) override {
+      for (Decl *D : DG) {
+        if (NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
+          auto &Ctx = D->getASTContext();
+          const auto *RC = Ctx.getRawCommentForAnyRedecl(D);
+          Action.Comments.push_back(FoundComment{
+              ND->getNameAsString(), IsDefinition(D),
+              RC ? RC->getRawText(Ctx.getSourceManager()).str() : ""});
+        }
+      }
+
+      return true;
+    }
+
+    static bool IsDefinition(const Decl *D) {
+      if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+        return FD->isThisDeclarationADefinition();
+      }
+      if (const TagDecl *TD = dyn_cast<TagDecl>(D)) {
+        return TD->isThisDeclarationADefinition();
+      }
+      return false;
+    }
+  };
+};
+
+TEST(RawCommentForDecl, DefinitionComment) {
+  std::vector<FoundComment> Comments;
+  auto Action = std::make_unique<CollectCommentsAction>(Comments);
+  ASSERT_TRUE(tooling::runToolOnCode(std::move(Action), R"cpp(
+    void f();
+
+    // f is the best
+    void f() {}
+  )cpp"));
+  EXPECT_THAT(Comments, testing::ElementsAre(
+                            FoundComment{"f", false, ""},
+                            FoundComment{"f", true, "// f is the best"}));
+}
+
+} // namespace clang