Skip to content

Commit 74bcb4a

Browse files
Revert "Add tests and dev-doc"
This reverts commit c42c907.
1 parent e1e07e0 commit 74bcb4a

File tree

10 files changed

+24
-221
lines changed

10 files changed

+24
-221
lines changed

docs/DevelopersDocumentation.rst

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -423,54 +423,12 @@ library files and run pytest:
423423
python -m pip install pytest
424424
python -m pytest -sv
425425
426-
###################################
426+
***********************************
427427
CppInterOp Internal Documentation
428-
###################################
428+
***********************************
429429

430430
CppInterOp maintains an internal Doxygen documentation of its components.
431431
Internal documentation aims to capture intrinsic details and overall usage of
432432
code components. The goal of internal documentation is to make the codebase
433433
easier to understand for the new developers. Internal documentation can be
434434
visited : `here <build/html/index.html>`_
435-
436-
**************************************
437-
Multiple Interpreter & Thread-Safety
438-
**************************************
439-
440-
CppInterOp allows the user to create multiple interpreters at a time and
441-
use those interpreters. The interpreters that are created are stored in a
442-
stack and a map. The stack is used to enable the model where the user
443-
wants to create a temporary interpreter and destroy it after performing a
444-
few operations. In such a use case, the top of the stack is the only
445-
interpreter in use at any given point in time.
446-
447-
The map is used to store the mapping from :code:`clang::ASTContext` to
448-
:code:`Cpp::InterpreterInfo`. This is required to figure out which
449-
interpreter an object belongs to. Say the library user performs the
450-
following operations:
451-
452-
1. Create an Interpreter
453-
2. Compile some code with variable :code:`a`
454-
3. Create another Interpreter
455-
4. Performs :code:`Cpp::GetVariableOffset(a)`
456-
457-
In step 4, the top of the stack is an interpreter without the definition of
458-
:code:`a`. And we cannot use it to figure out the address of :code:`a`.
459-
The :code:`clang::Decl` passed to :code:`Cpp::GetVariableOffset` is used to
460-
retrieve the :code:`clang::ASTContext`, using
461-
:code:`clang::Decl::getASTContext`. We then use the map to figure out the
462-
exact Interpreter Instance this :code:`clang::Decl` belongs to and perform
463-
the operation.
464-
465-
A shortcoming of this is that if the CppInterOp accepts a
466-
:code:`clang::QualType` instead of :code:`clang::Decl`, then it is not
467-
possible to get the :code:`clang::ASTContext` from the :code:`clang::QualType`.
468-
In such cases, we iterate over the Allocator of all the Interpreters in our
469-
stack and figure out which :code:`clang::ASTContext` allocated this
470-
:code:`clang::QualType`. This is a very expensive operation. But there is no
471-
alternative to this.
472-
473-
For **thread-safety**, we introduce a lock for each of the interpreters we
474-
create. And lock only that one specific interpreter when required. We also
475-
have 2 global locks, one for LLVM, and another is used to lock operations
476-
performed on the interpreter stack and the map itself.

lib/CppInterOp/CppInterOp.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3459,8 +3459,6 @@ static inline auto find_interpreter_in_map(InterpreterInfo* I) {
34593459
bool DeleteInterpreter(TInterp_t I /*=nullptr*/) {
34603460
std::lock_guard<std::recursive_mutex> Lock(InterpreterStackLock);
34613461
assert(sInterpreters->size() == sInterpreterASTMap->size());
3462-
if (sInterpreters->empty())
3463-
return false;
34643462

34653463
if (!I) {
34663464
auto foundAST = find_interpreter_in_map(sInterpreters->back().get());

unittests/CppInterOp/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,6 @@ set_output_directory(DynamicLibraryManagerTests
128128
)
129129

130130
add_dependencies(DynamicLibraryManagerTests TestSharedLib)
131-
add_dependencies(DynamicLibraryManagerTests TestSharedLib2)
132131

133132
#export_executable_symbols_for_plugins(TestSharedLib)
134133
add_subdirectory(TestSharedLib)
135-
add_subdirectory(TestSharedLib2)

unittests/CppInterOp/DynamicLibraryManagerTest.cpp

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ std::string GetExecutablePath(const char* Argv0) {
1919
return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
2020
}
2121

22-
TEST(DynamicLibraryManagerTest, Sanity1) {
22+
TEST(DynamicLibraryManagerTest, Sanity) {
2323
#ifdef EMSCRIPTEN
2424
GTEST_SKIP() << "Test fails for Emscipten builds";
2525
#endif
@@ -66,55 +66,6 @@ TEST(DynamicLibraryManagerTest, Sanity1) {
6666
// EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
6767
}
6868

69-
TEST(DynamicLibraryManagerTest, Sanity2) {
70-
#ifdef EMSCRIPTEN
71-
GTEST_SKIP() << "Test fails for Emscipten builds";
72-
#endif
73-
74-
#if CLANG_VERSION_MAJOR == 18 && defined(CPPINTEROP_USE_CLING) && \
75-
defined(_WIN32)
76-
GTEST_SKIP() << "Test fails with Cling on Windows";
77-
#endif
78-
79-
auto* I = Cpp::CreateInterpreter();
80-
EXPECT_TRUE(I);
81-
EXPECT_FALSE(Cpp::GetFunctionAddress("ret_one", I));
82-
83-
std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr);
84-
llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
85-
Cpp::AddSearchPath(Dir.str().c_str(), true, false, I);
86-
87-
// FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format
88-
// adds an additional underscore (_) prefix to the lowered names. Figure out
89-
// how to harmonize that API.
90-
#ifdef __APPLE__
91-
std::string PathToTestSharedLib =
92-
Cpp::SearchLibrariesForSymbol("_ret_one", /*system_search=*/false, I);
93-
#else
94-
std::string PathToTestSharedLib =
95-
Cpp::SearchLibrariesForSymbol("ret_one", /*system_search=*/false, I);
96-
#endif // __APPLE__
97-
98-
EXPECT_STRNE("", PathToTestSharedLib.c_str())
99-
<< "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str()
100-
<< "'";
101-
102-
EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str(), true, I));
103-
// Force ExecutionEngine to be created.
104-
Cpp::Process("", I);
105-
Cpp::Declare("", I);
106-
// FIXME: Conda returns false to run this code on osx.
107-
#ifndef __APPLE__
108-
EXPECT_TRUE(Cpp::GetFunctionAddress("ret_one", I));
109-
#endif //__APPLE__
110-
111-
Cpp::UnloadLibrary("TestSharedLib2", I);
112-
// We have no reliable way to check if it was unloaded because posix does not
113-
// require the library to be actually unloaded but just the handle to be
114-
// invalidated...
115-
// EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
116-
}
117-
11869
TEST(DynamicLibraryManagerTest, BasicSymbolLookup) {
11970
#ifndef EMSCRIPTEN
12071
GTEST_SKIP() << "This test is only intended for Emscripten builds.";

unittests/CppInterOp/FunctionReflectionTest.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,8 +1014,6 @@ TEST(FunctionReflectionTest, BestOverloadFunctionMatch1) {
10141014
GetAllTopLevelDecls(code, Decls);
10151015
std::vector<Cpp::TCppFunction_t> candidates;
10161016

1017-
EXPECT_FALSE(Cpp::BestOverloadFunctionMatch({}, {}, {}));
1018-
10191017
for (auto decl : Decls)
10201018
if (Cpp::IsTemplatedFunction(decl)) candidates.push_back((Cpp::TCppFunction_t)decl);
10211019

unittests/CppInterOp/InterpreterTest.cpp

Lines changed: 21 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,13 @@
1717
#include "clang-c/CXCppInterOp.h"
1818

1919
#include "llvm/ADT/SmallString.h"
20-
#include "llvm/Support/FileSystem.h"
2120
#include "llvm/Support/Path.h"
22-
#include "llvm/Support/raw_ostream.h"
21+
#include <llvm/Support/FileSystem.h>
2322

2423
#include <gmock/gmock.h>
2524
#include "gtest/gtest.h"
2625

2726
#include <algorithm>
28-
#include <cstdint>
29-
#include <thread>
3027
#include <utility>
3128

3229
using ::testing::StartsWith;
@@ -250,7 +247,7 @@ TEST(InterpreterTest, DISABLED_DetectResourceDir) {
250247
#ifdef _WIN32
251248
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
252249
#endif
253-
auto* I = Cpp::CreateInterpreter();
250+
Cpp::CreateInterpreter();
254251
EXPECT_STRNE(Cpp::DetectResourceDir().c_str(), Cpp::GetResourceDir());
255252
llvm::SmallString<256> Clang(LLVM_BINARY_DIR);
256253
llvm::sys::path::append(Clang, "bin", "clang");
@@ -259,7 +256,7 @@ TEST(InterpreterTest, DISABLED_DetectResourceDir) {
259256
GTEST_SKIP() << "Test not run (Clang binary does not exist)";
260257

261258
std::string DetectedPath = Cpp::DetectResourceDir(Clang.str().str().c_str());
262-
EXPECT_STREQ(DetectedPath.c_str(), Cpp::GetResourceDir(I));
259+
EXPECT_STREQ(DetectedPath.c_str(), Cpp::GetResourceDir());
263260
}
264261

265262
TEST(InterpreterTest, DetectSystemCompilerIncludePaths) {
@@ -367,86 +364,28 @@ if (llvm::sys::RunningOnValgrind())
367364
#endif
368365
}
369366

370-
static int printf_jit(const char* format, ...) {
371-
llvm::errs() << "printf_jit called!\n";
372-
return 0;
373-
}
374-
375367
TEST(InterpreterTest, MultipleInterpreter) {
376-
#ifdef EMSCRIPTEN
377-
GTEST_SKIP() << "Test fails for Emscipten builds";
378-
#endif
379-
#ifdef _WIN32
380-
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
368+
#if CLANG_VERSION_MAJOR < 20 && defined(EMSCRIPTEN)
369+
GTEST_SKIP() << "Test fails for Emscipten LLVM 20 builds";
381370
#endif
382-
GTEST_SKIP() << "Test does not consistently pass so skipping for now";
383-
// delete all old interpreters
384-
while (Cpp::DeleteInterpreter())
385-
;
386-
std::vector<const char*> interpreter_args = {"-include", "new"};
387-
auto* I1 = Cpp::CreateInterpreter(interpreter_args);
388-
EXPECT_TRUE(I1);
389-
390-
auto F = [](Cpp::TInterp_t I) {
391-
bool hasError = true;
392-
EXPECT_TRUE(Cpp::Evaluate("__cplusplus", &hasError, I) == 201402);
393-
EXPECT_FALSE(hasError);
394-
395-
Cpp::Declare(R"(
396-
#include <new>
397-
extern "C" int printf(const char*,...);
398-
class MyKlass {};
399-
)",
400-
false, I);
401-
Cpp::TCppScope_t f = Cpp::GetNamed("printf", Cpp::GetGlobalScope(I));
402-
EXPECT_TRUE(f);
403-
EXPECT_TRUE(Cpp::GetComplexType(Cpp::GetType("int", I)));
404-
Cpp::TCppType_t MyKlass = Cpp::GetType("MyKlass", I);
405-
EXPECT_EQ(Cpp::GetTypeAsString(MyKlass), "MyKlass");
406-
EXPECT_EQ(Cpp::GetNumBases(MyKlass), 0);
407-
EXPECT_FALSE(Cpp::GetBaseClass(MyKlass, 3));
408-
std::vector<Cpp::TCppScope_t> members;
409-
Cpp::GetEnumConstantDatamembers(Cpp::GetScopeFromType(MyKlass), members);
410-
EXPECT_EQ(members.size(), 0);
411-
412-
EXPECT_FALSE(
413-
Cpp::InsertOrReplaceJitSymbol("printf", (uint64_t)&printf_jit, I));
414-
415-
auto f_callable = Cpp::MakeFunctionCallable(f);
416-
EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall);
417-
418-
EXPECT_FALSE(
419-
Cpp::TakeInterpreter((TInterp_t)1)); // try to take ownership of an
420-
// interpreter that does not exist
421-
422-
std::vector<std::string> includes;
423-
Cpp::AddIncludePath("/non/existent/", I);
424-
Cpp::GetIncludePaths(includes, false, false, I);
425-
EXPECT_NE(std::find(includes.begin(), includes.end(), "/non/existent/"),
426-
std::end(includes));
427-
428-
EXPECT_TRUE(Cpp::InsertOrReplaceJitSymbol("non_existent", (uint64_t)&f, I));
429-
};
430-
F(I1);
371+
auto* I = Cpp::CreateInterpreter();
372+
EXPECT_TRUE(I);
373+
Cpp::Declare(R"(
374+
void f() {}
375+
)");
376+
Cpp::TCppScope_t f = Cpp::GetNamed("f");
431377

432-
auto* I2 = Cpp::CreateInterpreter(interpreter_args);
433-
auto* I3 = Cpp::CreateInterpreter(interpreter_args);
434-
auto* I4 = Cpp::CreateInterpreter(interpreter_args);
378+
auto* I2 = Cpp::CreateInterpreter();
435379
EXPECT_TRUE(I2);
436-
EXPECT_TRUE(I3);
437-
EXPECT_TRUE(I4);
438-
439-
std::thread t2(F, I2);
440-
std::thread t3(F, I3);
441-
std::thread t4(F, I4);
442-
t2.join();
443-
t3.join();
444-
t4.join();
445-
446-
testing::internal::CaptureStderr();
447-
Cpp::Process("printf(\"Blah\");", I2);
448-
std::string cerrs = testing::internal::GetCapturedStderr();
449-
EXPECT_STREQ(cerrs.c_str(), "printf_jit called!\n");
380+
Cpp::Declare(R"(
381+
void ff() {}
382+
)");
383+
Cpp::TCppScope_t ff = Cpp::GetNamed("ff");
384+
385+
auto f_callable = Cpp::MakeFunctionCallable(f);
386+
EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall);
387+
auto ff_callable = Cpp::MakeFunctionCallable(ff);
388+
EXPECT_EQ(ff_callable.getKind(), Cpp::JitCall::Kind::kGenericCall);
450389
}
451390

452391
TEST(InterpreterTest, ASMParsing) {

unittests/CppInterOp/TestSharedLib2/CMakeLists.txt

Lines changed: 0 additions & 23 deletions
This file was deleted.

unittests/CppInterOp/TestSharedLib2/TestSharedLib.cpp

Lines changed: 0 additions & 3 deletions
This file was deleted.

unittests/CppInterOp/TestSharedLib2/TestSharedLib.h

Lines changed: 0 additions & 11 deletions
This file was deleted.

unittests/CppInterOp/VariableReflectionTest.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,6 @@ TEST(VariableReflectionTest, GetVariableOffset) {
287287
std::vector<Cpp::TCppScope_t> datamembers;
288288
Cpp::GetDatamembers(Decls[4], datamembers);
289289

290-
EXPECT_FALSE((bool)Cpp::GetVariableOffset(nullptr));
291-
292290
EXPECT_TRUE((bool) Cpp::GetVariableOffset(Decls[0])); // a
293291
EXPECT_TRUE((bool) Cpp::GetVariableOffset(Decls[1])); // N
294292
EXPECT_TRUE((bool)Cpp::GetVariableOffset(Decls[2])); // S

0 commit comments

Comments
 (0)