Author: Emery Conrad Date: 2026-08-18T16:14:30+03:00 New Revision: 77229133dabed0b927748f2a5ae1442b0e97f686
URL: https://github.com/llvm/llvm-project/commit/77229133dabed0b927748f2a5ae1442b0e97f686 DIFF: https://github.com/llvm/llvm-project/commit/77229133dabed0b927748f2a5ae1442b0e97f686.diff LOG: [clang-repl] Unlink the withdrawn partial translation unit on Undo (#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. `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 <[email protected]> Added: Modified: clang/lib/Interpreter/IncrementalParser.cpp clang/lib/Interpreter/IncrementalParser.h clang/unittests/Interpreter/InterpreterTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index f6d2779d64b2b..59018907056b8 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 @@ -221,7 +240,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 +249,9 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) { !D->getLangOpts().CPlusPlus) S.IdResolver.RemoveDecl(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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
