Stack the created interpreters.

This patch offers an API to switch back and forth different interpreters which
can come at handy in several occasions including research into implementing
threading.

Fixes #302.

Author: vgvassilev

include/CppInterOp/CppInterOp.h

@@ -595,15 +595,26 @@ CPPINTEROP_API void GetOperator(TCppScope_t scope, Operator op,
                                 std::vector<TCppFunction_t>& operators,
                                 OperatorArity kind = kBoth);
 
-/// Creates an instance of the interpreter we need for the various interop
-/// services.
+/// Creates an owned instance of the interpreter we need for the various interop
+/// services and pushes it onto a stack.
 ///\param[in] Args - the list of arguments for interpreter constructor.
 ///\param[in] CPPINTEROP_EXTRA_INTERPRETER_ARGS - an env variable, if defined,
 ///           adds additional arguments to the interpreter.
+///\returns nullptr on failure.
 CPPINTEROP_API TInterp_t
 CreateInterpreter(const std::vector<const char*>& Args = {},
                   const std::vector<const char*>& GpuArgs = {});
 
+/// Deletes an instance of an interpreter.
+///\param[in] I - the interpreter to be deleted, if nullptr, deletes the last.
+///\returns false on failure or if \c I is not tracked in the stack.
+CPPINTEROP_API bool DeleteInterpreter(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.
+CPPINTEROP_API bool ActivateInterpreter(TInterp_t I);
+
 /// Checks which Interpreter backend was CppInterOp library built with (Cling,
 /// Clang-REPL, etcetera). In practice, the selected interpreter should not
 /// matter, since the library will function in the same way.

lib/CppInterOp/CppInterOp.cpp

@@ -46,14 +46,18 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_os_ostream.h"
 
 #include <algorithm>
+#include <cassert>
 #include <iterator>
 #include <map>
+#include <memory>
 #include <set>
 #include <sstream>
+#include <stack>
 #include <string>
 
 // Stream redirect.
@@ -71,30 +75,29 @@
 #include <unistd.h>
 #endif // WIN32
 
-#include <stack>
-
 namespace Cpp {
 
 using namespace clang;
 using namespace llvm;
 using namespace std;
 
-// 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() = default;
-} Deleter;
+struct InterpreterInfo {
+  compat::Interpreter* Interpreter = nullptr;
+  bool isOwned = true;
+
+  // 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.
+  ~InterpreterInfo() = default;
+};
+static llvm::SmallVector<InterpreterInfo, 8> sInterpreters;
 
 static compat::Interpreter& getInterp() {
-  assert(sInterpreter &&
+  assert(!sInterpreters.empty() &&
          "Interpreter instance must be set before calling this!");
-  return *sInterpreter;
+  return *sInterpreters.back().Interpreter;
 }
 static clang::Sema& getSema() { return getInterp().getCI()->getSema(); }
 static clang::ASTContext& getASTContext() { return getSema().getASTContext(); }
@@ -2920,19 +2923,54 @@ TInterp_t CreateInterpreter(const std::vector<const char*>& Args /*={}*/,
     Args[NumArgs + 1] = nullptr;
     llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
-  // FIXME: Enable this assert once we figure out how to fix the multiple
-  // calls to CreateInterpreter.
-  // assert(!sInterpreter && "Interpreter already set.");
-  sInterpreter = I;
+
+  sInterpreters.push_back({I, /*isOwned=*/true});
+
   return I;
 }
 
-TInterp_t GetInterpreter() { return sInterpreter; }
+bool DeleteInterpreter(TInterp_t I /*=nullptr*/) {
+  if (!I) {
+    sInterpreters.pop_back();
+    return true;
+  }
+
+  auto* found =
+      std::find_if(sInterpreters.begin(), sInterpreters.end(),
+                   [&I](const auto& Info) { return Info.Interpreter == I; });
+  if (found == sInterpreters.end())
+    return false; // failure
+
+  sInterpreters.erase(found);
+  return true;
+}
+
+bool ActivateInterpreter(TInterp_t I) {
+  if (!I)
+    return false;
+
+  auto* found =
+      std::find_if(sInterpreters.begin(), sInterpreters.end(),
+                   [&I](const auto& Info) { return Info.Interpreter == I; });
+  if (found == sInterpreters.end())
+    return false;
+
+  if (std::next(found) != sInterpreters.end()) // if not already last element.
+    std::rotate(found, found + 1, sInterpreters.end());
+
+  return true; // success
+}
+
+TInterp_t GetInterpreter() {
+  if (sInterpreters.empty())
+    return nullptr;
+  return sInterpreters.back().Interpreter;
+}
 
 void UseExternalInterpreter(TInterp_t I) {
-  assert(!sInterpreter && "sInterpreter already in use!");
-  sInterpreter = static_cast<compat::Interpreter*>(I);
-  OwningSInterpreter = false;
+  assert(sInterpreters.empty() && "sInterpreter already in use!");
+  sInterpreters.push_back(
+      {static_cast<compat::Interpreter*>(I), /*isOwned=*/false});
 }
 
 void AddSearchPath(const char* dir, bool isUser, bool prepend) {

unittests/CppInterOp/InterpreterTest.cpp

@@ -80,6 +80,47 @@ TEST(InterpreterTest, Evaluate) {
   EXPECT_FALSE(HadError) ;
 }
 
+TEST(InterpreterTest, DeleteInterpreter) {
+  auto* I1 = Cpp::CreateInterpreter();
+  auto* I2 = Cpp::CreateInterpreter();
+  auto* I3 = Cpp::CreateInterpreter();
+  EXPECT_TRUE(I1 && I2 && I3) << "Failed to create interpreters";
+
+  EXPECT_EQ(I3, Cpp::GetInterpreter()) << "I3 is not active";
+
+  EXPECT_TRUE(Cpp::DeleteInterpreter(nullptr));
+  EXPECT_EQ(I2, Cpp::GetInterpreter());
+
+  auto* I4 = reinterpret_cast<void*>(static_cast<std::uintptr_t>(~0U));
+  EXPECT_FALSE(Cpp::DeleteInterpreter(I4));
+
+  EXPECT_TRUE(Cpp::DeleteInterpreter(I1));
+  EXPECT_EQ(I2, Cpp::GetInterpreter()) << "I2 is not active";
+}
+
+TEST(InterpreterTest, ActivateInterpreter) {
+  EXPECT_FALSE(Cpp::ActivateInterpreter(nullptr));
+  auto* Cpp11 = Cpp::CreateInterpreter({"-std=c++11"});
+  auto* Cpp14 = Cpp::CreateInterpreter({"-std=c++14"});
+  auto* Cpp17 = Cpp::CreateInterpreter({"-std=c++17"});
+
+  EXPECT_TRUE(Cpp11 && Cpp14 && Cpp17);
+  EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201703L)
+      << "Failed to activate C++17";
+
+  auto* UntrackedI = reinterpret_cast<void*>(static_cast<std::uintptr_t>(~0U));
+  EXPECT_FALSE(Cpp::ActivateInterpreter(UntrackedI));
+
+  EXPECT_TRUE(Cpp::ActivateInterpreter(Cpp11));
+  EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201103L);
+
+  Cpp::DeleteInterpreter(Cpp11);
+  EXPECT_EQ(Cpp::GetInterpreter(), Cpp17);
+
+  EXPECT_TRUE(Cpp::ActivateInterpreter(Cpp14));
+  EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201402L);
+}
+
 TEST(InterpreterTest, Process) {
 #ifdef _WIN32
   GTEST_SKIP() << "Disabled on Windows. Needs fixing.";