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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

<details>
<summary>Changes</summary>

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.

Fixes https://github.com/llvm/llvm-project/issues/108145

---
Full diff: https://github.com/llvm/llvm-project/pull/108475.diff


3 Files Affected:

- (modified) clang/lib/AST/ASTContext.cpp (+1-1) 
- (modified) clang/unittests/AST/CMakeLists.txt (+1) 
- (added) clang/unittests/AST/RawCommentForDeclTest.cpp (+99) 


``````````diff
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/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

``````````

</details>


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