[clang] [clang-repl] Unlink the withdrawn partial translation unit on Undo (PR #213235)
Emery Conrad via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 31 02:58:44 PDT 2026
https://github.com/conrade-ctc created https://github.com/llvm/llvm-project/pull/213235
`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.
Add `ASTContext::withdrawTranslationUnitDecl()`, the inverse of `addTranslationUnitDecl()`, and call it from `IncrementalParser::CleanUpPTU`. 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.
>From 78b3ba9a19aae2e07e7a2ab263a1dcdb27e84986 Mon Sep 17 00:00:00 2001
From: Emery Conrad <emery.conrad at chicagotrading.com>
Date: Fri, 31 Jul 2026 04:10:45 -0500
Subject: [PATCH 1/2] [clang-repl] Add tests for declaration reachability
across partial units
Undo clears lookup but leaves the withdrawn unit's TranslationUnitDecl in the
redeclaration chain, so a lexical walk still reports removed declarations.
UndoLeavesDeclsInTranslationUnitChain fails on that;
TranslationUnitRedeclChainAcrossManyPTUs passes, bounding the problem to Undo.
---
.../unittests/Interpreter/InterpreterTest.cpp | 48 +++++++++++++++++++
1 file changed, 48 insertions(+)
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
>From 3737913dcf5defaab595d63a64e770bf520812b9 Mon Sep 17 00:00:00 2001
From: Emery Conrad <emery.conrad at chicagotrading.com>
Date: Fri, 31 Jul 2026 04:57:09 -0500
Subject: [PATCH 2/2] [clang-repl] Unlink the withdrawn partial translation
unit on Undo
Undo cleared name lookup but left the withdrawn unit's TranslationUnitDecl in
the redeclaration chain, and ASTContext::TUDecl still pointed at it, so the next
input chained onto a unit the user had removed.
Add ASTContext::withdrawTranslationUnitDecl(), the inverse of
addTranslationUnitDecl(), and call it from CleanUpPTU. The declarations stay
bump-allocated; they just stop being reachable.
---
clang/include/clang/AST/ASTContext.h | 13 +++++++++++++
clang/include/clang/AST/Decl.h | 3 +++
clang/lib/AST/Decl.cpp | 9 +++++++++
clang/lib/Interpreter/IncrementalParser.cpp | 4 +++-
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 763039e690dec..3ca7420c9479a 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1317,6 +1317,19 @@ class ASTContext : public RefCountedBase<ASTContext> {
TUDecl = NewTUDecl;
}
+ /// Undo the most recent addTranslationUnitDecl().
+ void withdrawTranslationUnitDecl() {
+ assert(TUKind == TU_Incremental);
+ TranslationUnitDecl *Withdrawn = TUDecl;
+ TranslationUnitDecl *Prev = Withdrawn->getPreviousDecl();
+ if (!Prev)
+ return;
+ Withdrawn->withdrawFromRedeclChain();
+ if (TraversalScope.size() == 1 && TraversalScope.back() == Withdrawn)
+ TraversalScope = {Prev};
+ TUDecl = Prev;
+ }
+
ExternCContextDecl *getExternCContextDecl() const;
#define BuiltinTemplate(BTName) BuiltinTemplateDecl *get##BTName##Decl() const;
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 0a6f256afa2cc..689f10faec67b 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -145,6 +145,9 @@ class TranslationUnitDecl : public Decl,
static TranslationUnitDecl *Create(ASTContext &C);
+ /// Make the previous unit the most recent again, unlinking this one.
+ void withdrawFromRedeclChain();
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == TranslationUnit; }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5a76a726cd1f1..00f334a4c71c0 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5490,6 +5490,15 @@ TranslationUnitDecl *TranslationUnitDecl::Create(ASTContext &C) {
return new (C, (DeclContext *)nullptr) TranslationUnitDecl(C);
}
+// The withdrawn unit's declarations stay allocated; they are bump-allocated in
+// the ASTContext and cannot be released individually.
+void TranslationUnitDecl::withdrawFromRedeclChain() {
+ TranslationUnitDecl *Prev = getPreviousDecl();
+ assert(Prev && "cannot withdraw the initial translation unit");
+ assert(getMostRecentDecl() == this && "not the most recent unit");
+ First->RedeclLink.setLatest(Prev);
+}
+
void TranslationUnitDecl::setAnonymousNamespace(NamespaceDecl *D) {
AnonymousNamespace = D;
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index b13d318a1df76..5c117a176cc20 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -221,7 +221,6 @@ 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())
@@ -231,6 +230,9 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
!D->getLangOpts().CPlusPlus)
S.IdResolver.RemoveDecl(ND);
}
+
+ // Lookup alone is not enough: the redeclaration chain still reaches these.
+ S.getASTContext().withdrawTranslationUnitDecl();
}
PartialTranslationUnit &
More information about the cfe-commits
mailing list