Skip to content

Commit 721d33b

Browse files
authored
Add error propagation to interpreter creation logic (#553)
* Fixing creation of Interpreter
1 parent 208c36a commit 721d33b

File tree

6 files changed

+63
-11
lines changed

6 files changed

+63
-11
lines changed

lib/Interpreter/CXCppInterOp.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,15 @@ static inline compat::Interpreter* getInterpreter(const CXInterpreterImpl* I) {
271271
}
272272

273273
CXInterpreter clang_createInterpreter(const char* const* argv, int argc) {
274-
auto* I = new CXInterpreterImpl(); // NOLINT(*-owning-memory)
274+
auto I = std::make_unique<CXInterpreterImpl>();
275+
#ifdef CPPINTEROP_USE_CLING
275276
I->Interp = std::make_unique<compat::Interpreter>(argc, argv);
277+
#else
278+
I->Interp = compat::Interpreter::create(argc, argv);
279+
if (!I->Interp) {
280+
return nullptr;
281+
}
282+
#endif
276283
// create a bridge between CXTranslationUnit and clang::Interpreter
277284
// auto AU = std::make_unique<ASTUnit>(false);
278285
// AU->FileMgr = I->Interp->getCompilerInstance().getFileManager();
@@ -281,7 +288,7 @@ CXInterpreter clang_createInterpreter(const char* const* argv, int argc) {
281288
// AU->Ctx = &I->Interp->getSema().getASTContext();
282289
// I->TU.reset(MakeCXTranslationUnit(static_cast<CIndexer*>(clang_createIndex(0,
283290
// 0)), AU));
284-
return I;
291+
return I.release();
285292
}
286293

287294
CXInterpreter clang_createInterpreterFromRawPtr(TInterp_t I) {

lib/Interpreter/Compatibility.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,11 @@ createClangInterpreter(std::vector<const char*>& args) {
294294
return nullptr;
295295
}
296296
if (CudaEnabled) {
297-
if (auto Err = (*innerOrErr)->LoadDynamicLibrary("libcudart.so"))
297+
if (auto Err = (*innerOrErr)->LoadDynamicLibrary("libcudart.so")) {
298298
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
299299
"Failed load libcudart.so runtime:");
300+
return nullptr;
301+
}
300302
}
301303

302304
return std::move(*innerOrErr);

lib/Interpreter/CppInterOp.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,7 +2957,15 @@ namespace Cpp {
29572957
std::back_inserter(ClingArgv),
29582958
[&](const std::string& str) { return str.c_str(); });
29592959

2960+
#ifdef CPPINTEROP_USE_CLING
29602961
auto I = new compat::Interpreter(ClingArgv.size(), &ClingArgv[0]);
2962+
#else
2963+
auto Interp = compat::Interpreter::create(
2964+
static_cast<int>(ClingArgv.size()), ClingArgv.data());
2965+
if (!Interp)
2966+
return nullptr;
2967+
auto* I = Interp.release();
2968+
#endif
29612969

29622970
// Honor -mllvm.
29632971
//

lib/Interpreter/CppInterOpInterpreter.h

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#if CLANG_VERSION_MAJOR >= 19
2626
#include "clang/Sema/Redeclaration.h"
2727
#endif
28+
#include "clang/Serialization/ModuleFileExtension.h"
2829

2930
#include "llvm/ADT/DenseMap.h"
3031
#include "llvm/ADT/SmallSet.h"
@@ -34,6 +35,10 @@
3435
#include "llvm/Support/Compiler.h"
3536
#include "llvm/Support/Error.h"
3637
#include "llvm/Support/TargetSelect.h"
38+
#include "llvm/Support/raw_ostream.h"
39+
40+
#include <utility>
41+
#include <vector>
3742

3843
namespace clang {
3944
class CompilerInstance;
@@ -136,19 +141,28 @@ class Interpreter {
136141
private:
137142
std::unique_ptr<clang::Interpreter> inner;
138143

144+
Interpreter(std::unique_ptr<clang::Interpreter> CI) : inner(std::move(CI)) {}
145+
139146
public:
140-
Interpreter(int argc, const char* const* argv, const char* llvmdir = 0,
141-
const std::vector<std::shared_ptr<clang::ModuleFileExtension>>&
142-
moduleExtensions = {},
143-
void* extraLibHandle = 0, bool noRuntime = true) {
147+
static std::unique_ptr<Interpreter>
148+
create(int argc, const char* const* argv, const char* llvmdir = nullptr,
149+
const std::vector<std::shared_ptr<clang::ModuleFileExtension>>&
150+
moduleExtensions = {},
151+
void* extraLibHandle = nullptr, bool noRuntime = true) {
144152
// Initialize all targets (required for device offloading)
145153
llvm::InitializeAllTargetInfos();
146154
llvm::InitializeAllTargets();
147155
llvm::InitializeAllTargetMCs();
148156
llvm::InitializeAllAsmPrinters();
149157

150158
std::vector<const char*> vargs(argv + 1, argv + argc);
151-
inner = compat::createClangInterpreter(vargs);
159+
auto CI = compat::createClangInterpreter(vargs);
160+
if (!CI) {
161+
llvm::errs() << "Interpreter creation failed\n";
162+
return nullptr;
163+
}
164+
165+
return std::unique_ptr<Interpreter>(new Interpreter(std::move(CI)));
152166
}
153167

154168
~Interpreter() {}

unittests/CppInterOp/CUDATest.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ static bool HasCudaSDK() {
1313
// FIXME: Enable this for cling.
1414
return false;
1515
#endif // CLANG_VERSION_MAJOR < 16
16-
Cpp::CreateInterpreter({}, {"--cuda"});
16+
if (!Cpp::CreateInterpreter({}, {"--cuda"}))
17+
return false;
1718
return Cpp::Declare("__global__ void test_func() {}"
1819
"test_func<<<1,1>>>();") == 0;
1920
};
@@ -30,7 +31,8 @@ static bool HasCudaRuntime() {
3031
if (!HasCudaSDK())
3132
return false;
3233

33-
Cpp::CreateInterpreter({}, {"--cuda"});
34+
if (!Cpp::CreateInterpreter({}, {"--cuda"}))
35+
return false;
3436
if (Cpp::Declare("__global__ void test_func() {}"
3537
"test_func<<<1,1>>>();"))
3638
return false;
@@ -74,4 +76,4 @@ TEST(CUDATest, CUDARuntime) {
7476
GTEST_SKIP() << "Skipping CUDA tests as CUDA runtime not found";
7577

7678
EXPECT_TRUE(HasCudaRuntime());
77-
}
79+
}

unittests/CppInterOp/InterpreterTest.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,25 @@ TEST(InterpreterTest, CreateInterpreter) {
167167
#endif
168168
}
169169

170+
#ifndef CPPINTEROP_USE_CLING
171+
TEST(InterpreterTest, CreateInterpreterCAPI) {
172+
const char* argv[] = {"-std=c++17"};
173+
auto *CXI = clang_createInterpreter(argv, 1);
174+
auto CLI = clang_Interpreter_getClangInterpreter(CXI);
175+
EXPECT_TRUE(CLI);
176+
clang_Interpreter_dispose(CXI);
177+
}
178+
179+
TEST(InterpreterTest, CreateInterpreterCAPIFailure) {
180+
#ifdef _WIN32
181+
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
182+
#endif
183+
const char* argv[] = {"-fsyntax-only", "-Xclang", "-invalid-plugin"};
184+
auto *CXI = clang_createInterpreter(argv, 3);
185+
EXPECT_EQ(CXI, nullptr);
186+
}
187+
#endif
188+
170189
#ifdef LLVM_BINARY_DIR
171190
TEST(InterpreterTest, DetectResourceDir) {
172191
#ifdef EMSCRIPTEN

0 commit comments

Comments
 (0)