Skip to content

Commit 9da7679

Browse files
committed
Add support for an external interpreter
The new interface can be called by an external library like ROOT, that manages it's own `TInterpreter` instance. In this case the `cling::Interpreter*` initialised by TCling is passed to InterOp and a flag indicates that InterOp does not have ownership
1 parent 4e4400e commit 9da7679

File tree

3 files changed

+69
-7
lines changed

3 files changed

+69
-7
lines changed

include/clang/Interpreter/CppInterOp.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,13 @@ namespace Cpp {
530530
///\returns the current interpreter instance, if any.
531531
CPPINTEROP_API TInterp_t GetInterpreter();
532532

533+
/// Sets the Interpreter instance with an external interpreter, meant to
534+
/// be called by an external library that manages it's own interpreter.
535+
/// Sets a flag signifying CppInterOp does not have ownership of the
536+
/// sInterpreter.
537+
///\param[in] Args - the pointer to an external interpreter
538+
CPPINTEROP_API void UseExternalInterpreter(TInterp_t I);
539+
533540
/// Adds a Search Path for the Interpreter to get the libraries.
534541
CPPINTEROP_API void AddSearchPath(const char* dir, bool isUser = true,
535542
bool prepend = false);

lib/Interpreter/CppInterOp.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,22 @@ namespace Cpp {
6060
using namespace llvm;
6161
using namespace std;
6262

63-
static std::unique_ptr<compat::Interpreter> sInterpreter;
63+
// Flag to indicate ownership when an external interpreter instance is used.
64+
static bool OwningSInterpreter = true;
65+
static compat::Interpreter* sInterpreter = nullptr;
6466
// Valgrind complains about __cxa_pure_virtual called when deleting
6567
// llvm::SectionMemoryManager::~SectionMemoryManager as part of the dtor chain
6668
// of the Interpreter.
6769
// This might fix the issue https://reviews.llvm.org/D107087
6870
// FIXME: For now we just leak the Interpreter.
6971
struct InterpDeleter {
70-
~InterpDeleter() { sInterpreter.release(); }
72+
~InterpDeleter() = default;
7173
} Deleter;
7274

7375
static compat::Interpreter& getInterp() {
74-
assert(sInterpreter.get() && "Must be set before calling this!");
75-
return *sInterpreter.get();
76+
assert(sInterpreter &&
77+
"Interpreter instance must be set before calling this!");
78+
return *sInterpreter;
7679
}
7780
static clang::Sema& getSema() { return getInterp().getCI()->getSema(); }
7881
static clang::ASTContext& getASTContext() { return getSema().getASTContext(); }
@@ -2691,12 +2694,16 @@ namespace Cpp {
26912694
// FIXME: Enable this assert once we figure out how to fix the multiple
26922695
// calls to CreateInterpreter.
26932696
//assert(!sInterpreter && "Interpreter already set.");
2694-
sInterpreter.reset(I);
2697+
sInterpreter = I;
26952698
return I;
26962699
}
26972700

2698-
TInterp_t GetInterpreter() {
2699-
return sInterpreter.get();
2701+
TInterp_t GetInterpreter() { return sInterpreter; }
2702+
2703+
void UseExternalInterpreter(TInterp_t I) {
2704+
assert(sInterpreter && "sInterpreter already in use!");
2705+
sInterpreter = static_cast<compat::Interpreter*>(I);
2706+
OwningSInterpreter = false;
27002707
}
27012708

27022709
void AddSearchPath(const char *dir, bool isUser,

unittests/CppInterOp/InterpreterTest.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
1+
12
#include "Utils.h"
23

34
#include "clang/Interpreter/CppInterOp.h"
5+
6+
#ifdef USE_CLING
7+
#include "cling/Interpreter/Interpreter.h"
8+
#endif // USE_CLING
9+
10+
#ifdef USE_REPL
11+
#include "clang/Interpreter/Interpreter.h"
12+
#endif // USE_REPL
13+
414
#include "clang/Basic/Version.h"
515

616
#include "llvm/ADT/SmallString.h"
@@ -162,3 +172,41 @@ TEST(InterpreterTest, CodeCompletion) {
162172
GTEST_SKIP();
163173
#endif
164174
}
175+
176+
TEST(InterpreterTest, ExternalInterpreterTest) {
177+
178+
#ifdef USE_REPL
179+
llvm::ExitOnError ExitOnErr;
180+
clang::IncrementalCompilerBuilder CB;
181+
CB.SetCompilerArgs({"-std=c++20"});
182+
183+
// Create the incremental compiler instance.
184+
std::unique_ptr<clang::CompilerInstance> CI;
185+
CI = ExitOnErr(CB.CreateCpp());
186+
187+
// Create the interpreter instance.
188+
std::unique_ptr<clang::Interpreter> I =
189+
ExitOnErr(clang::Interpreter::create(std::move(CI)));
190+
auto ExtInterp = I.get();
191+
#endif
192+
193+
#ifdef USE_CLING
194+
std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr);
195+
std::string ResourceDir = compat::MakeResourceDir(LLVM_BINARY_DIR);
196+
std::vector<const char *> ClingArgv = {"-resource-dir", ResourceDir.c_str(),
197+
"-std=c++14"};
198+
ClingArgv.insert(ClingArgv.begin(), MainExecutableName.c_str());
199+
auto *ExtInterp = new compat::Interpreter(ClingArgv.size(), &ClingArgv[0]);
200+
#endif
201+
202+
EXPECT_NE(ExtInterp, nullptr);
203+
Cpp::UseExternalInterpreter(ExtInterp);
204+
EXPECT_TRUE(Cpp::GetInterpreter()) << "External Interpreter not set";
205+
206+
#ifdef USE_REPL
207+
I.release();
208+
#endif
209+
#ifdef USE_CLING
210+
delete ExtInterp;
211+
#endif
212+
}

0 commit comments

Comments
 (0)