diff --git a/include/CppInterOp/CppInterOp.h b/include/CppInterOp/CppInterOp.h index 9f23be254..2c525ba60 100644 --- a/include/CppInterOp/CppInterOp.h +++ b/include/CppInterOp/CppInterOp.h @@ -595,15 +595,26 @@ CPPINTEROP_API void GetOperator(TCppScope_t scope, Operator op, std::vector& 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& Args = {}, const std::vector& 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. diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 0cafbb113..08fa66ded 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/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 +#include #include #include +#include #include #include +#include #include // Stream redirect. @@ -71,30 +75,29 @@ #include #endif // WIN32 -#include - 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 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& 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(I); - OwningSInterpreter = false; + assert(sInterpreters.empty() && "sInterpreter already in use!"); + sInterpreters.push_back( + {static_cast(I), /*isOwned=*/false}); } void AddSearchPath(const char* dir, bool isUser, bool prepend) { diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index 4c82f0b02..841913c54 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/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(static_cast(~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* Cpp14 = Cpp::CreateInterpreter({"-std=c++14"}); + auto* Cpp17 = Cpp::CreateInterpreter({"-std=c++17"}); + auto* Cpp20 = Cpp::CreateInterpreter({"-std=c++20"}); + + EXPECT_TRUE(Cpp14 && Cpp17 && Cpp20); + EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 202002L) + << "Failed to activate C++20"; + + auto* UntrackedI = reinterpret_cast(static_cast(~0U)); + EXPECT_FALSE(Cpp::ActivateInterpreter(UntrackedI)); + + EXPECT_TRUE(Cpp::ActivateInterpreter(Cpp14)); + EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201402L); + + Cpp::DeleteInterpreter(Cpp14); + EXPECT_EQ(Cpp::GetInterpreter(), Cpp20); + + EXPECT_TRUE(Cpp::ActivateInterpreter(Cpp17)); + EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201703L); +} + TEST(InterpreterTest, Process) { #ifdef _WIN32 GTEST_SKIP() << "Disabled on Windows. Needs fixing.";