diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b8816d7b555e87..3f146cb9247a78 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -408,6 +408,8 @@ Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed a crash that occurred when dividing by zero in complex integer division. (#GH55390). +- Fixed a bug in ``ASTContext::getRawCommentForAnyRedecl()`` where the function could + sometimes incorrectly return null even if a comment was present. (#GH108145) Miscellaneous Bug Fixes ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index ebd4a41ee6367a..85b3984940ffc2 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -440,12 +440,26 @@ 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 = [&]() { + const Decl *LastChecked = CommentlessRedeclChains.lookup(CanonicalD); + bool CanUseCommentlessCache = false; + if (LastChecked) { + for (auto *Redecl : CanonicalD->redecls()) { + if (Redecl == D) { + CanUseCommentlessCache = true; + break; + } + if (Redecl == LastChecked) + break; + } + } + // FIXME: This could be improved so that even if CanUseCommentlessCache + // is false, once we've traversed past CanonicalD we still skip ahead + // LastChecked. + return CanUseCommentlessCache ? LastChecked : nullptr; }(); - for (const auto Redecl : D->redecls()) { + for (const Decl *Redecl : D->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 40d2e1ff77a601..bfa6082a6ffa4b 100644 --- a/clang/unittests/AST/CMakeLists.txt +++ b/clang/unittests/AST/CMakeLists.txt @@ -34,6 +34,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..c81e56bf7e6b0c --- /dev/null +++ b/clang/unittests/AST/RawCommentForDeclTest.cpp @@ -0,0 +1,98 @@ +//===- 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 &Comments) + : Comments(Comments) {} + + std::unique_ptr CreateASTConsumer(CompilerInstance &CI, + llvm::StringRef) override { + CI.getLangOpts().CommentOpts.ParseAllComments = true; + return std::make_unique(*this); + } + + std::vector &Comments; + +private: + class Consumer : public clang::ASTConsumer { + private: + CollectCommentsAction &Action; + + public: + Consumer(CollectCommentsAction &Action) : Action(Action) {} + + bool HandleTopLevelDecl(DeclGroupRef DG) override { + for (Decl *D : DG) { + if (NamedDecl *ND = dyn_cast(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(D)) { + return FD->isThisDeclarationADefinition(); + } + if (const TagDecl *TD = dyn_cast(D)) { + return TD->isThisDeclarationADefinition(); + } + return false; + } + }; +}; + +TEST(RawCommentForDecl, DefinitionComment) { + std::vector Comments; + auto Action = std::make_unique(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