From 74bcb4ab6beab57751c32d676e4105318d95e5d6 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 14 Oct 2025 17:44:09 +0200 Subject: [PATCH 1/4] Revert "Add tests and dev-doc" This reverts commit c42c90762c5cd839302cd733b103f312c0ae202b. --- docs/DevelopersDocumentation.rst | 46 +------- lib/CppInterOp/CppInterOp.cpp | 2 - unittests/CppInterOp/CMakeLists.txt | 2 - .../CppInterOp/DynamicLibraryManagerTest.cpp | 51 +-------- .../CppInterOp/FunctionReflectionTest.cpp | 2 - unittests/CppInterOp/InterpreterTest.cpp | 103 ++++-------------- .../CppInterOp/TestSharedLib2/CMakeLists.txt | 23 ---- .../TestSharedLib2/TestSharedLib.cpp | 3 - .../CppInterOp/TestSharedLib2/TestSharedLib.h | 11 -- .../CppInterOp/VariableReflectionTest.cpp | 2 - 10 files changed, 24 insertions(+), 221 deletions(-) delete mode 100644 unittests/CppInterOp/TestSharedLib2/CMakeLists.txt delete mode 100644 unittests/CppInterOp/TestSharedLib2/TestSharedLib.cpp delete mode 100644 unittests/CppInterOp/TestSharedLib2/TestSharedLib.h diff --git a/docs/DevelopersDocumentation.rst b/docs/DevelopersDocumentation.rst index e586cc5d8..0be22db90 100644 --- a/docs/DevelopersDocumentation.rst +++ b/docs/DevelopersDocumentation.rst @@ -423,54 +423,12 @@ library files and run pytest: python -m pip install pytest python -m pytest -sv -################################### +*********************************** CppInterOp Internal Documentation -################################### +*********************************** CppInterOp maintains an internal Doxygen documentation of its components. Internal documentation aims to capture intrinsic details and overall usage of code components. The goal of internal documentation is to make the codebase easier to understand for the new developers. Internal documentation can be visited : `here `_ - -************************************** - Multiple Interpreter & Thread-Safety -************************************** - -CppInterOp allows the user to create multiple interpreters at a time and -use those interpreters. The interpreters that are created are stored in a -stack and a map. The stack is used to enable the model where the user -wants to create a temporary interpreter and destroy it after performing a -few operations. In such a use case, the top of the stack is the only -interpreter in use at any given point in time. - -The map is used to store the mapping from :code:`clang::ASTContext` to -:code:`Cpp::InterpreterInfo`. This is required to figure out which -interpreter an object belongs to. Say the library user performs the -following operations: - -1. Create an Interpreter -2. Compile some code with variable :code:`a` -3. Create another Interpreter -4. Performs :code:`Cpp::GetVariableOffset(a)` - -In step 4, the top of the stack is an interpreter without the definition of -:code:`a`. And we cannot use it to figure out the address of :code:`a`. -The :code:`clang::Decl` passed to :code:`Cpp::GetVariableOffset` is used to -retrieve the :code:`clang::ASTContext`, using -:code:`clang::Decl::getASTContext`. We then use the map to figure out the -exact Interpreter Instance this :code:`clang::Decl` belongs to and perform -the operation. - -A shortcoming of this is that if the CppInterOp accepts a -:code:`clang::QualType` instead of :code:`clang::Decl`, then it is not -possible to get the :code:`clang::ASTContext` from the :code:`clang::QualType`. -In such cases, we iterate over the Allocator of all the Interpreters in our -stack and figure out which :code:`clang::ASTContext` allocated this -:code:`clang::QualType`. This is a very expensive operation. But there is no -alternative to this. - -For **thread-safety**, we introduce a lock for each of the interpreters we -create. And lock only that one specific interpreter when required. We also -have 2 global locks, one for LLVM, and another is used to lock operations -performed on the interpreter stack and the map itself. diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index f1527b4a8..a20978dd2 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -3459,8 +3459,6 @@ static inline auto find_interpreter_in_map(InterpreterInfo* I) { bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { std::lock_guard Lock(InterpreterStackLock); assert(sInterpreters->size() == sInterpreterASTMap->size()); - if (sInterpreters->empty()) - return false; if (!I) { auto foundAST = find_interpreter_in_map(sInterpreters->back().get()); diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 843846f25..4b4b43bdd 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -128,8 +128,6 @@ set_output_directory(DynamicLibraryManagerTests ) add_dependencies(DynamicLibraryManagerTests TestSharedLib) -add_dependencies(DynamicLibraryManagerTests TestSharedLib2) #export_executable_symbols_for_plugins(TestSharedLib) add_subdirectory(TestSharedLib) -add_subdirectory(TestSharedLib2) diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index 030c61416..4925af329 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -19,7 +19,7 @@ std::string GetExecutablePath(const char* Argv0) { return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); } -TEST(DynamicLibraryManagerTest, Sanity1) { +TEST(DynamicLibraryManagerTest, Sanity) { #ifdef EMSCRIPTEN GTEST_SKIP() << "Test fails for Emscipten builds"; #endif @@ -66,55 +66,6 @@ TEST(DynamicLibraryManagerTest, Sanity1) { // EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero")); } -TEST(DynamicLibraryManagerTest, Sanity2) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test fails for Emscipten builds"; -#endif - -#if CLANG_VERSION_MAJOR == 18 && defined(CPPINTEROP_USE_CLING) && \ - defined(_WIN32) - GTEST_SKIP() << "Test fails with Cling on Windows"; -#endif - - auto* I = Cpp::CreateInterpreter(); - EXPECT_TRUE(I); - EXPECT_FALSE(Cpp::GetFunctionAddress("ret_one", I)); - - std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); - llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); - Cpp::AddSearchPath(Dir.str().c_str(), true, false, I); - - // FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format - // adds an additional underscore (_) prefix to the lowered names. Figure out - // how to harmonize that API. -#ifdef __APPLE__ - std::string PathToTestSharedLib = - Cpp::SearchLibrariesForSymbol("_ret_one", /*system_search=*/false, I); -#else - std::string PathToTestSharedLib = - Cpp::SearchLibrariesForSymbol("ret_one", /*system_search=*/false, I); -#endif // __APPLE__ - - EXPECT_STRNE("", PathToTestSharedLib.c_str()) - << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() - << "'"; - - EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str(), true, I)); - // Force ExecutionEngine to be created. - Cpp::Process("", I); - Cpp::Declare("", I); - // FIXME: Conda returns false to run this code on osx. -#ifndef __APPLE__ - EXPECT_TRUE(Cpp::GetFunctionAddress("ret_one", I)); -#endif //__APPLE__ - - Cpp::UnloadLibrary("TestSharedLib2", I); - // We have no reliable way to check if it was unloaded because posix does not - // require the library to be actually unloaded but just the handle to be - // invalidated... - // EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero")); -} - TEST(DynamicLibraryManagerTest, BasicSymbolLookup) { #ifndef EMSCRIPTEN GTEST_SKIP() << "This test is only intended for Emscripten builds."; diff --git a/unittests/CppInterOp/FunctionReflectionTest.cpp b/unittests/CppInterOp/FunctionReflectionTest.cpp index caf0d3391..1c76bf8d1 100644 --- a/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -1014,8 +1014,6 @@ TEST(FunctionReflectionTest, BestOverloadFunctionMatch1) { GetAllTopLevelDecls(code, Decls); std::vector candidates; - EXPECT_FALSE(Cpp::BestOverloadFunctionMatch({}, {}, {})); - for (auto decl : Decls) if (Cpp::IsTemplatedFunction(decl)) candidates.push_back((Cpp::TCppFunction_t)decl); diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index bbcf27b43..d27c34d36 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/unittests/CppInterOp/InterpreterTest.cpp @@ -17,16 +17,13 @@ #include "clang-c/CXCppInterOp.h" #include "llvm/ADT/SmallString.h" -#include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" -#include "llvm/Support/raw_ostream.h" +#include #include #include "gtest/gtest.h" #include -#include -#include #include using ::testing::StartsWith; @@ -250,7 +247,7 @@ TEST(InterpreterTest, DISABLED_DetectResourceDir) { #ifdef _WIN32 GTEST_SKIP() << "Disabled on Windows. Needs fixing."; #endif - auto* I = Cpp::CreateInterpreter(); + Cpp::CreateInterpreter(); EXPECT_STRNE(Cpp::DetectResourceDir().c_str(), Cpp::GetResourceDir()); llvm::SmallString<256> Clang(LLVM_BINARY_DIR); llvm::sys::path::append(Clang, "bin", "clang"); @@ -259,7 +256,7 @@ TEST(InterpreterTest, DISABLED_DetectResourceDir) { GTEST_SKIP() << "Test not run (Clang binary does not exist)"; std::string DetectedPath = Cpp::DetectResourceDir(Clang.str().str().c_str()); - EXPECT_STREQ(DetectedPath.c_str(), Cpp::GetResourceDir(I)); + EXPECT_STREQ(DetectedPath.c_str(), Cpp::GetResourceDir()); } TEST(InterpreterTest, DetectSystemCompilerIncludePaths) { @@ -367,86 +364,28 @@ if (llvm::sys::RunningOnValgrind()) #endif } -static int printf_jit(const char* format, ...) { - llvm::errs() << "printf_jit called!\n"; - return 0; -} - TEST(InterpreterTest, MultipleInterpreter) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test fails for Emscipten builds"; -#endif -#ifdef _WIN32 - GTEST_SKIP() << "Disabled on Windows. Needs fixing."; +#if CLANG_VERSION_MAJOR < 20 && defined(EMSCRIPTEN) + GTEST_SKIP() << "Test fails for Emscipten LLVM 20 builds"; #endif - GTEST_SKIP() << "Test does not consistently pass so skipping for now"; - // delete all old interpreters - while (Cpp::DeleteInterpreter()) - ; - std::vector interpreter_args = {"-include", "new"}; - auto* I1 = Cpp::CreateInterpreter(interpreter_args); - EXPECT_TRUE(I1); - - auto F = [](Cpp::TInterp_t I) { - bool hasError = true; - EXPECT_TRUE(Cpp::Evaluate("__cplusplus", &hasError, I) == 201402); - EXPECT_FALSE(hasError); - - Cpp::Declare(R"( - #include - extern "C" int printf(const char*,...); - class MyKlass {}; - )", - false, I); - Cpp::TCppScope_t f = Cpp::GetNamed("printf", Cpp::GetGlobalScope(I)); - EXPECT_TRUE(f); - EXPECT_TRUE(Cpp::GetComplexType(Cpp::GetType("int", I))); - Cpp::TCppType_t MyKlass = Cpp::GetType("MyKlass", I); - EXPECT_EQ(Cpp::GetTypeAsString(MyKlass), "MyKlass"); - EXPECT_EQ(Cpp::GetNumBases(MyKlass), 0); - EXPECT_FALSE(Cpp::GetBaseClass(MyKlass, 3)); - std::vector members; - Cpp::GetEnumConstantDatamembers(Cpp::GetScopeFromType(MyKlass), members); - EXPECT_EQ(members.size(), 0); - - EXPECT_FALSE( - Cpp::InsertOrReplaceJitSymbol("printf", (uint64_t)&printf_jit, I)); - - auto f_callable = Cpp::MakeFunctionCallable(f); - EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); - - EXPECT_FALSE( - Cpp::TakeInterpreter((TInterp_t)1)); // try to take ownership of an - // interpreter that does not exist - - std::vector includes; - Cpp::AddIncludePath("/non/existent/", I); - Cpp::GetIncludePaths(includes, false, false, I); - EXPECT_NE(std::find(includes.begin(), includes.end(), "/non/existent/"), - std::end(includes)); - - EXPECT_TRUE(Cpp::InsertOrReplaceJitSymbol("non_existent", (uint64_t)&f, I)); - }; - F(I1); + auto* I = Cpp::CreateInterpreter(); + EXPECT_TRUE(I); + Cpp::Declare(R"( + void f() {} + )"); + Cpp::TCppScope_t f = Cpp::GetNamed("f"); - auto* I2 = Cpp::CreateInterpreter(interpreter_args); - auto* I3 = Cpp::CreateInterpreter(interpreter_args); - auto* I4 = Cpp::CreateInterpreter(interpreter_args); + auto* I2 = Cpp::CreateInterpreter(); EXPECT_TRUE(I2); - EXPECT_TRUE(I3); - EXPECT_TRUE(I4); - - std::thread t2(F, I2); - std::thread t3(F, I3); - std::thread t4(F, I4); - t2.join(); - t3.join(); - t4.join(); - - testing::internal::CaptureStderr(); - Cpp::Process("printf(\"Blah\");", I2); - std::string cerrs = testing::internal::GetCapturedStderr(); - EXPECT_STREQ(cerrs.c_str(), "printf_jit called!\n"); + Cpp::Declare(R"( + void ff() {} + )"); + Cpp::TCppScope_t ff = Cpp::GetNamed("ff"); + + auto f_callable = Cpp::MakeFunctionCallable(f); + EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); + auto ff_callable = Cpp::MakeFunctionCallable(ff); + EXPECT_EQ(ff_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); } TEST(InterpreterTest, ASMParsing) { diff --git a/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt deleted file mode 100644 index 4d7537b6d..000000000 --- a/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt +++ /dev/null @@ -1,23 +0,0 @@ -add_llvm_library(TestSharedLib2 - SHARED - DISABLE_LLVM_LINK_LLVM_DYLIB - BUILDTREE_ONLY - TestSharedLib.cpp) -# Put TestSharedLib2 next to the unit test executable. -set_output_directory(TestSharedLib2 - BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/../TestSharedLib/unittests/bin/$/ - LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/../TestSharedLib/unittests/bin/$/ - ) - - -if (EMSCRIPTEN) - set_target_properties(TestSharedLib2 - PROPERTIES NO_SONAME 1 - ) - target_link_options(TestSharedLib2 - PRIVATE "SHELL: -s WASM_BIGINT" - PRIVATE "SHELL: -s SIDE_MODULE=1" - ) -endif() - -set_target_properties(TestSharedLib2 PROPERTIES FOLDER "Tests") diff --git a/unittests/CppInterOp/TestSharedLib2/TestSharedLib.cpp b/unittests/CppInterOp/TestSharedLib2/TestSharedLib.cpp deleted file mode 100644 index 6f59fc4f7..000000000 --- a/unittests/CppInterOp/TestSharedLib2/TestSharedLib.cpp +++ /dev/null @@ -1,3 +0,0 @@ -#include "TestSharedLib.h" - -int ret_one() { return 1; } diff --git a/unittests/CppInterOp/TestSharedLib2/TestSharedLib.h b/unittests/CppInterOp/TestSharedLib2/TestSharedLib.h deleted file mode 100644 index fdb3dbaf4..000000000 --- a/unittests/CppInterOp/TestSharedLib2/TestSharedLib.h +++ /dev/null @@ -1,11 +0,0 @@ -#ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H -#define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H - -// Avoid having to mangle/demangle the symbol name in tests -#ifdef _WIN32 -extern "C" __declspec(dllexport) int ret_one(); -#else -extern "C" int __attribute__((visibility("default"))) ret_one(); -#endif - -#endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H diff --git a/unittests/CppInterOp/VariableReflectionTest.cpp b/unittests/CppInterOp/VariableReflectionTest.cpp index 5c9829316..e6bde0e8d 100644 --- a/unittests/CppInterOp/VariableReflectionTest.cpp +++ b/unittests/CppInterOp/VariableReflectionTest.cpp @@ -287,8 +287,6 @@ TEST(VariableReflectionTest, GetVariableOffset) { std::vector datamembers; Cpp::GetDatamembers(Decls[4], datamembers); - EXPECT_FALSE((bool)Cpp::GetVariableOffset(nullptr)); - EXPECT_TRUE((bool) Cpp::GetVariableOffset(Decls[0])); // a EXPECT_TRUE((bool) Cpp::GetVariableOffset(Decls[1])); // N EXPECT_TRUE((bool)Cpp::GetVariableOffset(Decls[2])); // S From d606c8fec92791d852e7cee0cfea2e4e1f3601fc Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 14 Oct 2025 17:44:19 +0200 Subject: [PATCH 2/4] Revert "Add `TInterp_t` argument to funcs to resolve interpreter instance" This reverts commit db363c60d54febf6f7414cca6f7f9119975bad91. --- include/CppInterOp/CppInterOp.h | 71 +++------ lib/CppInterOp/CppInterOp.cpp | 267 ++++++++++++-------------------- 2 files changed, 122 insertions(+), 216 deletions(-) diff --git a/include/CppInterOp/CppInterOp.h b/include/CppInterOp/CppInterOp.h index 5ad261b75..c2e839089 100644 --- a/include/CppInterOp/CppInterOp.h +++ b/include/CppInterOp/CppInterOp.h @@ -389,7 +389,7 @@ CPPINTEROP_API std::string GetQualifiedCompleteName(TCppScope_t klass); CPPINTEROP_API std::vector GetUsingNamespaces(TCppScope_t scope); /// Gets the global scope of the whole C++ instance. -CPPINTEROP_API TCppScope_t GetGlobalScope(TInterp_t interp = nullptr); +CPPINTEROP_API TCppScope_t GetGlobalScope(); /// Strips the typedef and returns the underlying class, and if the /// underlying decl is not a class it returns the input unchanged. @@ -398,28 +398,18 @@ CPPINTEROP_API TCppScope_t GetUnderlyingScope(TCppScope_t scope); /// Gets the namespace or class (by stripping typedefs) for the name /// passed as a parameter, and if the parent is not passed, /// then global scope will be assumed. -/// Looks up the name in parent, if parent is nullptr, -/// interp is used to select the interpreter if multiple in-use. -/// interp is ignored if parent is non-null. CPPINTEROP_API TCppScope_t GetScope(const std::string& name, - TCppScope_t parent = nullptr, - TInterp_t interp = nullptr); + TCppScope_t parent = nullptr); /// When the namespace is known, then the parent doesn't need /// to be specified. This will probably be phased-out in /// future versions of the interop library. -/// interp is used to select the interpreter if multiple in-use. -CPPINTEROP_API TCppScope_t GetScopeFromCompleteName(const std::string& name, - TInterp_t interp = nullptr); +CPPINTEROP_API TCppScope_t GetScopeFromCompleteName(const std::string& name); /// This function performs a lookup within the specified parent, /// a specific named entity (functions, enums, etcetera). -/// Looks up the name in parent, if parent is nullptr, -/// interp is used to select the interpreter if multiple in-use. -/// interp is ignored if parent is non-null. CPPINTEROP_API TCppScope_t GetNamed(const std::string& name, - TCppScope_t parent = nullptr, - TInterp_t interp = nullptr); + TCppScope_t parent = nullptr); /// Gets the parent of the scope that is passed as a parameter. CPPINTEROP_API TCppScope_t GetParentScope(TCppScope_t scope); @@ -503,12 +493,8 @@ CPPINTEROP_API bool IsTemplatedFunction(TCppFunction_t func); /// This function performs a lookup to check if there is a /// templated function of that type. -/// Looks up the name in parent, if parent is nullptr, -/// interp is used to select the interpreter if multiple in-use. -/// interp is ignored if parent is non-null. CPPINTEROP_API bool ExistsFunctionTemplate(const std::string& name, - TCppScope_t parent = nullptr, - TInterp_t interp = nullptr); + TCppScope_t parent = nullptr); /// Sets a list of all the constructor for a scope/class that is /// supplied as a parameter. @@ -556,8 +542,7 @@ CPPINTEROP_API bool IsDestructor(TCppConstFunction_t method); CPPINTEROP_API bool IsStaticMethod(TCppConstFunction_t method); ///\returns the address of the function given its potentially mangled name. -CPPINTEROP_API TCppFuncAddr_t GetFunctionAddress(const char* mangled_name, - TInterp_t interp = nullptr); +CPPINTEROP_API TCppFuncAddr_t GetFunctionAddress(const char* mangled_name); ///\returns the address of the function given its function declaration. CPPINTEROP_API TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method); @@ -658,9 +643,7 @@ CPPINTEROP_API TCppType_t GetCanonicalType(TCppType_t type); /// Used to either get the built-in type of the provided string, or /// use the name to lookup the actual type. -/// interp is used to select the interpreter if multiple in-use. -CPPINTEROP_API TCppType_t GetType(const std::string& type, - TInterp_t interp = nullptr); +CPPINTEROP_API TCppType_t GetType(const std::string& type); ///\returns the complex of the provided type. CPPINTEROP_API TCppType_t GetComplexType(TCppType_t element_type); @@ -744,10 +727,10 @@ 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, TInterp_t I = nullptr); + bool prepend = false); /// Returns the resource-dir path (for headers). -CPPINTEROP_API const char* GetResourceDir(TInterp_t I = nullptr); +CPPINTEROP_API const char* GetResourceDir(); /// Uses the underlying clang compiler to detect the resource directory. /// In essence calling clang -print-resource-dir and checks if it ends with @@ -768,52 +751,46 @@ DetectSystemCompilerIncludePaths(std::vector& Paths, /// Secondary search path for headers, if not found using the /// GetResourceDir() function. -CPPINTEROP_API void AddIncludePath(const char* dir, TInterp_t I = nullptr); +CPPINTEROP_API void AddIncludePath(const char* dir); // Gets the currently used include paths ///\param[out] IncludePaths - the list of include paths /// CPPINTEROP_API void GetIncludePaths(std::vector& IncludePaths, bool withSystem = false, - bool withFlags = false, - TInterp_t I = nullptr); + bool withFlags = false); /// Only Declares a code snippet in \c code and does not execute it. ///\returns 0 on success -CPPINTEROP_API int Declare(const char* code, bool silent = false, - TInterp_t I = nullptr); +CPPINTEROP_API int Declare(const char* code, bool silent = false); /// Declares and executes a code snippet in \c code. ///\returns 0 on success -CPPINTEROP_API int Process(const char* code, TInterp_t I = nullptr); +CPPINTEROP_API int Process(const char* code); /// Declares, executes and returns the execution result as a intptr_t. ///\returns the expression results as a intptr_t. -CPPINTEROP_API intptr_t Evaluate(const char* code, bool* HadError = nullptr, - TInterp_t I = nullptr); +CPPINTEROP_API intptr_t Evaluate(const char* code, bool* HadError = nullptr); /// Looks up the library if access is enabled. ///\returns the path to the library. -CPPINTEROP_API std::string LookupLibrary(const char* lib_name, - TInterp_t I = nullptr); +CPPINTEROP_API std::string LookupLibrary(const char* lib_name); /// Finds \c lib_stem considering the list of search paths and loads it by /// calling dlopen. /// \returns true on success. -CPPINTEROP_API bool LoadLibrary(const char* lib_stem, bool lookup = true, - TInterp_t I = nullptr); +CPPINTEROP_API bool LoadLibrary(const char* lib_stem, bool lookup = true); /// Finds \c lib_stem considering the list of search paths and unloads it by /// calling dlclose. /// function. -CPPINTEROP_API void UnloadLibrary(const char* lib_stem, TInterp_t I = nullptr); +CPPINTEROP_API void UnloadLibrary(const char* lib_stem); /// Scans all libraries on the library search path for a given potentially /// mangled symbol name. ///\returns the path to the first library that contains the symbol definition. -CPPINTEROP_API std::string SearchLibrariesForSymbol(const char* mangled_name, - bool search_system /*true*/, - TInterp_t I = nullptr); +CPPINTEROP_API std::string +SearchLibrariesForSymbol(const char* mangled_name, bool search_system /*true*/); /// Inserts or replaces a symbol in the JIT with the one provided. This is /// useful for providing our own implementations of facilities such as printf. @@ -824,8 +801,7 @@ CPPINTEROP_API std::string SearchLibrariesForSymbol(const char* mangled_name, /// ///\returns true on failure. CPPINTEROP_API bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, - uint64_t address, - TInterp_t I = nullptr); + uint64_t address); /// Tries to load provided objects in a string format (prettyprint). CPPINTEROP_API std::string ObjToString(const char* type, void* obj); @@ -862,10 +838,9 @@ GetClassTemplateInstantiationArgs(TCppScope_t templ_instance, /// Instantiates a function template from a given string representation. This /// function also does overload resolution. -///\param[in] interp - is used to select the interpreter if multiple in-use. ///\returns the instantiated function template declaration. -CPPINTEROP_API TCppFunction_t InstantiateTemplateFunctionFromString( - const char* function_template, TInterp_t interp = nullptr); +CPPINTEROP_API TCppFunction_t +InstantiateTemplateFunctionFromString(const char* function_template); /// Finds best overload match based on explicit template parameters (if any) /// and argument types. @@ -963,7 +938,7 @@ CPPINTEROP_API void CodeComplete(std::vector& Results, /// Reverts the last N operations performed by the interpreter. ///\param[in] N The number of operations to undo. Defaults to 1. ///\returns 0 on success, non-zero on failure. -CPPINTEROP_API int Undo(unsigned N = 1, TInterp_t interp = nullptr); +CPPINTEROP_API int Undo(unsigned N = 1); } // end namespace Cpp diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index a20978dd2..fe98c58b7 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -100,8 +100,6 @@ using namespace std; std::lock_guard interop_lock( \ (InterpInfo).InterpreterLock) -#define NULLPTR (static_cast(nullptr)) - struct InterpreterInfo { compat::Interpreter* Interpreter = nullptr; bool isOwned = true; @@ -152,21 +150,27 @@ static std::recursive_mutex InterpreterStackLock; static std::recursive_mutex LLVMLock; // NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables) +static InterpreterInfo& getInterpInfo() { + std::lock_guard Lock(InterpreterStackLock); + assert(!sInterpreters->empty() && + "Interpreter instance must be set before calling this!"); + return *sInterpreters->back(); +} static InterpreterInfo& getInterpInfo(const clang::Decl* D) { std::lock_guard Lock(InterpreterStackLock); if (!D) - return *sInterpreters->back(); + return getInterpInfo(); if (sInterpreters->size() == 1) return *sInterpreters->back(); return *(*sInterpreterASTMap)[&D->getASTContext()]; } static InterpreterInfo& getInterpInfo(const void* D) { std::lock_guard Lock(InterpreterStackLock); - if (!D) - return *sInterpreters->back(); QualType QT = QualType::getFromOpaquePtr(D); if (auto* D = QT->getAsTagDecl()) return getInterpInfo(D); + if (!D) + return getInterpInfo(); if (sInterpreters->size() == 1) return *sInterpreters->back(); for (auto& item : *sInterpreterASTMap) { @@ -176,22 +180,17 @@ static InterpreterInfo& getInterpInfo(const void* D) { llvm_unreachable( "This pointer does not belong to any interpreter instance.\n"); } -static InterpreterInfo& getInterpInfo(compat::Interpreter* I) { + +static compat::Interpreter& getInterp() { std::lock_guard Lock(InterpreterStackLock); - if (!I) - return *sInterpreters->back(); - auto res = - std::find_if(sInterpreters->begin(), sInterpreters->end(), - [&](const auto& item) { return item->Interpreter == I; }); - if (res != sInterpreters->end()) - return **res; - llvm_unreachable("Invalid State"); + assert(!sInterpreters->empty() && + "Interpreter instance must be set before calling this!"); + return *sInterpreters->back()->Interpreter; } - static compat::Interpreter& getInterp(const clang::Decl* D) { std::lock_guard Lock(InterpreterStackLock); if (!D) - return *getInterpInfo(NULLPTR).Interpreter; + return getInterp(); if (sInterpreters->size() == 1) return *sInterpreters->back()->Interpreter; return *(*sInterpreterASTMap)[&D->getASTContext()]->Interpreter; @@ -200,12 +199,18 @@ static compat::Interpreter& getInterp(const void* D) { return *getInterpInfo(D).Interpreter; } +static clang::Sema& getSema() { return getInterp().getCI()->getSema(); } static clang::Sema& getSema(const clang::Decl* D) { + if (!D) + return getSema(); return getInterpInfo(D).Interpreter->getSema(); } static clang::Sema& getSema(const void* D) { return getInterp(D).getSema(); } +static clang::ASTContext& getASTContext() { return getSema().getASTContext(); } static clang::ASTContext& getASTContext(const clang::Decl* D) { + if (!D) + return getASTContext(); return getSema(D).getASTContext(); } static clang::ASTContext& getASTContext(const void* D) { @@ -736,18 +741,8 @@ std::vector GetUsingNamespaces(TCppScope_t scope) { return {}; } -TCppScope_t GetGlobalScope(TInterp_t interp) { - if (interp) { - auto* I = static_cast(interp); - return I->getSema() - .getASTContext() - .getTranslationUnitDecl() - ->getFirstDecl(); - } - return getSema(NULLPTR) - .getASTContext() - .getTranslationUnitDecl() - ->getFirstDecl(); +TCppScope_t GetGlobalScope() { + return getSema().getASTContext().getTranslationUnitDecl()->getFirstDecl(); } static Decl* GetScopeFromType(QualType QT) { @@ -786,16 +781,15 @@ TCppScope_t GetUnderlyingScope(TCppScope_t scope) { return GetUnderlyingScope((clang::Decl*)scope); } -TCppScope_t GetScope(const std::string& name, TCppScope_t parent, - TInterp_t interp) { +TCppScope_t GetScope(const std::string& name, TCppScope_t parent) { // FIXME: GetScope should be replaced by a general purpose lookup // and filter function. The function should be like GetNamed but // also take in a filter parameter which determines which results // to pass back if (name == "") - return GetGlobalScope(interp); + return GetGlobalScope(); - auto* ND = static_cast(GetNamed(name, parent, interp)); + auto* ND = (NamedDecl*)GetNamed(name, parent); if (!ND || ND == (NamedDecl*)-1) return 0; @@ -808,30 +802,28 @@ TCppScope_t GetScope(const std::string& name, TCppScope_t parent, return 0; } -TCppScope_t GetScopeFromCompleteName(const std::string& name, - TInterp_t interp) { +TCppScope_t GetScopeFromCompleteName(const std::string& name) { std::string delim = "::"; size_t start = 0; size_t end = name.find(delim); TCppScope_t curr_scope = 0; while (end != std::string::npos) { - curr_scope = GetScope(name.substr(start, end - start), curr_scope, interp); + curr_scope = GetScope(name.substr(start, end - start), curr_scope); start = end + delim.length(); end = name.find(delim, start); } - return GetScope(name.substr(start, end), curr_scope, interp); + return GetScope(name.substr(start, end), curr_scope); } -TCppScope_t GetNamed(const std::string& name, TCppScope_t parent /*= nullptr*/, - TInterp_t interp /*= nullptr*/) { - if (!parent) - parent = GetGlobalScope(interp); - +TCppScope_t GetNamed(const std::string& name, + TCppScope_t parent /*= nullptr*/) { + clang::DeclContext* Within = 0; auto* D = static_cast(parent); LOCK(getInterpInfo(D)); - - D = GetUnderlyingScope(D); - auto* Within = llvm::dyn_cast(D); + if (parent) { + D = GetUnderlyingScope(D); + Within = llvm::dyn_cast(D); + } auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { @@ -1247,13 +1239,13 @@ bool IsTemplatedFunction(TCppFunction_t func) { // FIXME: This lookup is broken, and should no longer be used in favour of // `GetClassTemplatedMethods` If the candidate set returned is =1, that means // the template function exists and >1 means overloads -bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent, - TInterp_t interp) { - if (!parent) - parent = GetGlobalScope(interp); - +bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) { + DeclContext* Within = 0; auto* D = static_cast(parent); - auto* Within = llvm::dyn_cast(D); + if (parent) { + Within = llvm::dyn_cast(D); + } + LOCK(getInterpInfo(D)); auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); @@ -1472,12 +1464,9 @@ bool IsStaticMethod(TCppConstFunction_t method) { return false; } -TCppFuncAddr_t GetFunctionAddress(const char* mangled_name, - TInterp_t interp /*=nullptr*/) { - auto* I = static_cast(interp); - if (!I) - I = &getInterp(NULLPTR); - auto FDAorErr = compat::getSymbolAddress(*I, mangled_name); +TCppFuncAddr_t GetFunctionAddress(const char* mangled_name) { + auto& I = getInterp(); + auto FDAorErr = compat::getSymbolAddress(I, mangled_name); if (llvm::Error Err = FDAorErr.takeError()) llvm::consumeError(std::move(Err)); // nullptr if missing else @@ -1520,9 +1509,9 @@ TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) && !FD->getDefinition()) InstantiateFunctionDefinition(D); - ASTContext& C = getASTContext(FD); + ASTContext& C = getASTContext(); if (isDiscardableGVALinkage(C.GetGVALinkageForFunction(FD))) - ForceCodeGen(FD, getInterp(FD)); + ForceCodeGen(FD, getInterp()); return GetFunctionAddress(FD); } return nullptr; @@ -1848,7 +1837,7 @@ bool IsRValueReferenceType(TCppType_t type) { TCppType_t GetPointerType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); - return getASTContext(type) + return getASTContext() .getPointerType(QT) .getAsOpaquePtr(); // FIXME: which ASTContext? } @@ -2023,18 +2012,13 @@ static QualType findBuiltinType(llvm::StringRef typeName, ASTContext& Context) { } } // namespace -TCppType_t GetType(const std::string& name, TInterp_t interp) { - auto* I = static_cast(interp); - QualType builtin; - if (I) - builtin = findBuiltinType(name, I->getSema().getASTContext()); - else - builtin = - findBuiltinType(name, getInterp(NULLPTR).getSema().getASTContext()); +TCppType_t GetType(const std::string& name) { + QualType builtin = findBuiltinType(name, getASTContext()); if (!builtin.isNull()) return builtin.getAsOpaquePtr(); - auto* D = static_cast(GetNamed(name, /*parent=*/nullptr, interp)); + LOCK(getInterpInfo()); + auto* D = (Decl*)GetNamed(name, /* Within= */ 0); if (auto* TD = llvm::dyn_cast_or_null(D)) { return QualType(TD->getTypeForDecl(), 0).getAsOpaquePtr(); } @@ -3539,22 +3523,13 @@ void UseExternalInterpreter(TInterp_t I) { assert(sInterpreters->size() == sInterpreterASTMap->size()); } -void AddSearchPath(const char* dir, bool isUser, bool prepend, - TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - interp->getDynamicLibraryManager()->addSearchPath(dir, isUser, prepend); - else - getInterp(NULLPTR).getDynamicLibraryManager()->addSearchPath(dir, isUser, - prepend); +void AddSearchPath(const char* dir, bool isUser, bool prepend) { + LOCK(getInterpInfo()); + getInterp().getDynamicLibraryManager()->addSearchPath(dir, isUser, prepend); } -const char* GetResourceDir(TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - if (interp) - return interp->getCI()->getHeaderSearchOpts().ResourceDir.c_str(); - return getInterp(NULLPTR).getCI()->getHeaderSearchOpts().ResourceDir.c_str(); +const char* GetResourceDir() { + return getInterp().getCI()->getHeaderSearchOpts().ResourceDir.c_str(); } ///\returns 0 on success. @@ -3613,23 +3588,15 @@ void DetectSystemCompilerIncludePaths(std::vector& Paths, exec(cmd.c_str(), Paths); } -void AddIncludePath(const char* dir, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - interp->AddIncludePath(dir); - else - getInterp(NULLPTR).AddIncludePath(dir); +void AddIncludePath(const char* dir) { + LOCK(getInterpInfo()); + getInterp().AddIncludePath(dir); } void GetIncludePaths(std::vector& IncludePaths, bool withSystem, - bool withFlags, TInterp_t I /*=nullptr*/) { + bool withFlags) { llvm::SmallVector paths(1); - auto* interp = static_cast(I); - if (interp) - interp->GetIncludePaths(paths, withSystem, withFlags); - else - getInterp(NULLPTR).GetIncludePaths(paths, withSystem, withFlags); + getInterp().GetIncludePaths(paths, withSystem, withFlags); for (auto& i : paths) IncludePaths.push_back(i); } @@ -3660,24 +3627,17 @@ int Declare(compat::Interpreter& I, const char* code, bool silent) { return I.declare(code); } -int Declare(const char* code, bool silent, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - return Declare(*interp, code, silent); - return Declare(getInterp(NULLPTR), code, silent); +int Declare(const char* code, bool silent) { + LOCK(getInterpInfo()); + return Declare(getInterp(), code, silent); } -int Process(const char* code, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - return interp->process(code); - return getInterp(NULLPTR).process(code); +int Process(const char* code) { + LOCK(getInterpInfo()); + return getInterp().process(code); } -intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/, - TInterp_t I /*=nullptr*/) { +intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/) { #ifdef CPPINTEROP_USE_CLING cling::Value V; #else @@ -3687,14 +3647,8 @@ intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/, if (HadError) *HadError = false; - auto* interp = static_cast(I); - compat::Interpreter::CompilationResult res; - LOCK(getInterpInfo(interp)); - if (interp) - res = interp->evaluate(code, V); - else - res = getInterp(NULLPTR).evaluate(code, V); - + LOCK(getInterpInfo()); + auto res = getInterp().evaluate(code, V); if (res != 0) { // 0 is success if (HadError) *HadError = true; @@ -3705,42 +3659,27 @@ intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/, return compat::convertTo(V); } -std::string LookupLibrary(const char* lib_name, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - if (interp) - return interp->getDynamicLibraryManager()->lookupLibrary(lib_name); - return getInterp(NULLPTR).getDynamicLibraryManager()->lookupLibrary(lib_name); +std::string LookupLibrary(const char* lib_name) { + return getInterp().getDynamicLibraryManager()->lookupLibrary(lib_name); } -bool LoadLibrary(const char* lib_stem, bool lookup, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - compat::Interpreter::CompilationResult res; - if (interp) - res = interp->loadLibrary(lib_stem, lookup); - else - res = getInterp(NULLPTR).loadLibrary(lib_stem, lookup); +bool LoadLibrary(const char* lib_stem, bool lookup) { + LOCK(getInterpInfo()); + compat::Interpreter::CompilationResult res = + getInterp().loadLibrary(lib_stem, lookup); return res == compat::Interpreter::kSuccess; } -void UnloadLibrary(const char* lib_stem, TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - interp->getDynamicLibraryManager()->unloadLibrary(lib_stem); - else - getInterp(NULLPTR).getDynamicLibraryManager()->unloadLibrary(lib_stem); +void UnloadLibrary(const char* lib_stem) { + LOCK(getInterpInfo()); + getInterp().getDynamicLibraryManager()->unloadLibrary(lib_stem); } std::string SearchLibrariesForSymbol(const char* mangled_name, - bool search_system /*true*/, - TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (!interp) - interp = &getInterp(NULLPTR); - auto* DLM = interp->getDynamicLibraryManager(); + bool search_system /*true*/) { + LOCK(getInterpInfo()); + auto* DLM = getInterp().getDynamicLibraryManager(); return DLM->searchLibrariesForSymbol(mangled_name, search_system); } @@ -3823,14 +3762,10 @@ bool InsertOrReplaceJitSymbol(compat::Interpreter& I, return false; } -bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, uint64_t address, - TInterp_t I /*=nullptr*/) { - auto* interp = static_cast(I); - LOCK(getInterpInfo(interp)); - if (interp) - return InsertOrReplaceJitSymbol(*interp, linker_mangled_name, address); - return InsertOrReplaceJitSymbol(getInterp(NULLPTR), linker_mangled_name, - address); +bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, + uint64_t address) { + LOCK(getInterpInfo()); + return InsertOrReplaceJitSymbol(getInterp(), linker_mangled_name, address); } std::string ObjToString(const char* type, void* obj) { @@ -3842,6 +3777,7 @@ std::string ObjToString(const char* type, void* obj) { static Decl* InstantiateTemplate(TemplateDecl* TemplateD, TemplateArgumentListInfo& TLI, Sema& S, bool instantiate_body) { + LOCK(getInterpInfo()); // This is not right but we don't have a lot of options to choose from as a // template instantiation requires a valid source location. SourceLocation fakeLoc = GetValidSLoc(S); @@ -3968,8 +3904,7 @@ void GetClassTemplateInstantiationArgs(TCppScope_t templ_instance, } TCppFunction_t -InstantiateTemplateFunctionFromString(const char* function_template, - TInterp_t interp /*=nullptr*/) { +InstantiateTemplateFunctionFromString(const char* function_template) { // FIXME: Drop this interface and replace it with the proper overload // resolution handling and template instantiation selection. @@ -3978,13 +3913,11 @@ InstantiateTemplateFunctionFromString(const char* function_template, std::string id = "__Cppyy_GetMethTmpl_" + std::to_string(var_count++); std::string instance = "auto " + id + " = " + function_template + ";\n"; - auto* I = static_cast(interp); - LOCK(getInterpInfo(I)); - if (!Cpp::Declare(instance.c_str(), /*silent=*/false, interp)) { - auto* VD = static_cast(Cpp::GetNamed(id, nullptr, interp)); - Expr* E = VD->getInit()->IgnoreImpCasts(); - if (auto* DRE = llvm::dyn_cast(E)) - return DRE->getDecl(); + LOCK(getInterpInfo()); + if (!Cpp::Declare(instance.c_str(), /*silent=*/false)) { + VarDecl* VD = (VarDecl*)Cpp::GetNamed(id, 0); + DeclRefExpr* DRE = (DeclRefExpr*)VD->getInit()->IgnoreImpCasts(); + return DRE->getDecl(); } return nullptr; } @@ -4373,16 +4306,14 @@ void CodeComplete(std::vector& Results, const char* code, complete_column); } -int Undo(unsigned N, TInterp_t interp) { - auto* I = static_cast(interp); - if (!I) - I = &getInterp(NULLPTR); +int Undo(unsigned N) { #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(I); - I->unload(N); + auto& I = getInterp(); + cling::Interpreter::PushTransactionRAII RAII(&I); + I.unload(N); return compat::Interpreter::kSuccess; #else - return I->undo(N); + return getInterp().undo(N); #endif } From f9e78db0ccfdbdc56fb684b4af297426826c62b4 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 14 Oct 2025 17:44:32 +0200 Subject: [PATCH 3/4] Revert "Add `Cpp::TakeInterpreter`" This reverts commit da06567e6ce8bae28dfbf8af610457d8b49fd832. --- include/CppInterOp/CppInterOp.h | 5 -- lib/CppInterOp/CppInterOp.cpp | 90 +++++++----------------- lib/CppInterOp/CppInterOpInterpreter.h | 3 +- unittests/CppInterOp/InterpreterTest.cpp | 23 +++--- 4 files changed, 37 insertions(+), 84 deletions(-) diff --git a/include/CppInterOp/CppInterOp.h b/include/CppInterOp/CppInterOp.h index c2e839089..2262de047 100644 --- a/include/CppInterOp/CppInterOp.h +++ b/include/CppInterOp/CppInterOp.h @@ -702,11 +702,6 @@ CreateInterpreter(const std::vector& 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. diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index fe98c58b7..ab20cdd10 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -54,7 +54,6 @@ #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" @@ -139,16 +138,16 @@ struct InterpreterInfo { InterpreterInfo& operator=(const InterpreterInfo&) = delete; }; -// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables) +// NOLINTBEGIN // std::deque avoids relocations and calling the dtor of InterpreterInfo. -static llvm::ManagedStatic>> +static llvm::ManagedStatic>> sInterpreters; static llvm::ManagedStatic< - std::unordered_map> + std::unordered_map>> sInterpreterASTMap; static std::recursive_mutex InterpreterStackLock; static std::recursive_mutex LLVMLock; -// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables) +// NOLINTEND static InterpreterInfo& getInterpInfo() { std::lock_guard Lock(InterpreterStackLock); @@ -162,20 +161,17 @@ static InterpreterInfo& getInterpInfo(const clang::Decl* D) { return getInterpInfo(); if (sInterpreters->size() == 1) return *sInterpreters->back(); - return *(*sInterpreterASTMap)[&D->getASTContext()]; + return *(*sInterpreterASTMap)[&D->getASTContext()].lock(); } static InterpreterInfo& getInterpInfo(const void* D) { std::lock_guard Lock(InterpreterStackLock); - QualType QT = QualType::getFromOpaquePtr(D); - if (auto* D = QT->getAsTagDecl()) - return getInterpInfo(D); if (!D) return getInterpInfo(); if (sInterpreters->size() == 1) return *sInterpreters->back(); for (auto& item : *sInterpreterASTMap) { if (item.first->getAllocator().identifyObject(D)) - return *item.second; + return *item.second.lock(); } llvm_unreachable( "This pointer does not belong to any interpreter instance.\n"); @@ -193,7 +189,7 @@ static compat::Interpreter& getInterp(const clang::Decl* D) { return getInterp(); if (sInterpreters->size() == 1) return *sInterpreters->back()->Interpreter; - return *(*sInterpreterASTMap)[&D->getASTContext()]->Interpreter; + return *(*sInterpreterASTMap)[&D->getASTContext()].lock()->Interpreter; } static compat::Interpreter& getInterp(const void* D) { return *getInterpInfo(D).Interpreter; @@ -3342,8 +3338,6 @@ static std::string MakeResourcesPath() { TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, const std::vector& GpuArgs /*={}*/) { std::lock_guard Lock(InterpreterStackLock); - assert(sInterpreters->size() == sInterpreterASTMap->size()); - std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr); std::string ResourceDir = MakeResourcesPath(); std::vector ClingArgv = {"-resource-dir", ResourceDir.c_str(), @@ -3420,73 +3414,42 @@ TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, )"); sInterpreters->emplace_back( - std::make_unique(I, /*Owned=*/true)); + std::make_shared(I, /*Owned=*/true)); sInterpreterASTMap->insert( {&sInterpreters->back()->Interpreter->getSema().getASTContext(), - sInterpreters->back().get()}); + sInterpreters->back()}); - 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 == I; }); -} - bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { std::lock_guard Lock(InterpreterStackLock); - assert(sInterpreters->size() == sInterpreterASTMap->size()); if (!I) { - auto foundAST = find_interpreter_in_map(sInterpreters->back().get()); - assert(foundAST != sInterpreterASTMap->end()); + auto foundAST = + std::find_if(sInterpreterASTMap->begin(), sInterpreterASTMap->end(), + [](const auto& Item) { + return Item.second.lock() == sInterpreters->back(); + }); sInterpreterASTMap->erase(foundAST); sInterpreters->pop_back(); return true; } - auto found = find_interpreter_in_stack(I); + auto found = + std::find_if(sInterpreters->begin(), sInterpreters->end(), + [&I](const auto& Info) { return Info->Interpreter == I; }); if (found == sInterpreters->end()) return false; // failure - auto foundAST = find_interpreter_in_map((*found).get()); - assert(foundAST != sInterpreterASTMap->end()); + auto foundAST = std::find_if( + sInterpreterASTMap->begin(), sInterpreterASTMap->end(), + [&found](const auto& Item) { return Item.second.lock() == *found; }); sInterpreterASTMap->erase(foundAST); sInterpreters->erase(found); return true; } -TInterp_t TakeInterpreter(TInterp_t I /*=nullptr*/) { - std::lock_guard 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 Lock(InterpreterStackLock); @@ -3514,13 +3477,10 @@ TInterp_t GetInterpreter() { void UseExternalInterpreter(TInterp_t I) { std::lock_guard Lock(InterpreterStackLock); + assert(sInterpreters->empty() && "sInterpreter already in use!"); sInterpreters->emplace_back( - std::make_unique(static_cast(I), + std::make_shared(static_cast(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) { @@ -3774,9 +3734,9 @@ std::string ObjToString(const char* type, void* obj) { return getInterp(NULLPTR).toString(type, obj); } -static Decl* InstantiateTemplate(TemplateDecl* TemplateD, - TemplateArgumentListInfo& TLI, Sema& S, - bool instantiate_body) { +Decl* InstantiateTemplate(TemplateDecl* TemplateD, + TemplateArgumentListInfo& TLI, Sema& S, + bool instantiate_body) { LOCK(getInterpInfo()); // This is not right but we don't have a lot of options to choose from as a // template instantiation requires a valid source location. diff --git a/lib/CppInterOp/CppInterOpInterpreter.h b/lib/CppInterOp/CppInterOpInterpreter.h index 8189f9653..06a04da01 100644 --- a/lib/CppInterOp/CppInterOpInterpreter.h +++ b/lib/CppInterOp/CppInterOpInterpreter.h @@ -140,11 +140,12 @@ namespace Cpp { /// CppInterOp Interpreter /// class Interpreter { +private: std::unique_ptr inner; -public: Interpreter(std::unique_ptr CI) : inner(std::move(CI)) {} +public: static std::unique_ptr create(int argc, const char* const* argv, const char* llvmdir = nullptr, const std::vector>& diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index d27c34d36..eb9842681 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/unittests/CppInterOp/InterpreterTest.cpp @@ -24,7 +24,6 @@ #include "gtest/gtest.h" #include -#include using ::testing::StartsWith; @@ -151,8 +150,6 @@ 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) { @@ -332,9 +329,7 @@ if (llvm::sys::RunningOnValgrind()) // Create the interpreter instance. std::unique_ptr I = ExitOnErr(clang::Interpreter::create(std::move(CI))); - - auto CPPI = Cpp::Interpreter(std::move(I)); - auto* ExtInterp = &CPPI; + auto ExtInterp = I.get(); #endif // CPPINTEROP_USE_REPL #ifdef CPPINTEROP_USE_CLING @@ -351,9 +346,12 @@ if (llvm::sys::RunningOnValgrind()) EXPECT_NE(ExtInterp, nullptr); - Cpp::UseExternalInterpreter(ExtInterp); - EXPECT_EQ(ExtInterp, Cpp::GetInterpreter()); - EXPECT_EQ(ExtInterp, Cpp::TakeInterpreter(ExtInterp)); +#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST +#ifndef _WIN32 // Windows seems to fail to die... + EXPECT_DEATH(Cpp::UseExternalInterpreter(ExtInterp), "sInterpreter already in use!"); +#endif // _WIN32 +#endif + EXPECT_TRUE(Cpp::GetInterpreter()) << "External Interpreter not set"; #ifndef CPPINTEROP_USE_CLING I.release(); @@ -368,15 +366,14 @@ TEST(InterpreterTest, MultipleInterpreter) { #if CLANG_VERSION_MAJOR < 20 && defined(EMSCRIPTEN) GTEST_SKIP() << "Test fails for Emscipten LLVM 20 builds"; #endif - auto* I = Cpp::CreateInterpreter(); - EXPECT_TRUE(I); + + EXPECT_TRUE(Cpp::CreateInterpreter()); Cpp::Declare(R"( void f() {} )"); Cpp::TCppScope_t f = Cpp::GetNamed("f"); - auto* I2 = Cpp::CreateInterpreter(); - EXPECT_TRUE(I2); + EXPECT_TRUE(Cpp::CreateInterpreter()); Cpp::Declare(R"( void ff() {} )"); From 9368bb98ac7ce5db2ced8d6488975f31a028e6a1 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 14 Oct 2025 17:44:56 +0200 Subject: [PATCH 4/4] Revert "Thread-Safty: Add Mutex per Interpreter" This reverts commit d210d68f381de9c8e2160fbaaa3495c615b2afce. --- lib/CppInterOp/CppInterOp.cpp | 364 ++++++----------------- unittests/CppInterOp/InterpreterTest.cpp | 51 ---- 2 files changed, 86 insertions(+), 329 deletions(-) diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index ab20cdd10..b6cf555e8 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -65,14 +65,11 @@ #include #include #include -#include #include #include #include #include -#include #include -#include // Stream redirect. #ifdef _WIN32 @@ -95,15 +92,9 @@ using namespace clang; using namespace llvm; using namespace std; -#define LOCK(InterpInfo) \ - std::lock_guard interop_lock( \ - (InterpInfo).InterpreterLock) - struct InterpreterInfo { compat::Interpreter* Interpreter = nullptr; bool isOwned = true; - std::recursive_mutex InterpreterLock; - InterpreterInfo(compat::Interpreter* I, bool Owned) : Interpreter(I), isOwned(Owned) {} @@ -138,80 +129,16 @@ struct InterpreterInfo { InterpreterInfo& operator=(const InterpreterInfo&) = delete; }; -// NOLINTBEGIN // std::deque avoids relocations and calling the dtor of InterpreterInfo. -static llvm::ManagedStatic>> - sInterpreters; -static llvm::ManagedStatic< - std::unordered_map>> - sInterpreterASTMap; -static std::recursive_mutex InterpreterStackLock; -static std::recursive_mutex LLVMLock; -// NOLINTEND - -static InterpreterInfo& getInterpInfo() { - std::lock_guard Lock(InterpreterStackLock); - assert(!sInterpreters->empty() && - "Interpreter instance must be set before calling this!"); - return *sInterpreters->back(); -} -static InterpreterInfo& getInterpInfo(const clang::Decl* D) { - std::lock_guard Lock(InterpreterStackLock); - if (!D) - return getInterpInfo(); - if (sInterpreters->size() == 1) - return *sInterpreters->back(); - return *(*sInterpreterASTMap)[&D->getASTContext()].lock(); -} -static InterpreterInfo& getInterpInfo(const void* D) { - std::lock_guard Lock(InterpreterStackLock); - if (!D) - return getInterpInfo(); - if (sInterpreters->size() == 1) - return *sInterpreters->back(); - for (auto& item : *sInterpreterASTMap) { - if (item.first->getAllocator().identifyObject(D)) - return *item.second.lock(); - } - llvm_unreachable( - "This pointer does not belong to any interpreter instance.\n"); -} +static llvm::ManagedStatic> sInterpreters; static compat::Interpreter& getInterp() { - std::lock_guard Lock(InterpreterStackLock); assert(!sInterpreters->empty() && "Interpreter instance must be set before calling this!"); - return *sInterpreters->back()->Interpreter; -} -static compat::Interpreter& getInterp(const clang::Decl* D) { - std::lock_guard Lock(InterpreterStackLock); - if (!D) - return getInterp(); - if (sInterpreters->size() == 1) - return *sInterpreters->back()->Interpreter; - return *(*sInterpreterASTMap)[&D->getASTContext()].lock()->Interpreter; + return *sInterpreters->back().Interpreter; } -static compat::Interpreter& getInterp(const void* D) { - return *getInterpInfo(D).Interpreter; -} - static clang::Sema& getSema() { return getInterp().getCI()->getSema(); } -static clang::Sema& getSema(const clang::Decl* D) { - if (!D) - return getSema(); - return getInterpInfo(D).Interpreter->getSema(); -} -static clang::Sema& getSema(const void* D) { return getInterp(D).getSema(); } - static clang::ASTContext& getASTContext() { return getSema().getASTContext(); } -static clang::ASTContext& getASTContext(const clang::Decl* D) { - if (!D) - return getASTContext(); - return getSema(D).getASTContext(); -} -static clang::ASTContext& getASTContext(const void* D) { - return getSema(D).getASTContext(); -} static void ForceCodeGen(Decl* D, compat::Interpreter& I) { // The decl was deferred by CodeGen. Force its emission. @@ -339,29 +266,21 @@ std::string Demangle(const std::string& mangled_name) { return demangle; } -void EnableDebugOutput(bool value /* =true*/) { - std::lock_guard Lock(LLVMLock); - llvm::DebugFlag = value; -} +void EnableDebugOutput(bool value /* =true*/) { llvm::DebugFlag = value; } -bool IsDebugOutputEnabled() { - std::lock_guard Lock(LLVMLock); - return llvm::DebugFlag; -} +bool IsDebugOutputEnabled() { return llvm::DebugFlag; } static void InstantiateFunctionDefinition(Decl* D) { + compat::SynthesizingCodeRAII RAII(&getInterp()); if (auto* FD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(FD)); - compat::SynthesizingCodeRAII RAII(&getInterp(FD)); - getSema(FD).InstantiateFunctionDefinition(SourceLocation(), FD, - /*Recursive=*/true, - /*DefinitionRequired=*/true); + getSema().InstantiateFunctionDefinition(SourceLocation(), FD, + /*Recursive=*/true, + /*DefinitionRequired=*/true); } } bool IsAggregate(TCppScope_t scope) { Decl* D = static_cast(scope); - LOCK(getInterpInfo(D)); // Aggregates are only arrays or tag decls. if (ValueDecl* ValD = dyn_cast(D)) @@ -397,11 +316,9 @@ bool IsFunctionPointerType(TCppType_t type) { bool IsClassPolymorphic(TCppScope_t klass) { Decl* D = static_cast(klass); - if (auto* CXXRD = llvm::dyn_cast(D)) { - LOCK(getInterpInfo(CXXRD)); + if (auto* CXXRD = llvm::dyn_cast(D)) if (auto* CXXRDD = CXXRD->getDefinition()) return CXXRDD->isPolymorphic(); - } return false; } @@ -417,13 +334,12 @@ bool IsComplete(TCppScope_t scope) { Decl* D = static_cast(scope); - LOCK(getInterpInfo(D)); if (isa(D)) { QualType QT = QualType::getFromOpaquePtr(GetTypeFromScope(scope)); - clang::Sema& S = getSema(D); + clang::Sema& S = getSema(); SourceLocation fakeLoc = GetValidSLoc(S); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp(D)); + cling::Interpreter::PushTransactionRAII RAII(&getInterp()); #endif // CPPINTEROP_USE_CLING return S.isCompleteType(fakeLoc, QT); } @@ -443,7 +359,6 @@ size_t SizeOf(TCppScope_t scope) { return 0; if (auto* RD = dyn_cast(static_cast(scope))) { - LOCK(getInterpInfo(RD)); ASTContext& Context = RD->getASTContext(); const ASTRecordLayout& Layout = Context.getASTRecordLayout(RD); return Layout.getSize().getQuantity(); @@ -477,10 +392,8 @@ bool IsTypedefed(TCppScope_t handle) { bool IsAbstract(TCppType_t klass) { auto* D = (clang::Decl*)klass; - if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); + if (auto* CXXRD = llvm::dyn_cast_or_null(D)) return CXXRD->isAbstract(); - } return false; } @@ -514,7 +427,6 @@ static bool isSmartPointer(const RecordType* RT) { }; const RecordDecl* Record = RT->getDecl(); - LOCK(getInterpInfo(Record)); if (IsUseCountPresent(Record)) return true; @@ -582,7 +494,6 @@ std::vector GetEnumConstants(TCppScope_t handle) { auto* D = (clang::Decl*)handle; if (auto* ED = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(ED)); std::vector enum_constants; for (auto* ECD : ED->enumerators()) { enum_constants.push_back((TCppScope_t)ECD); @@ -620,7 +531,7 @@ size_t GetSizeOfType(TCppType_t type) { return SizeOf(TT->getDecl()); // FIXME: Can we get the size of a non-tag type? - auto TI = getASTContext(type).getTypeInfo(QT); + auto TI = getSema().getASTContext().getTypeInfo(QT); size_t TypeSize = TI.Width; return TypeSize / 8; } @@ -645,8 +556,8 @@ std::string GetName(TCppType_t klass) { } std::string GetCompleteName(TCppType_t klass) { + auto& C = getSema().getASTContext(); auto* D = (Decl*)klass; - auto& C = getSema(D).getASTContext(); PrintingPolicy Policy = C.getPrintingPolicy(); Policy.SuppressUnwrittenScope = true; @@ -696,8 +607,8 @@ std::string GetQualifiedName(TCppType_t klass) { // FIXME: Figure out how to merge with GetCompleteName. std::string GetQualifiedCompleteName(TCppType_t klass) { + auto& C = getSema().getASTContext(); auto* D = (Decl*)klass; - auto& C = getSema(D).getASTContext(); if (auto* ND = llvm::dyn_cast_or_null(D)) { if (auto* TD = llvm::dyn_cast(ND)) { @@ -726,7 +637,6 @@ std::vector GetUsingNamespaces(TCppScope_t scope) { auto* D = (clang::Decl*)scope; if (auto* DC = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(D)); std::vector namespaces; for (auto UD : DC->using_directives()) { namespaces.push_back((TCppScope_t)UD->getNominatedNamespace()); @@ -814,14 +724,13 @@ TCppScope_t GetScopeFromCompleteName(const std::string& name) { TCppScope_t GetNamed(const std::string& name, TCppScope_t parent /*= nullptr*/) { clang::DeclContext* Within = 0; - auto* D = static_cast(parent); - LOCK(getInterpInfo(D)); if (parent) { + auto* D = (clang::Decl*)parent; D = GetUnderlyingScope(D); Within = llvm::dyn_cast(D); } - auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); + auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { return (TCppScope_t)(ND->getCanonicalDecl()); } @@ -851,13 +760,10 @@ TCppScope_t GetParentScope(TCppScope_t scope) { TCppIndex_t GetNumBases(TCppScope_t klass) { auto* D = (Decl*)klass; - if (auto* CTSD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CTSD)); + if (auto* CTSD = llvm::dyn_cast_or_null(D)) if (!CTSD->hasDefinition()) - compat::InstantiateClassTemplateSpecialization(getInterp(CTSD), CTSD); - } + compat::InstantiateClassTemplateSpecialization(getInterp(), CTSD); if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); if (CXXRD->hasDefinition()) return CXXRD->getNumBases(); } @@ -868,12 +774,8 @@ TCppIndex_t GetNumBases(TCppScope_t klass) { TCppScope_t GetBaseClass(TCppScope_t klass, TCppIndex_t ibase) { auto* D = (Decl*)klass; auto* CXXRD = llvm::dyn_cast_or_null(D); - if (!CXXRD) - return nullptr; - - LOCK(getInterpInfo(CXXRD)); - if (CXXRD->getNumBases() <= ibase) - return nullptr; + if (!CXXRD || CXXRD->getNumBases() <= ibase) + return 0; auto type = (CXXRD->bases_begin() + ibase)->getType(); if (auto RT = type->getAs()) @@ -962,15 +864,12 @@ int64_t GetBaseClassOffset(TCppScope_t derived, TCppScope_t base) { return -1; CXXRecordDecl* DCXXRD = cast(DD); CXXRecordDecl* BCXXRD = cast(BD); - - LOCK(getInterpInfo(DD)); - CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, /*DetectVirtual=*/false); DCXXRD->isDerivedFrom(BCXXRD, Paths); // FIXME: We might want to cache these requests as they seem expensive. - return ComputeBaseOffset(getSema(DD).getASTContext(), DCXXRD, Paths.front()); + return ComputeBaseOffset(getSema().getASTContext(), DCXXRD, Paths.front()); } template @@ -980,7 +879,6 @@ static void GetClassDecls(TCppScope_t klass, return; auto* D = (clang::Decl*)klass; - LOCK(getInterpInfo(D)); if (auto* TD = dyn_cast(D)) D = GetScopeFromType(TD->getUnderlyingType()); @@ -990,11 +888,11 @@ static void GetClassDecls(TCppScope_t klass, auto* CXXRD = dyn_cast(D); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp(CXXRD)); + cling::Interpreter::PushTransactionRAII RAII(&getInterp()); #endif // CPPINTEROP_USE_CLING if (CXXRD->hasDefinition()) CXXRD = CXXRD->getDefinition(); - getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); for (Decl* DI : CXXRD->decls()) { if (auto* MD = dyn_cast(DI)) methods.push_back(MD); @@ -1020,7 +918,7 @@ static void GetClassDecls(TCppScope_t klass, // Result is appended to the decls, i.e. CXXRD, iterator // non-shadowed decl will be push_back later // methods.push_back(Result); - getSema(CXXRD).findInheritingConstructor(SourceLocation(), CXXCD, CUSD); + getSema().findInheritingConstructor(SourceLocation(), CXXCD, CUSD); } } } @@ -1037,10 +935,8 @@ void GetFunctionTemplatedDecls(TCppScope_t klass, bool HasDefaultConstructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; - if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); + if (auto* CXXRD = llvm::dyn_cast_or_null(D)) return CXXRD->hasDefaultConstructor(); - } return false; } @@ -1051,21 +947,18 @@ TCppFunction_t GetDefaultConstructor(compat::Interpreter& interp, return nullptr; auto* CXXRD = (clang::CXXRecordDecl*)scope; - LOCK(getInterpInfo(CXXRD)); return interp.getCI()->getSema().LookupDefaultConstructor(CXXRD); } TCppFunction_t GetDefaultConstructor(TCppScope_t scope) { - auto* CXXRD = static_cast(scope); - return GetDefaultConstructor(getInterp(CXXRD), scope); + return GetDefaultConstructor(getInterp(), scope); } TCppFunction_t GetDestructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); - getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); return CXXRD->getDestructor(); } @@ -1084,14 +977,12 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, if (!scope || name.empty()) return {}; - LOCK(getInterpInfo(D)); - D = GetUnderlyingScope(D); std::vector funcs; llvm::StringRef Name(name); - auto& S = getSema(D); - DeclarationName DName = &S.getASTContext().Idents.get(name); + auto& S = getSema(); + DeclarationName DName = &getASTContext().Idents.get(name); clang::LookupResult R(S, DName, SourceLocation(), Sema::LookupOrdinaryName, For_Visible_Redeclaration); @@ -1112,7 +1003,6 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, TCppType_t GetFunctionReturnType(TCppFunction_t func) { auto* D = (clang::Decl*)func; if (auto* FD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(FD)); QualType Type = FD->getReturnType(); if (Type->isUndeducedAutoType()) { bool needInstantiation = false; @@ -1131,10 +1021,8 @@ TCppType_t GetFunctionReturnType(TCppFunction_t func) { return Type.getAsOpaquePtr(); } - if (auto* FD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(FD)); + if (auto* FD = llvm::dyn_cast_or_null(D)) return (FD->getTemplatedDecl())->getReturnType().getAsOpaquePtr(); - } return 0; } @@ -1162,9 +1050,9 @@ TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) { } TCppType_t GetFunctionArgType(TCppFunction_t func, TCppIndex_t iarg) { - auto* D = static_cast(func); + auto* D = (clang::Decl*)func; + if (auto* FD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(FD)); if (iarg < FD->getNumParams()) { auto* PVD = FD->getParamDecl(iarg); return PVD->getOriginalType().getAsOpaquePtr(); @@ -1190,7 +1078,7 @@ std::string GetFunctionSignature(TCppFunction_t func) { std::string Signature; raw_string_ostream SS(Signature); - PrintingPolicy Policy = getASTContext(D).getPrintingPolicy(); + PrintingPolicy Policy = getASTContext().getPrintingPolicy(); // Skip printing the body Policy.TerseOutput = true; Policy.FullyQualifiedName = true; @@ -1237,14 +1125,12 @@ bool IsTemplatedFunction(TCppFunction_t func) { // the template function exists and >1 means overloads bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) { DeclContext* Within = 0; - auto* D = static_cast(parent); if (parent) { + auto* D = (Decl*)parent; Within = llvm::dyn_cast(D); } - LOCK(getInterpInfo(D)); - - auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); + auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); if ((intptr_t)ND == (intptr_t)0) return false; @@ -1263,10 +1149,8 @@ void LookupConstructors(const std::string& name, TCppScope_t parent, auto* D = (Decl*)parent; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); - - getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); - DeclContextLookupResult Result = getSema(CXXRD).LookupConstructors(CXXRD); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); + DeclContextLookupResult Result = getSema().LookupConstructors(CXXRD); // Obtaining all constructors when we intend to lookup a method under a // scope can lead to crashes. We avoid that by accumulating constructors // only if the Decl matches the lookup name. @@ -1282,14 +1166,12 @@ bool GetClassTemplatedMethods(const std::string& name, TCppScope_t parent, if (!D && name.empty()) return false; - LOCK(getInterpInfo(D)); - // Accumulate constructors LookupConstructors(name, parent, funcs); - auto& S = getSema(D); + auto& S = getSema(); D = GetUnderlyingScope(D); llvm::StringRef Name(name); - DeclarationName DName = &S.getASTContext().Idents.get(name); + DeclarationName DName = &getASTContext().Idents.get(name); clang::LookupResult R(S, DName, SourceLocation(), Sema::LookupOrdinaryName, For_Visible_Redeclaration); auto* DC = clang::Decl::castToDeclContext(D); @@ -1325,16 +1207,11 @@ TCppFunction_t BestOverloadFunctionMatch(const std::vector& candidates, const std::vector& explicit_types, const std::vector& arg_types) { - if (candidates.empty()) - return nullptr; - InterpreterInfo& II = getInterpInfo(static_cast(candidates[0])); - LOCK(II); - - auto& S = II.Interpreter->getSema(); + auto& S = getSema(); auto& C = S.getASTContext(); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(II.Interpreter); + cling::Interpreter::PushTransactionRAII RAII(&getInterp()); #endif // The overload resolution interfaces in Sema require a list of expressions. @@ -1453,7 +1330,6 @@ bool IsDestructor(TCppConstFunction_t method) { bool IsStaticMethod(TCppConstFunction_t method) { const auto* D = static_cast(method); if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(D)); return CXXMD->isStatic(); } @@ -1473,7 +1349,7 @@ TCppFuncAddr_t GetFunctionAddress(const char* mangled_name) { static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) { const auto get_mangled_name = [](const FunctionDecl* FD) { - auto* MangleCtxt = getASTContext(FD).createMangleContext(); + auto MangleCtxt = getASTContext().createMangleContext(); if (!MangleCtxt->shouldMangleDeclName(FD)) { return FD->getNameInfo().getName().getAsString(); @@ -1500,7 +1376,6 @@ static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) { TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { auto* D = static_cast(method); if (auto* FD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(FD)); if ((IsTemplateInstantiationOrSpecialization(FD) || FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) && !FD->getDefinition()) @@ -1516,7 +1391,6 @@ TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { bool IsVirtualMethod(TCppFunction_t method) { auto* D = (Decl*)method; if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXMD)); return CXXMD->isVirtual(); } @@ -1527,9 +1401,7 @@ void GetDatamembers(TCppScope_t scope, std::vector& datamembers) { auto* D = (Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo(CXXRD)); - - getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); if (CXXRD->hasDefinition()) CXXRD = CXXRD->getDefinition(); @@ -1576,11 +1448,6 @@ void GetEnumConstantDatamembers(TCppScope_t scope, bool include_enum_class) { std::vector EDs; GetClassDecls(scope, EDs); - if (EDs.empty()) - return; - - LOCK(getInterpInfo(static_cast(EDs[0]))); - for (TCppScope_t i : EDs) { auto* ED = static_cast(i); @@ -1595,13 +1462,12 @@ void GetEnumConstantDatamembers(TCppScope_t scope, TCppScope_t LookupDatamember(const std::string& name, TCppScope_t parent) { clang::DeclContext* Within = 0; - auto* D = static_cast(parent); if (parent) { + auto* D = (clang::Decl*)parent; Within = llvm::dyn_cast(D); } - LOCK(getInterpInfo(D)); - auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); + auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { if (llvm::isa_and_nonnull(ND)) { return (TCppScope_t)ND; @@ -1643,6 +1509,9 @@ TCppType_t GetVariableType(TCppScope_t var) { intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, CXXRecordDecl* BaseCXXRD) { + if (!D) + return 0; + auto& C = I.getSema().getASTContext(); if (auto* FD = llvm::dyn_cast(D)) { @@ -1717,9 +1586,9 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, if (!address) { if (!VD->hasInit()) { #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp(VD)); + cling::Interpreter::PushTransactionRAII RAII(&getInterp()); #endif // CPPINTEROP_USE_CLING - getSema(VD).InstantiateVariableDefinition(SourceLocation(), VD); + getSema().InstantiateVariableDefinition(SourceLocation(), VD); VD = VD->getDefinition(); } if (VD->hasInit() && @@ -1750,11 +1619,8 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, intptr_t GetVariableOffset(TCppScope_t var, TCppScope_t parent) { auto* D = static_cast(var); - if (!D) - return 0; - LOCK(getInterpInfo(D)); auto* RD = llvm::dyn_cast_or_null(static_cast(parent)); - return GetVariableOffset(getInterp(D), D, RD); + return GetVariableOffset(getInterp(), D, RD); } // Check if the Access Specifier of the variable matches the provided value. @@ -1777,7 +1643,11 @@ bool IsPrivateVariable(TCppScope_t var) { bool IsStaticVariable(TCppScope_t var) { auto* D = (Decl*)var; - return llvm::isa_and_nonnull(D); + if (llvm::isa_and_nonnull(D)) { + return true; + } + + return false; } bool IsConstVariable(TCppScope_t var) { @@ -1801,7 +1671,7 @@ bool IsPODType(TCppType_t type) { if (QT.isNull()) return false; - return QT.isPODType(getASTContext(type)); + return QT.isPODType(getASTContext()); } bool IsPointerType(TCppType_t type) { @@ -1833,16 +1703,14 @@ bool IsRValueReferenceType(TCppType_t type) { TCppType_t GetPointerType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); - return getASTContext() - .getPointerType(QT) - .getAsOpaquePtr(); // FIXME: which ASTContext? + return getASTContext().getPointerType(QT).getAsOpaquePtr(); } TCppType_t GetReferencedType(TCppType_t type, bool rvalue) { QualType QT = QualType::getFromOpaquePtr(type); if (rvalue) - return getASTContext(type).getRValueReferenceType(QT).getAsOpaquePtr(); - return getASTContext(type).getLValueReferenceType(QT).getAsOpaquePtr(); + return getASTContext().getRValueReferenceType(QT).getAsOpaquePtr(); + return getASTContext().getLValueReferenceType(QT).getAsOpaquePtr(); } TCppType_t GetNonReferenceType(TCppType_t type) { @@ -2013,7 +1881,6 @@ TCppType_t GetType(const std::string& name) { if (!builtin.isNull()) return builtin.getAsOpaquePtr(); - LOCK(getInterpInfo()); auto* D = (Decl*)GetNamed(name, /* Within= */ 0); if (auto* TD = llvm::dyn_cast_or_null(D)) { return QualType(TD->getTypeForDecl(), 0).getAsOpaquePtr(); @@ -2025,8 +1892,7 @@ TCppType_t GetType(const std::string& name) { TCppType_t GetComplexType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); - LOCK(getInterpInfo(type)); - return getASTContext(type).getComplexType(QT).getAsOpaquePtr(); + return getASTContext().getComplexType(QT).getAsOpaquePtr(); } TCppType_t GetTypeFromScope(TCppScope_t klass) { @@ -2034,7 +1900,7 @@ TCppType_t GetTypeFromScope(TCppScope_t klass) { return 0; auto* D = (Decl*)klass; - ASTContext& C = getASTContext(D); + ASTContext& C = getASTContext(); if (ValueDecl* VD = dyn_cast(D)) return VD->getType().getAsOpaquePtr(); @@ -2425,8 +2291,8 @@ void make_narg_call(const FunctionDecl* FD, const std::string& return_type, // available, while there is a move constructor. // include utility header if not already included for std::move - DeclarationName DMove = &getASTContext(FD).Idents.get("move"); - auto result = getSema(FD).getStdNamespace()->lookup(DMove); + DeclarationName DMove = &getASTContext().Idents.get("move"); + auto result = getSema().getStdNamespace()->lookup(DMove); if (result.empty()) Cpp::Declare("#include "); @@ -2639,8 +2505,6 @@ void make_narg_call_with_return(compat::Interpreter& I, const FunctionDecl* FD, int get_wrapper_code(compat::Interpreter& I, const FunctionDecl* FD, std::string& wrapper_name, std::string& wrapper) { assert(FD && "generate_wrapper called without a function decl!"); - LOCK(getInterpInfo(FD)); - ASTContext& Context = FD->getASTContext(); // // Get the class or namespace name. @@ -3061,8 +2925,6 @@ JitCall::GenericCall make_wrapper(compat::Interpreter& I, const FunctionDecl* FD) { static std::map gWrapperStore; - LOCK(getInterpInfo(FD)); - auto R = gWrapperStore.find(FD); if (R != gWrapperStore.end()) return (JitCall::GenericCall)R->second; @@ -3155,8 +3017,6 @@ static JitCall::DestructorCall make_dtor_wrapper(compat::Interpreter& interp, static map gDtorWrapperStore; - LOCK(getInterpInfo(D)); - auto I = gDtorWrapperStore.find(D); if (I != gDtorWrapperStore.end()) return (JitCall::DestructorCall)I->second; @@ -3305,8 +3165,7 @@ CPPINTEROP_API JitCall MakeFunctionCallable(TInterp_t I, } CPPINTEROP_API JitCall MakeFunctionCallable(TCppConstFunction_t func) { - const auto* D = static_cast(func); - return MakeFunctionCallable(&getInterp(D), func); + return MakeFunctionCallable(&getInterp(), func); } namespace { @@ -3337,7 +3196,6 @@ static std::string MakeResourcesPath() { TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, const std::vector& GpuArgs /*={}*/) { - std::lock_guard Lock(InterpreterStackLock); std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr); std::string ResourceDir = MakeResourcesPath(); std::vector ClingArgv = {"-resource-dir", ResourceDir.c_str(), @@ -3413,52 +3271,34 @@ TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, } // namespace __internal_CppInterOp )"); - sInterpreters->emplace_back( - std::make_shared(I, /*Owned=*/true)); - sInterpreterASTMap->insert( - {&sInterpreters->back()->Interpreter->getSema().getASTContext(), - sInterpreters->back()}); + sInterpreters->emplace_back(I, /*Owned=*/true); return I; } bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { - std::lock_guard Lock(InterpreterStackLock); - if (!I) { - auto foundAST = - std::find_if(sInterpreterASTMap->begin(), sInterpreterASTMap->end(), - [](const auto& Item) { - return Item.second.lock() == sInterpreters->back(); - }); - 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; }); + [&I](const auto& Info) { return Info.Interpreter == 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; }); - sInterpreterASTMap->erase(foundAST); sInterpreters->erase(found); return true; } bool ActivateInterpreter(TInterp_t I) { - std::lock_guard Lock(InterpreterStackLock); - if (!I) return false; auto found = std::find_if(sInterpreters->begin(), sInterpreters->end(), - [&I](const auto& Info) { return Info->Interpreter == I; }); + [&I](const auto& Info) { return Info.Interpreter == I; }); if (found == sInterpreters->end()) return false; @@ -3469,22 +3309,18 @@ bool ActivateInterpreter(TInterp_t I) { } TInterp_t GetInterpreter() { - std::lock_guard Lock(InterpreterStackLock); if (sInterpreters->empty()) return nullptr; - return sInterpreters->back()->Interpreter; + return sInterpreters->back().Interpreter; } void UseExternalInterpreter(TInterp_t I) { - std::lock_guard Lock(InterpreterStackLock); assert(sInterpreters->empty() && "sInterpreter already in use!"); - sInterpreters->emplace_back( - std::make_shared(static_cast(I), - /*isOwned=*/false)); + sInterpreters->emplace_back(static_cast(I), + /*isOwned=*/false); } void AddSearchPath(const char* dir, bool isUser, bool prepend) { - LOCK(getInterpInfo()); getInterp().getDynamicLibraryManager()->addSearchPath(dir, isUser, prepend); } @@ -3548,10 +3384,7 @@ void DetectSystemCompilerIncludePaths(std::vector& Paths, exec(cmd.c_str(), Paths); } -void AddIncludePath(const char* dir) { - LOCK(getInterpInfo()); - getInterp().AddIncludePath(dir); -} +void AddIncludePath(const char* dir) { getInterp().AddIncludePath(dir); } void GetIncludePaths(std::vector& IncludePaths, bool withSystem, bool withFlags) { @@ -3588,14 +3421,10 @@ int Declare(compat::Interpreter& I, const char* code, bool silent) { } int Declare(const char* code, bool silent) { - LOCK(getInterpInfo()); return Declare(getInterp(), code, silent); } -int Process(const char* code) { - LOCK(getInterpInfo()); - return getInterp().process(code); -} +int Process(const char* code) { return getInterp().process(code); } intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/) { #ifdef CPPINTEROP_USE_CLING @@ -3607,7 +3436,6 @@ intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/) { if (HadError) *HadError = false; - LOCK(getInterpInfo()); auto res = getInterp().evaluate(code, V); if (res != 0) { // 0 is success if (HadError) @@ -3624,7 +3452,6 @@ std::string LookupLibrary(const char* lib_name) { } bool LoadLibrary(const char* lib_stem, bool lookup) { - LOCK(getInterpInfo()); compat::Interpreter::CompilationResult res = getInterp().loadLibrary(lib_stem, lookup); @@ -3632,13 +3459,11 @@ bool LoadLibrary(const char* lib_stem, bool lookup) { } void UnloadLibrary(const char* lib_stem) { - LOCK(getInterpInfo()); getInterp().getDynamicLibraryManager()->unloadLibrary(lib_stem); } std::string SearchLibrariesForSymbol(const char* mangled_name, bool search_system /*true*/) { - LOCK(getInterpInfo()); auto* DLM = getInterp().getDynamicLibraryManager(); return DLM->searchLibrariesForSymbol(mangled_name, search_system); } @@ -3724,20 +3549,16 @@ bool InsertOrReplaceJitSymbol(compat::Interpreter& I, bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, uint64_t address) { - LOCK(getInterpInfo()); return InsertOrReplaceJitSymbol(getInterp(), linker_mangled_name, address); } std::string ObjToString(const char* type, void* obj) { - LOCK(getInterpInfo(NULLPTR)); // FIXME: not enough information to lock the - // current interpreter - return getInterp(NULLPTR).toString(type, obj); + return getInterp().toString(type, obj); } -Decl* InstantiateTemplate(TemplateDecl* TemplateD, - TemplateArgumentListInfo& TLI, Sema& S, - bool instantiate_body) { - LOCK(getInterpInfo()); +static Decl* InstantiateTemplate(TemplateDecl* TemplateD, + TemplateArgumentListInfo& TLI, Sema& S, + bool instantiate_body) { // This is not right but we don't have a lot of options to choose from as a // template instantiation requires a valid source location. SourceLocation fakeLoc = GetValidSLoc(S); @@ -3832,16 +3653,13 @@ TCppScope_t InstantiateTemplate(TCppScope_t tmpl, const TemplateArgInfo* template_args, size_t template_args_size, bool instantiate_body) { - auto* D = static_cast(tmpl); - LOCK(getInterpInfo(D)); - return InstantiateTemplate(getInterp(D), tmpl, template_args, + return InstantiateTemplate(getInterp(), tmpl, template_args, template_args_size, instantiate_body); } void GetClassTemplateInstantiationArgs(TCppScope_t templ_instance, std::vector& args) { auto* CTSD = static_cast(templ_instance); - LOCK(getInterpInfo(CTSD)); for (const auto& TA : CTSD->getTemplateInstantiationArgs().asArray()) { switch (TA.getKind()) { default: @@ -3873,7 +3691,6 @@ InstantiateTemplateFunctionFromString(const char* function_template) { std::string id = "__Cppyy_GetMethTmpl_" + std::to_string(var_count++); std::string instance = "auto " + id + " = " + function_template + ";\n"; - LOCK(getInterpInfo()); if (!Cpp::Declare(instance.c_str(), /*silent=*/false)) { VarDecl* VD = (VarDecl*)Cpp::GetNamed(id, 0); DeclRefExpr* DRE = (DeclRefExpr*)VD->getInit()->IgnoreImpCasts(); @@ -3887,7 +3704,6 @@ void GetAllCppNames(TCppScope_t scope, std::set& names) { clang::DeclContext* DC; clang::DeclContext::decl_iterator decl; - LOCK(getInterpInfo(D)); if (auto* TD = dyn_cast_or_null(D)) { DC = clang::TagDecl::castToDeclContext(TD); decl = DC->decls_begin(); @@ -3917,7 +3733,6 @@ void GetEnums(TCppScope_t scope, std::vector& Result) { auto* DC = llvm::dyn_cast(D); - LOCK(getInterpInfo(D)); llvm::SmallVector DCs; DC->collectAllContexts(DCs); @@ -3960,15 +3775,13 @@ std::vector GetDimensions(TCppType_t type) { } bool IsTypeDerivedFrom(TCppType_t derived, TCppType_t base) { + auto& S = getSema(); + auto fakeLoc = GetValidSLoc(S); auto derivedType = clang::QualType::getFromOpaquePtr(derived); auto baseType = clang::QualType::getFromOpaquePtr(base); - auto* CXXRD = baseType->getAsRecordDecl(); - LOCK(getInterpInfo(CXXRD)); - auto& S = getSema(CXXRD); - auto fakeLoc = GetValidSLoc(S); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp(CXXRD)); + cling::Interpreter::PushTransactionRAII RAII(&getInterp()); #endif return S.IsDerivedFrom(fakeLoc, derivedType, baseType); } @@ -3978,7 +3791,6 @@ std::string GetFunctionArgDefault(TCppFunction_t func, auto* D = (clang::Decl*)func; clang::ParmVarDecl* PI = nullptr; - LOCK(getInterpInfo(D)); if (auto* FD = llvm::dyn_cast_or_null(D)) PI = FD->getParamDecl(param_index); @@ -4074,7 +3886,6 @@ OperatorArity GetOperatorArity(TCppFunction_t op) { void GetOperator(TCppScope_t scope, Operator op, std::vector& operators, OperatorArity kind) { Decl* D = static_cast(scope); - LOCK(getInterpInfo(D)); if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { auto fn = [&operators, kind, op](const RecordDecl* RD) { ASTContext& C = RD->getASTContext(); @@ -4090,7 +3901,7 @@ void GetOperator(TCppScope_t scope, Operator op, fn(CXXRD); CXXRD->forallBases(fn); } else if (auto* DC = llvm::dyn_cast_or_null(D)) { - ASTContext& C = getSema(D).getASTContext(); + ASTContext& C = getSema().getASTContext(); DeclContextLookupResult Result = DC->lookup(C.DeclarationNames.getCXXOperatorName( (clang::OverloadedOperatorKind)op)); @@ -4139,8 +3950,7 @@ TCppObject_t Construct(compat::Interpreter& interp, TCppScope_t scope, TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/, TCppIndex_t count /*=1UL*/) { - auto* D = static_cast(scope); - return Construct(getInterp(D), scope, arena, count); + return Construct(getInterp(), scope, arena, count); } bool Destruct(compat::Interpreter& interp, TCppObject_t This, const Decl* Class, @@ -4156,7 +3966,7 @@ bool Destruct(compat::Interpreter& interp, TCppObject_t This, const Decl* Class, bool Destruct(TCppObject_t This, TCppConstScope_t scope, bool withFree /*=true*/, TCppIndex_t count /*=0UL*/) { const auto* Class = static_cast(scope); - return Destruct(getInterp(Class), This, Class, withFree, count); + return Destruct(getInterp(), This, Class, withFree, count); } class StreamCaptureInfo { @@ -4260,9 +4070,7 @@ std::string EndStdStreamCapture() { void CodeComplete(std::vector& Results, const char* code, unsigned complete_line /* = 1U */, unsigned complete_column /* = 1U */) { - LOCK(getInterpInfo(NULLPTR)); // FIXME: Not enough info to lock the current - // interpreter - compat::codeComplete(Results, getInterp(NULLPTR), code, complete_line, + compat::codeComplete(Results, getInterp(), code, complete_line, complete_column); } diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index eb9842681..e9b82ea1d 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/unittests/CppInterOp/InterpreterTest.cpp @@ -361,54 +361,3 @@ if (llvm::sys::RunningOnValgrind()) delete ExtInterp; #endif } - -TEST(InterpreterTest, MultipleInterpreter) { -#if CLANG_VERSION_MAJOR < 20 && defined(EMSCRIPTEN) - GTEST_SKIP() << "Test fails for Emscipten LLVM 20 builds"; -#endif - - EXPECT_TRUE(Cpp::CreateInterpreter()); - Cpp::Declare(R"( - void f() {} - )"); - Cpp::TCppScope_t f = Cpp::GetNamed("f"); - - EXPECT_TRUE(Cpp::CreateInterpreter()); - Cpp::Declare(R"( - void ff() {} - )"); - Cpp::TCppScope_t ff = Cpp::GetNamed("ff"); - - auto f_callable = Cpp::MakeFunctionCallable(f); - EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); - auto ff_callable = Cpp::MakeFunctionCallable(ff); - EXPECT_EQ(ff_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); -} - -TEST(InterpreterTest, ASMParsing) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test fails for Emscipten builds"; -#endif -#ifdef _WIN32 - GTEST_SKIP() << "Disabled on Windows. Needs fixing."; -#endif - if (llvm::sys::RunningOnValgrind()) - GTEST_SKIP() << "XFAIL due to Valgrind report"; - if (!IsTargetX86()) - GTEST_SKIP() << "Skipped on ARM, we test ASM for x86_64"; - std::vector interpreter_args = {"-include", "new"}; - auto* I = Cpp::CreateInterpreter(interpreter_args); - EXPECT_TRUE(I); - - EXPECT_TRUE(Cpp::Declare(R"( - void foo(int &input) { - __asm__ volatile ("addl $10, %0" : "+r"(input)); - } - )", - I) == 0); - - bool hasError; - EXPECT_TRUE(Cpp::Process("int b = 42; foo(b);") == 0); - EXPECT_TRUE(Cpp::Evaluate("b", &hasError) == 52); - EXPECT_FALSE(hasError); -}