[llvm-branch-commits] [clang] release/23.x: [clang-repl] Unlink the withdrawn partial translation unit on Undo (#213235) (PR #218423)

[via llvm-branch-commits]

https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/218423

Backport 77229133dabe

Requested by: @conrade-ctc

>From 2c3a44266533879ef750d45cae1cd109b16e4102 Mon Sep 17 00:00:00 2001
From: Emery Conrad <conrade.ctc at gmail.com>
Date: Tue, 18 Aug 2026 09:14:30 -0400
Subject: [PATCH] [clang-repl] Unlink the withdrawn partial translation unit on
 Undo (#213235)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`Interpreter::Undo()` cleared name lookup for a withdrawn input but left its `TranslationUnitDecl` in the redeclaration chain, so walking the chain still reached declarations the user had removed. `ASTContext::TUDecl` also still pointed at the withdrawn unit, so the next input chained onto it.

`IncrementalParser::CleanUpPTU` now rebuilds the redeclaration chain without the withdrawn unit and makes its predecessor current again, the way cling's `removeRedeclFromChain` does. This needs no new AST API — `Redeclarable` and `ASTContext` already declare `friend class IncrementalParser`. The declarations stay bump-allocated in the `ASTContext`; they just stop being reachable, which is what the removed `// FIXME: We should de-allocate MostRecentTU` asked for.

`CleanUpPTU` also runs on the parse-failure path, so a failed input no longer leaves its unit in the chain either.

Fixes #213230.

---------

Co-authored-by: Emery Conrad <emery.conrad at chicagotrading.com>
(cherry picked from commit 77229133dabed0b927748f2a5ae1442b0e97f686)
---
 clang/lib/Interpreter/IncrementalParser.cpp   | 23 ++++++++-
 clang/lib/Interpreter/IncrementalParser.h     |  4 ++
 .../unittests/Interpreter/InterpreterTest.cpp | 48 +++++++++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index 0f28eed888a05..12beb542572d7 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -13,6 +13,7 @@
 #include "IncrementalParser.h"
 #include "IncrementalAction.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclContextInternals.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -172,6 +173,24 @@ IncrementalParser::Parse(llvm::StringRef input) {
   return PTU;
 }
 
+void IncrementalParser::withdrawMostRecentTU(
+    TranslationUnitDecl *MostRecentTU) {
+  TranslationUnitDecl *Prev = MostRecentTU->getPreviousDecl();
+  if (!Prev)
+    return;
+  assert(MostRecentTU->getMostRecentDecl() == MostRecentTU &&
+         "Not the most recent translation unit!");
+
+  // Rebuild A -> ... -> Prev -> MostRecentTU as A -> ... -> Prev.
+  MostRecentTU->getFirstDecl()->RedeclLink.setLatest(Prev);
+
+  // getTranslationUnitDecl() requires the active unit to be the latest one.
+  ASTContext &C = S.getASTContext();
+  if (C.TraversalScope.size() == 1 && C.TraversalScope.back() == MostRecentTU)
+    C.TraversalScope = {Prev};
+  C.TUDecl = Prev;
+}
+
 void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
   if (StoredDeclsMap *Map = MostRecentTU->getPrimaryContext()->getLookupPtr()) {
     // Collect the keys to erase: erasing during iteration invalidates the map
@@ -228,13 +247,15 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
     }
   }
 
-  // FIXME: We should de-allocate MostRecentTU
   for (Decl *D : MostRecentTU->decls()) {
     auto *ND = dyn_cast<NamedDecl>(D);
     if (!ND || ND->getDeclName().isEmpty())
       continue;
     RemoveFromIdResolver(ND);
   }
+
+  // Lookup alone is not enough: the redeclaration chain still reaches these.
+  withdrawMostRecentTU(MostRecentTU);
 }
 
 PartialTranslationUnit &
diff --git a/clang/lib/Interpreter/IncrementalParser.h b/clang/lib/Interpreter/IncrementalParser.h
index a2b654d2ac0f4..b626cebaafcd7 100644
--- a/clang/lib/Interpreter/IncrementalParser.h
+++ b/clang/lib/Interpreter/IncrementalParser.h
@@ -69,6 +69,10 @@ class IncrementalParser {
 
 private:
   llvm::Expected<TranslationUnitDecl *> ParseOrWrapTopLevelDecl();
+
+  /// Rebuild the translation unit redeclaration chain without \p MostRecentTU,
+  /// making its predecessor the current unit again.
+  void withdrawMostRecentTU(TranslationUnitDecl *MostRecentTU);
 };
 } // end namespace clang
 
diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp
index 6a461590ed12e..450be2a25a12f 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -27,6 +27,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include <set>
+
 using namespace clang;
 
 int Global = 42;
@@ -146,6 +148,52 @@ TEST_F(InterpreterTest, DeclsAndStatements) {
   EXPECT_TRUE(!!R2);
 }
 
+TEST_F(InterpreterTest, TranslationUnitRedeclChainAcrossManyPTUs) {
+  std::unique_ptr<Interpreter> Interp = createInterpreter();
+
+  // One partial translation unit per input, as an interop layer doing a
+  // type-probe per lookup would produce.
+  for (unsigned I = 0; I != 200; ++I)
+    cantFail(Interp->Parse("using probe_" + std::to_string(I) + " = int;"));
+
+  TranslationUnitDecl *TU = Interp->getASTContext().getTranslationUnitDecl();
+
+  unsigned Nodes = 0, Decls = 0;
+  for (auto *R : TU->redecls()) {
+    ++Nodes;
+    for (auto *D : cast<DeclContext>(R)->decls()) {
+      ++Decls;
+      // Walking up from the decl is what faults in a long-lived session.
+      EXPECT_EQ(&D->getASTContext(), &Interp->getASTContext());
+      if (auto *ND = dyn_cast<NamedDecl>(D))
+        (void)ND->getQualifiedNameAsString();
+    }
+  }
+  EXPECT_GT(Nodes, 1u);
+  EXPECT_GT(Decls, 200u);
+}
+
+TEST_F(InterpreterTest, UndoLeavesDeclsInTranslationUnitChain) {
+  std::unique_ptr<Interpreter> Interp = createInterpreter();
+
+  cantFail(Interp->Parse("struct Kept {};"));
+  cantFail(Interp->Parse("struct Withdrawn {};"));
+  cantFail(Interp->Undo());
+
+  // A partial translation unit gets its own TranslationUnitDecl, so collect
+  // names across the whole redeclaration chain.
+  std::set<std::string> Names;
+  TranslationUnitDecl *TU = Interp->getASTContext().getTranslationUnitDecl();
+  for (auto *R : TU->redecls())
+    for (auto *D : cast<DeclContext>(R)->decls())
+      if (auto *ND = dyn_cast<NamedDecl>(D))
+        Names.insert(ND->getNameAsString());
+
+  EXPECT_TRUE(Names.count("Kept"));
+  // Undo withdrew this input, so its declaration should not still be reachable.
+  EXPECT_FALSE(Names.count("Withdrawn"));
+}
+
 TEST_F(InterpreterTest, UndoCommand) {
 // FIXME : This test doesn't current work for Emscripten builds.
 // It should be possible to make it work.For details on how it fails and