[clang] be0fa60 - [clang][AST] Fix generation with multiple external sources (#219189)

[via cfe-commits]

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);
+}