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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
