[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

[Stefan Gränitz via cfe-commits]

https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/84176

>From 143ed8ccf592be46181fb3dcd814b4afa1b39833 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Mar 2024 12:37:50 +0100
Subject: [PATCH 1/2] [clang-repl] Refactor locking of runtime PTU stack (NFC)

The previous implementation seemed hacky, because it required the reader to be familiar with the internal workings of the PTU stack. The concept itself is a pragmatic solution and not very surprising. Keeping it behind a finalization call seems reasonable.
---
 clang/include/clang/Interpreter/Interpreter.h |  1 +
 clang/lib/Interpreter/Interpreter.cpp         | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..ce46aefe08a475 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -139,6 +139,7 @@ class Interpreter {
 
 private:
   size_t getEffectivePTUSize() const;
+  void finalizeInitPTUStack();
 
   bool FindRuntimeInterface();
 
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..e06ab45fc6fa0d 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -278,15 +278,14 @@ Interpreter::create(std::unique_ptr<CompilerInstance> CI) {
   if (Err)
     return std::move(Err);
 
+  // Runtimes contain essential definitions that are irrevocable. Lock their
+  // stack of initial PTUs to make them unavailable for undo.
   auto PTU = Interp->Parse(Runtimes);
   if (!PTU)
     return PTU.takeError();
+  Interp->finalizeInitPTUStack();
 
   Interp->ValuePrintingInfo.resize(4);
-  // FIXME: This is a ugly hack. Undo command checks its availability by looking
-  // at the size of the PTU list. However we have parsed something in the
-  // beginning of the REPL so we have to mark them as 'Irrevocable'.
-  Interp->InitPTUSize = Interp->IncrParser->getPTUs().size();
   return std::move(Interp);
 }
 
@@ -343,6 +342,11 @@ const ASTContext &Interpreter::getASTContext() const {
   return getCompilerInstance()->getASTContext();
 }
 
+void Interpreter::finalizeInitPTUStack() {
+  assert(!InitPTUSize && "We only do this once");
+  InitPTUSize = IncrParser->getPTUs().size();
+}
+
 size_t Interpreter::getEffectivePTUSize() const {
   std::list<PartialTranslationUnit> &PTUs = IncrParser->getPTUs();
   assert(PTUs.size() >= InitPTUSize && "empty PTU list?");

>From 6aa4f901c0ba0b946b60b9a4f03ce1a6c47b1924 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Mar 2024 17:28:40 +0100
Subject: [PATCH 2/2] Make interface accessible and add unittest

---
 clang/include/clang/Interpreter/Interpreter.h  |  5 ++---
 .../unittests/Interpreter/InterpreterTest.cpp  | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index ce46aefe08a475..402d3a79d3ed83 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -81,8 +81,6 @@ class Interpreter {
   // An optional parser for CUDA offloading
   std::unique_ptr<IncrementalParser> DeviceParser;
 
-  Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -92,6 +90,7 @@ class Interpreter {
   Value LastValue;
 
 public:
+  Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
   ~Interpreter();
   static llvm::Expected<std::unique_ptr<Interpreter>>
   create(std::unique_ptr<CompilerInstance> CI);
@@ -137,10 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:
   size_t getEffectivePTUSize() const;
   void finalizeInitPTUStack();
 
+private:
   bool FindRuntimeInterface();
 
   llvm::DenseMap<CXXRecordDecl *, llvm::orc::ExecutorAddr> Dtors;
diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp
index e76c0677db5ead..c8fdf969589cf7 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -136,6 +136,24 @@ TEST(InterpreterTest, DeclsAndStatements) {
   EXPECT_TRUE(!!R2);
 }
 
+TEST(InterpreterTest, PTUStack) {
+  clang::IncrementalCompilerBuilder CB;
+  auto CI = cantFail(CB.CreateCpp());
+
+  llvm::Error Err = llvm::Error::success();
+  auto Interp = std::make_unique<Interpreter>(std::move(CI), Err);
+  cantFail(std::move(Err));
+
+  auto NumBuiltinPTUs = Interp->getEffectivePTUSize();
+  cantFail(Interp->Parse("void firstRuntimePTU() {}"));
+  EXPECT_EQ(NumBuiltinPTUs + 1, Interp->getEffectivePTUSize());
+  cantFail(Interp->Parse("void secondRuntimePTU() {}"));
+  EXPECT_EQ(NumBuiltinPTUs + 2, Interp->getEffectivePTUSize());
+
+  Interp->finalizeInitPTUStack();
+  EXPECT_EQ(0ul, Interp->getEffectivePTUSize());
+}
+
 TEST(InterpreterTest, UndoCommand) {
   Args ExtraArgs = {"-Xclang", "-diagnostic-log-file", "-Xclang", "-"};