https://github.com/HighCommander4 created https://github.com/llvm/llvm-project/pull/108475
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 >From cd39b4bbdfa6d91af24ee040b066463d74049adb Mon Sep 17 00:00:00 2001 From: Nathan Ridge <zeratul...@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. --- clang/lib/AST/ASTContext.cpp | 2 +- clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/RawCommentForDeclTest.cpp | 99 +++++++++++++++++++ 3 files changed, 101 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/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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
