add Cpp::TakeInterpreter

pops interpreter with destroying it
destruction to be handled by the caller

Author: Vipul-Cariappa

include/CppInterOp/CppInterOp.h

@@ -702,6 +702,11 @@ CreateInterpreter(const std::vector<const char*>& Args = {},
 ///\returns false on failure or if \c I is not tracked in the stack.
 CPPINTEROP_API bool DeleteInterpreter(TInterp_t I = nullptr);
 
+/// Take ownership of an interpreter instance.
+///\param[in] I - the interpreter to be taken, if nullptr, returns the last.
+///\returns nullptr on failure or if \c I is not tracked in the stack.
+CPPINTEROP_API TInterp_t TakeInterpreter(TInterp_t I = nullptr);
+
 /// Activates an instance of an interpreter to handle subsequent API requests
 ///\param[in] I - the interpreter to be activated.
 ///\returns false on failure.

lib/CppInterOp/CppInterOp.cpp

@@ -54,6 +54,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
@@ -140,10 +141,10 @@ struct InterpreterInfo {
 
 // NOLINTBEGIN
 // std::deque avoids relocations and calling the dtor of InterpreterInfo.
-static llvm::ManagedStatic<std::deque<std::shared_ptr<InterpreterInfo>>>
+static llvm::ManagedStatic<std::deque<std::unique_ptr<InterpreterInfo>>>
     sInterpreters;
 static llvm::ManagedStatic<
-    std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>>
+    std::unordered_map<clang::ASTContext*, InterpreterInfo*>>
     sInterpreterASTMap;
 static std::recursive_mutex InterpreterStackLock;
 static std::recursive_mutex LLVMLock;
@@ -161,7 +162,7 @@ static InterpreterInfo& getInterpInfo(const clang::Decl* D) {
     return getInterpInfo();
   if (sInterpreters->size() == 1)
     return *sInterpreters->back();
-  return *(*sInterpreterASTMap)[&D->getASTContext()].lock();
+  return *(*sInterpreterASTMap)[&D->getASTContext()];
 }
 static InterpreterInfo& getInterpInfo(const void* D) {
   std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
@@ -171,7 +172,7 @@ static InterpreterInfo& getInterpInfo(const void* D) {
     return *sInterpreters->back();
   for (auto& item : *sInterpreterASTMap) {
     if (item.first->getAllocator().identifyObject(D))
-      return *item.second.lock();
+      return *item.second;
   }
   llvm_unreachable(
       "This pointer does not belong to any interpreter instance.\n");
@@ -189,7 +190,7 @@ static compat::Interpreter& getInterp(const clang::Decl* D) {
     return getInterp();
   if (sInterpreters->size() == 1)
     return *sInterpreters->back()->Interpreter;
-  return *(*sInterpreterASTMap)[&D->getASTContext()].lock()->Interpreter;
+  return *(*sInterpreterASTMap)[&D->getASTContext()]->Interpreter;
 }
 static compat::Interpreter& getInterp(const void* D) {
   return *getInterpInfo(D).Interpreter;
@@ -3338,6 +3339,8 @@ static std::string MakeResourcesPath() {
 TInterp_t CreateInterpreter(const std::vector<const char*>& Args /*={}*/,
                             const std::vector<const char*>& GpuArgs /*={}*/) {
   std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
+
   std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr);
   std::string ResourceDir = MakeResourcesPath();
   std::vector<const char*> ClingArgv = {"-resource-dir", ResourceDir.c_str(),
@@ -3414,42 +3417,73 @@ TInterp_t CreateInterpreter(const std::vector<const char*>& Args /*={}*/,
   )");
 
   sInterpreters->emplace_back(
-      std::make_shared<InterpreterInfo>(I, /*Owned=*/true));
+      std::make_unique<InterpreterInfo>(I, /*Owned=*/true));
   sInterpreterASTMap->insert(
       {&sInterpreters->back()->Interpreter->getSema().getASTContext(),
-       sInterpreters->back()});
+       sInterpreters->back().get()});
 
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
   return I;
 }
 
+static inline auto find_interpreter_in_stack(TInterp_t I) {
+  return std::find_if(
+      sInterpreters->begin(), sInterpreters->end(),
+      [&I](const auto& Info) { return Info->Interpreter == I; });
+}
+
+static inline auto find_interpreter_in_map(InterpreterInfo* I) {
+  return std::find_if(sInterpreterASTMap->begin(), sInterpreterASTMap->end(),
+                      [](const auto& Item) {
+                        return Item.second == sInterpreters->back().get();
+                      });
+}
+
 bool DeleteInterpreter(TInterp_t I /*=nullptr*/) {
   std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
 
   if (!I) {
-    auto foundAST =
-        std::find_if(sInterpreterASTMap->begin(), sInterpreterASTMap->end(),
-                     [](const auto& Item) {
-                       return Item.second.lock() == sInterpreters->back();
-                     });
+    auto foundAST = find_interpreter_in_map(sInterpreters->back().get());
     sInterpreterASTMap->erase(foundAST);
     sInterpreters->pop_back();
     return true;
   }
 
-  auto found =
-      std::find_if(sInterpreters->begin(), sInterpreters->end(),
-                   [&I](const auto& Info) { return Info->Interpreter == I; });
+  auto found = find_interpreter_in_stack(I);
   if (found == sInterpreters->end())
     return false; // failure
 
-  auto foundAST = std::find_if(
-      sInterpreterASTMap->begin(), sInterpreterASTMap->end(),
-      [&found](const auto& Item) { return Item.second.lock() == *found; });
+  auto foundAST = find_interpreter_in_map((*found).get());
   sInterpreterASTMap->erase(foundAST);
   sInterpreters->erase(found);
   return true;
 }
 
+TInterp_t TakeInterpreter(TInterp_t I /*=nullptr*/) {
+  std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
+
+  if (!I) {
+    auto foundAST = find_interpreter_in_map(sInterpreters->back().get());
+    sInterpreterASTMap->erase(foundAST);
+    InterpreterInfo* res = sInterpreters->back().release();
+    sInterpreters->pop_back();
+    return res->Interpreter;
+  }
+
+  auto found = find_interpreter_in_stack(I);
+  if (found == sInterpreters->end())
+    return nullptr; // failure
+
+  auto foundAST = find_interpreter_in_map((*found).get());
+  sInterpreterASTMap->erase(foundAST);
+  InterpreterInfo* res = (*found).release();
+  sInterpreters->erase(found);
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
+  return res->Interpreter;
+}
+
 bool ActivateInterpreter(TInterp_t I) {
   std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
 
@@ -3477,10 +3511,13 @@ TInterp_t GetInterpreter() {
 
 void UseExternalInterpreter(TInterp_t I) {
   std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
-  assert(sInterpreters->empty() && "sInterpreter already in use!");
   sInterpreters->emplace_back(
-      std::make_shared<InterpreterInfo>(static_cast<compat::Interpreter*>(I),
+      std::make_unique<InterpreterInfo>(static_cast<compat::Interpreter*>(I),
                                         /*isOwned=*/false));
+  sInterpreterASTMap->insert(
+      {&sInterpreters->back()->Interpreter->getSema().getASTContext(),
+       sInterpreters->back().get()});
+  assert(sInterpreters->size() == sInterpreterASTMap->size());
 }
 
 void AddSearchPath(const char* dir, bool isUser, bool prepend) {

unittests/CppInterOp/InterpreterTest.cpp

@@ -88,9 +88,19 @@ TEST(InterpreterTest, DeleteInterpreter) {
 
   EXPECT_EQ(I3, Cpp::GetInterpreter()) << "I3 is not active";
 
+  auto* D1 = Cpp::TakeInterpreter();
+  EXPECT_EQ(D1, I3);
+
+  Cpp::UseExternalInterpreter(D1);
+
   EXPECT_TRUE(Cpp::DeleteInterpreter(nullptr));
   EXPECT_EQ(I2, Cpp::GetInterpreter());
 
+  auto* D2 = Cpp::TakeInterpreter(I2);
+  EXPECT_EQ(I2, D2);
+
+  Cpp::UseExternalInterpreter(D2);
+
   auto* I4 = reinterpret_cast<void*>(static_cast<std::uintptr_t>(~0U));
   EXPECT_FALSE(Cpp::DeleteInterpreter(I4));
 
@@ -150,6 +160,8 @@ TEST(InterpreterTest, Process) {
   EXPECT_EQ(Res, CXError_Success);
   clang_Value_dispose(CXV);
   clang_Interpreter_dispose(CXI);
+  auto* OldI = Cpp::TakeInterpreter();
+  EXPECT_EQ(OldI, I);
 }
 
 TEST(InterpreterTest, EmscriptenExceptionHandling) {