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

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: llvmbot

<details>
<summary>Changes</summary>

Backport 77229133dabe

Requested by: @<!-- -->conrade-ctc

---
Full diff: https://github.com/llvm/llvm-project/pull/218423.diff


3 Files Affected:

- (modified) clang/lib/Interpreter/IncrementalParser.cpp (+22-1) 
- (modified) clang/lib/Interpreter/IncrementalParser.h (+4) 
- (modified) clang/unittests/Interpreter/InterpreterTest.cpp (+48) 


``````````diff
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

``````````

</details>


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