https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/108475
>From 1df905728da591bae0acf231e2d7c1f7492d43f3 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] Ensure getRawCommentsForAnyRedecl() does not miss any redecl with a comment The previous implementation had a bug where, if it was called on a Decl later in the redecl chain than `LastCheckedDecl`, it could incorrectly skip and overlook a Decl with a comment. The patch addresses this by only using `LastCheckedDecl` if the input Decl `D` is on the path from the first (canonical) Decl to `LastCheckedDecl`. An alternative that was considered was to start the iteration from the (canonical) Decl, however this ran into problems with the modelling of explicit template specializations in the AST where the canonical Decl can be unusual. With the current solution, if no Decls were checked yet, we prefer to check the input Decl over the canonical one. --- clang/lib/AST/ASTContext.cpp | 19 +++- clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/RawCommentForDeclTest.cpp | 99 +++++++++++++++++++ 3 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 clang/unittests/AST/RawCommentForDeclTest.cpp diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index a4e6d3b108c8a5..c9b9850dbd2b54 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -440,14 +440,23 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl( // Any redeclarations of D that we haven't checked for comments yet? // We can't use DenseMap::iterator directly since it'd get invalid. - auto LastCheckedRedecl = [this, CanonicalD]() -> const Decl * { - return CommentlessRedeclChains.lookup(CanonicalD); - }(); + const Decl *LastCheckedRedecl = CommentlessRedeclChains.lookup(CanonicalD); + bool CanUseCommentlessCache = false; + if (LastCheckedRedecl) { + for (auto *Redecl : CanonicalD->redecls()) { + if (Redecl == D) { + CanUseCommentlessCache = true; + break; + } + if (Redecl == LastCheckedRedecl) + break; + } + } - for (const auto Redecl : D->redecls()) { + for (const Decl *Redecl : D->redecls()) { assert(Redecl); // Skip all redeclarations that have been checked previously. - if (LastCheckedRedecl) { + if (CanUseCommentlessCache && LastCheckedRedecl) { if (LastCheckedRedecl == Redecl) { LastCheckedRedecl = nullptr; } 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