-
Notifications
You must be signed in to change notification settings - Fork 34
[interp] Register runtime symbols for clang-repl #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
// define __clang_Interpreter_SetValueNoAlloc in the JIT dylib for clang-repl | ||
#ifndef CPPINTEROP_USE_CLING | ||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueNoAlloc", | ||
(uint64_t)&__clang_Interpreter_SetValueNoAlloc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need the entire runtime not only that function.
7ab924a
to
b912765
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #726 +/- ##
==========================================
+ Coverage 81.03% 81.11% +0.08%
==========================================
Files 9 9
Lines 4213 4242 +29
==========================================
+ Hits 3414 3441 +27
- Misses 799 801 +2
... and 1 file with indirect coverage changes
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
lib/CppInterOp/CppInterOp.cpp
Outdated
#include <unistd.h> | ||
#endif // WIN32 | ||
|
||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, void* OutVal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, void* OutVal,
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
#include <unistd.h> | ||
#endif // WIN32 | ||
|
||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, void* OutVal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, void* OutVal,
^
this fix will not be applied because it overlaps with another fix
|
||
namespace { | ||
#ifndef CPPINTEROP_USE_CLING | ||
static bool DefineAbsoluteSymbol(compat::Interpreter& I, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'DefineAbsoluteSymbol' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
static bool DefineAbsoluteSymbol(compat::Interpreter& I, | |
bool DefineAbsoluteSymbol(compat::Interpreter& I, |
#ifndef CPPINTEROP_USE_CLING | ||
static bool DefineAbsoluteSymbol(compat::Interpreter& I, | ||
const char* linker_mangled_name, | ||
uint64_t address) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "uint64_t" is directly included [misc-include-cleaner]
uint64_t address) {
^
using namespace llvm::orc; | ||
|
||
llvm::orc::LLJIT& Jit = *compat::getExecutionEngine(I); | ||
llvm::orc::ExecutionSession& ES = Jit.getExecutionSession(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "llvm::orc::ExecutionSession" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:41:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/Core.h>
+ #if CLANG_VERSION_MAJOR >= 19
|
||
llvm::orc::LLJIT& Jit = *compat::getExecutionEngine(I); | ||
llvm::orc::ExecutionSession& ES = Jit.getExecutionSession(); | ||
JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "llvm::orc::JITDylib" is directly included [misc-include-cleaner]
JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get();
^
llvm::orc::ExecutionSession& ES = Jit.getExecutionSession(); | ||
JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get(); | ||
|
||
llvm::orc::SymbolMap InjectedSymbols; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "llvm::orc::SymbolMap" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:41:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/CoreContainers.h>
+ #if CLANG_VERSION_MAJOR >= 19
JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get(); | ||
|
||
llvm::orc::SymbolMap InjectedSymbols; | ||
auto& DL = compat::getExecutionEngine(I)->getDataLayout(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'auto &DL' can be declared as 'const auto &DL' [readability-qualified-auto]
auto& DL = compat::getExecutionEngine(I)->getDataLayout(); | |
const auto& DL = compat::getExecutionEngine(I)->getDataLayout(); |
InjectedSymbols[Name] = | ||
ExecutorSymbolDef(ExecutorAddr(address), JITSymbolFlags::Exported); | ||
|
||
if (Error Err = DyLib.define(absoluteSymbols(InjectedSymbols))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "llvm::orc::absoluteSymbols" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:41:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/AbsoluteSymbols.h>
+ #if CLANG_VERSION_MAJOR >= 19
// define __clang_Interpreter_SetValueNoAlloc in the JIT dylib for clang-repl | ||
#ifndef CPPINTEROP_USE_CLING | ||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueNoAlloc", | ||
(uint64_t)&__clang_Interpreter_SetValueNoAlloc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
(uint64_t)&__clang_Interpreter_SetValueNoAlloc);
^
d7da73e
to
9c6be69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
extern "C" void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | ||
void* OpaqueType) | ||
#else | ||
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_SetValueWithAlloc', which is a reserved identifier [bugprone-reserved-identifier]
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal,
^
this fix will not be applied because it overlaps with another fix
extern "C" void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | ||
void* OpaqueType) | ||
#else | ||
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function '__clang_Interpreter_SetValueWithAlloc' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | |
static void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, |
extern "C" void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | ||
void* OpaqueType) | ||
#else | ||
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '__clang_Interpreter_SetValueWithAlloc' [readability-identifier-naming]
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal,
^
this fix will not be applied because it overlaps with another fix
void* OpaqueType); | ||
#endif | ||
|
||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
void* OpaqueType); | ||
#endif | ||
|
||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
std::string mangledName; | ||
compat::maybeMangleDeclName(GD, mangledName); | ||
DefineAbsoluteSymbol(*I, mangledName.c_str(), | ||
(uint64_t)&__clang_Interpreter_SetValueWithAlloc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
(uint64_t)&__clang_Interpreter_SetValueWithAlloc);
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
lib/CppInterOp/CppInterOp.cpp
Outdated
#endif | ||
|
||
#if CLANG_VERSION_MAJOR >= 19 | ||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
#endif | ||
|
||
#if CLANG_VERSION_MAJOR >= 19 | ||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
} | ||
#else | ||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueNoAlloc", | ||
(uint64_t)&__clang_Interpreter_SetValueNoAlloc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
(uint64_t)&__clang_Interpreter_SetValueNoAlloc);
^
fdffc1d
to
740bcb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
#endif | ||
|
||
#if CLANG_VERSION_MAJOR >= 19 | ||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
#endif | ||
|
||
#if CLANG_VERSION_MAJOR >= 19 | ||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming]
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^
this fix will not be applied because it overlaps with another fix
} | ||
#else | ||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueNoAlloc", | ||
(uint64_t)&__clang_Interpreter_SetValueNoAlloc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
(uint64_t)&__clang_Interpreter_SetValueNoAlloc);
^
aadc275
to
4ef7df9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__ci_newtag', which is a reserved identifier [bugprone-reserved-identifier]
struct __clang_Interpreter_NewTag {} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_NewTag', which is a reserved identifier [bugprone-reserved-identifier]
struct __clang_Interpreter_NewTag {} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for class '__clang_Interpreter_NewTag' [readability-identifier-naming]
struct __clang_Interpreter_NewTag {} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for variable '__ci_newtag' [readability-identifier-naming]
struct __clang_Interpreter_NewTag {} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable '__ci_newtag' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
struct __clang_Interpreter_NewTag {} __ci_newtag; | |
static struct __clang_Interpreter_NewTag {} __ci_newtag; |
lib/CppInterOp/CppInterOp.cpp
Outdated
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag {} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable '__ci_newtag' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
struct __clang_Interpreter_NewTag {} __ci_newtag;
^
|
||
// Define runtime symbols in the JIT dylib for clang-repl | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
DefineAbsoluteSymbol(*I, "__ci_newtag", (uint64_t)&__ci_newtag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
DefineAbsoluteSymbol(*I, "__ci_newtag", (uint64_t)&__ci_newtag);
^
02883df
to
b45c436
Compare
b45c436
to
8d91bcd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__clang_Interpreter_NewTag', which is a reserved identifier [bugprone-reserved-identifier]
struct __clang_Interpreter_NewTag {
^
this fix will not be applied because it overlaps with another fix
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for class '__clang_Interpreter_NewTag' [readability-identifier-naming]
struct __clang_Interpreter_NewTag {
^
this fix will not be applied because it overlaps with another fix
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { | ||
} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: declaration uses identifier '__ci_newtag', which is a reserved identifier [bugprone-reserved-identifier]
} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { | ||
} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for variable '__ci_newtag' [readability-identifier-naming]
} __ci_newtag;
^
this fix will not be applied because it overlaps with another fix
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { | ||
} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable '__ci_newtag' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
lib/CppInterOp/CppInterOp.cpp:95:
- struct __clang_Interpreter_NewTag {
+ static struct __clang_Interpreter_NewTag {
// link to llvm | ||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||
struct __clang_Interpreter_NewTag { | ||
} __ci_newtag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable '__ci_newtag' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
} __ci_newtag;
^
#if CLANG_VERSION_MAJOR >= 19 | ||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, | ||
void* OutVal, | ||
void* OpaqueType, ...); | ||
#elif CLANG_VERSION_MAJOR == 18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#if CLANG_VERSION_MAJOR >= 19 | |
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, | |
void* OutVal, | |
void* OpaqueType, ...); | |
#elif CLANG_VERSION_MAJOR == 18 | |
#if CLANG_VERSION_MAJOR > 18 | |
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, | |
void* OutVal, | |
void* OpaqueType, ...); | |
#else |
Registering this runtime symbol fixes JIT session errors when calling
Cpp::Evaluate
if the process does not see symbols fromlibClangCppInterOp.so
.