From 4dd1e416d2278a5890a8947377d38c0b7a518cfb Mon Sep 17 00:00:00 2001
From: Aaron Jomy <aaronjomyjoseph@gmail.com>
Date: Tue, 29 Oct 2024 11:33:49 +0100
Subject: [PATCH] Add support for an external interpreter

The new interface can be called by an external library like ROOT, that manages it's own `TInterpreter` instance.
In this case the `cling::Interpreter*` initialised by TCling is passed to InterOp and a flag indicates that InterOp does not have ownership
---
 include/clang/Interpreter/CppInterOp.h   |  7 ++++
 lib/Interpreter/CppInterOp.cpp           | 21 +++++++----
 unittests/CppInterOp/InterpreterTest.cpp | 48 ++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h
index 9e818cd6f..e5b793041 100644
--- a/include/clang/Interpreter/CppInterOp.h
+++ b/include/clang/Interpreter/CppInterOp.h
@@ -530,6 +530,13 @@ namespace Cpp {
   ///\returns the current interpreter instance, if any.
   CPPINTEROP_API TInterp_t GetInterpreter();
 
+  /// Sets the Interpreter instance with an external interpreter, meant to
+  /// be called by an external library that manages it's own interpreter.
+  /// Sets a flag signifying CppInterOp does not have ownership of the
+  /// sInterpreter.
+  ///\param[in] Args - the pointer to an external interpreter
+  CPPINTEROP_API void UseExternalInterpreter(TInterp_t I);
+
   /// Adds a Search Path for the Interpreter to get the libraries.
   CPPINTEROP_API void AddSearchPath(const char* dir, bool isUser = true,
                                     bool prepend = false);
diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp
index b757cd233..78e506ce7 100644
--- a/lib/Interpreter/CppInterOp.cpp
+++ b/lib/Interpreter/CppInterOp.cpp
@@ -60,19 +60,22 @@ namespace Cpp {
   using namespace llvm;
   using namespace std;
 
-  static std::unique_ptr<compat::Interpreter> sInterpreter;
+  // Flag to indicate ownership when an external interpreter instance is used.
+  static bool OwningSInterpreter = true;
+  static compat::Interpreter* sInterpreter = nullptr;
   // Valgrind complains about __cxa_pure_virtual called when deleting
   // llvm::SectionMemoryManager::~SectionMemoryManager as part of the dtor chain
   // of the Interpreter.
   // This might fix the issue https://reviews.llvm.org/D107087
   // FIXME: For now we just leak the Interpreter.
   struct InterpDeleter {
-    ~InterpDeleter() { sInterpreter.release(); }
+    ~InterpDeleter() = default;
   } Deleter;
 
   static compat::Interpreter& getInterp() {
-    assert(sInterpreter.get() && "Must be set before calling this!");
-    return *sInterpreter.get();
+    assert(sInterpreter &&
+           "Interpreter instance must be set before calling this!");
+    return *sInterpreter;
   }
   static clang::Sema& getSema() { return getInterp().getCI()->getSema(); }
   static clang::ASTContext& getASTContext() { return getSema().getASTContext(); }
@@ -2691,12 +2694,16 @@ namespace Cpp {
     // FIXME: Enable this assert once we figure out how to fix the multiple
     // calls to CreateInterpreter.
     //assert(!sInterpreter && "Interpreter already set.");
-    sInterpreter.reset(I);
+    sInterpreter = I;
     return I;
   }
 
-  TInterp_t GetInterpreter() {
-    return sInterpreter.get();
+  TInterp_t GetInterpreter() { return sInterpreter; }
+
+  void UseExternalInterpreter(TInterp_t I) {
+    assert(sInterpreter && "sInterpreter already in use!");
+    sInterpreter = static_cast<compat::Interpreter*>(I);
+    OwningSInterpreter = false;
   }
 
   void AddSearchPath(const char *dir, bool isUser,
diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp
index 9b70b8a45..7a026d0f2 100644
--- a/unittests/CppInterOp/InterpreterTest.cpp
+++ b/unittests/CppInterOp/InterpreterTest.cpp
@@ -1,6 +1,16 @@
+
 #include "Utils.h"
 
 #include "clang/Interpreter/CppInterOp.h"
+
+#ifdef USE_CLING
+#include "cling/Interpreter/Interpreter.h"
+#endif // USE_CLING
+
+#ifdef USE_REPL
+#include "clang/Interpreter/Interpreter.h"
+#endif // USE_REPL
+
 #include "clang/Basic/Version.h"
 
 #include "llvm/ADT/SmallString.h"
@@ -162,3 +172,41 @@ TEST(InterpreterTest, CodeCompletion) {
   GTEST_SKIP();
 #endif
 }
+
+TEST(InterpreterTest, ExternalInterpreterTest) {
+
+#ifdef USE_REPL
+  llvm::ExitOnError ExitOnErr;
+  clang::IncrementalCompilerBuilder CB;
+  CB.SetCompilerArgs({"-std=c++20"});
+
+  // Create the incremental compiler instance.
+  std::unique_ptr<clang::CompilerInstance> CI;
+  CI = ExitOnErr(CB.CreateCpp());
+
+  // Create the interpreter instance.
+  std::unique_ptr<clang::Interpreter> I =
+      ExitOnErr(clang::Interpreter::create(std::move(CI)));
+  auto ExtInterp = I.get();
+#endif
+
+#ifdef USE_CLING
+    std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr);
+    std::string ResourceDir = compat::MakeResourceDir(LLVM_BINARY_DIR);
+    std::vector<const char *> ClingArgv = {"-resource-dir", ResourceDir.c_str(),
+                                           "-std=c++14"};
+    ClingArgv.insert(ClingArgv.begin(), MainExecutableName.c_str());
+    auto *ExtInterp = new compat::Interpreter(ClingArgv.size(), &ClingArgv[0]);
+#endif
+
+  EXPECT_NE(ExtInterp, nullptr);
+  Cpp::UseExternalInterpreter(ExtInterp);
+  EXPECT_TRUE(Cpp::GetInterpreter()) << "External Interpreter not set";
+
+#ifdef USE_REPL
+  I.release();
+#endif
+#ifdef USE_CLING
+  delete ExtInterp;
+#endif
+}