Author: Jonas Hahnfeld Date: 2026-09-07T13:23:37+02:00 New Revision: be0fa604519d749fdd6e3f3b943f85a9cbdb13bf
URL: https://github.com/llvm/llvm-project/commit/be0fa604519d749fdd6e3f3b943f85a9cbdb13bf DIFF: https://github.com/llvm/llvm-project/commit/be0fa604519d749fdd6e3f3b943f85a9cbdb13bf.diff LOG: [clang][AST] Fix generation with multiple external sources (#219189) ExternalASTSource::incrementGeneration returns the OldGeneration, which must be taken into account in case it is delegated to the topmost external source. This obsoletes a long-standing downstream patch in Cling that was previously submitted in https://reviews.llvm.org/D39714. Test proposed by Vassil Vassilev, assisted by Claude. Added: Modified: clang/lib/AST/ExternalASTSource.cpp clang/unittests/AST/ExternalASTSourceTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp index 118f6fd67d3a4..e2d57196ffe52 100644 --- a/clang/lib/AST/ExternalASTSource.cpp +++ b/clang/lib/AST/ExternalASTSource.cpp @@ -118,20 +118,21 @@ void ExternalASTSource::FindExternalLexicalDecls( void ExternalASTSource::getMemoryBufferSizes(MemoryBufferSizes &sizes) const {} uint32_t ExternalASTSource::incrementGeneration(ASTContext &C) { - uint32_t OldGeneration = CurrentGeneration; - // Make sure the generation of the topmost external source for the context is // incremented. That might not be us. auto *P = C.getExternalSource(); - if (P && P != this) + if (P && P != this) { + // The call itself returns the OldGeneration of the topmost external source. CurrentGeneration = P->incrementGeneration(C); - else { - // FIXME: Only bump the generation counter if the current generation number - // has been observed? - if (!++CurrentGeneration) - llvm::reportFatalUsageError("generation counter overflowed"); } + uint32_t OldGeneration = CurrentGeneration; + + // FIXME: Only bump the generation counter if the current generation number + // has been observed? + if (!++CurrentGeneration) + llvm::reportFatalUsageError("generation counter overflowed"); + return OldGeneration; } diff --git a/clang/unittests/AST/ExternalASTSourceTest.cpp b/clang/unittests/AST/ExternalASTSourceTest.cpp index f7ddebb7614d2..2cfc05bbfa14a 100644 --- a/clang/unittests/AST/ExternalASTSourceTest.cpp +++ b/clang/unittests/AST/ExternalASTSourceTest.cpp @@ -19,6 +19,7 @@ #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Lex/PreprocessorOptions.h" +#include "clang/Tooling/Tooling.h" #include "llvm/Support/VirtualFileSystem.h" #include "gtest/gtest.h" @@ -182,3 +183,45 @@ TEST(ExternalASTSourceTest, CompletesPatternInEitherOrder) { EXPECT_EQ(1u, Source->PartialCompletions) << Code; } } + +namespace { +// incrementGeneration is protected, so reaching it needs a source of our own. +struct BumpableSource : ExternalASTSource { + uint32_t bump(ASTContext &C) { return incrementGeneration(C); } +}; +} // namespace + +// Multiple external sources synchronize their generations, and the return +// values of incrementGeneration() and getGeneration() are as expected. This +// is relied upon by ASTReader and LazyGenerationalUpdatePtr. +TEST(ExternalASTSourceTest, IncrementGeneration) { + std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(""); + ASSERT_TRUE(AST != nullptr); + ASTContext &Ctx = AST->getASTContext(); + + auto Topmost = llvm::makeIntrusiveRefCnt<BumpableSource>(); + auto Secondary = llvm::makeIntrusiveRefCnt<BumpableSource>(); + Ctx.setExternalSource(Topmost); + ASSERT_EQ(Ctx.getExternalSource(), Topmost.get()); + + // The generation starts at the same value for the two sources. + ASSERT_EQ(Topmost->getGeneration(), 0u); + ASSERT_EQ(Secondary->getGeneration(), 0u); + + // Bumping the "secondary" source bumps the toplevel one, and leaves the two + // agreeing on the generation afterwards. + EXPECT_EQ(Secondary->bump(Ctx), 0u); + EXPECT_EQ(Secondary->getGeneration(), 1u); + EXPECT_EQ(Topmost->getGeneration(), 1u); + + // Bumping the topmost source increments its generation by one. + EXPECT_EQ(Topmost->bump(Ctx), 1u); + EXPECT_EQ(Topmost->getGeneration(), 2u); + // At this point, the "secondary" source is out-of-sync... + EXPECT_EQ(Secondary->getGeneration(), 1u); + + // Another bump synchronizes the two sources. + EXPECT_EQ(Secondary->bump(Ctx), 2u); + EXPECT_EQ(Secondary->getGeneration(), 3u); + EXPECT_EQ(Topmost->getGeneration(), 3u); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
