Skip to content

[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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[interp] Register runtime symbols for clang-repl #726

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b912765
[interp] Register runtime symbols for clang-repl
aaronj0 Oct 1, 2025
9c6be69
Register `__clang_Interpreter_SetValueWithAlloc`, mangle name if LLVM<22
aaronj0 Oct 1, 2025
e8a6b27
Overload resolution for __clang_Interpreter_SetValueNoAlloc symbols t…
aaronj0 Oct 3, 2025
740bcb3
Add tests for Cpp::Evaluate that use more runtime symbols
aaronj0 Oct 3, 2025
4ef7df9
do not define and register symbols if emscripten
aaronj0 Oct 3, 2025
8d91bcd
Add struct `__clang_Interpreter_NewTag`
aaronj0 Oct 3, 2025
3c17a65
review comments
aaronj0 Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions lib/CppInterOp/CppInterOp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@
#include <unistd.h>
#endif // WIN32

extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, void* OutVal,
Copy link
Contributor

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

Copy link
Contributor

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

void* OpaqueType, ...);

namespace Cpp {

using namespace clang;
Expand Down Expand Up @@ -3330,6 +3333,37 @@
}

namespace {
#ifndef CPPINTEROP_USE_CLING
static bool DefineAbsoluteSymbol(compat::Interpreter& I,
const char* linker_mangled_name,
uint64_t address) {
Copy link
Contributor

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;
using namespace llvm::orc;

llvm::orc::LLJIT& Jit = *compat::getExecutionEngine(I);
llvm::orc::ExecutionSession& ES = Jit.getExecutionSession();
Copy link
Contributor

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

JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get();
Copy link
Contributor

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::SymbolMap InjectedSymbols;
Copy link
Contributor

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

auto& DL = compat::getExecutionEngine(I)->getDataLayout();
Copy link
Contributor

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]

Suggested change
auto& DL = compat::getExecutionEngine(I)->getDataLayout();
const auto& DL = compat::getExecutionEngine(I)->getDataLayout();

char GlobalPrefix = DL.getGlobalPrefix();
std::string tmp(linker_mangled_name);
if (GlobalPrefix != '\0') {
tmp = std::string(1, GlobalPrefix) + tmp;

Check warning on line 3352 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L3352 

Added line #L3352 was not covered by tests
}
auto Name = ES.intern(tmp);
InjectedSymbols[Name] =
ExecutorSymbolDef(ExecutorAddr(address), JITSymbolFlags::Exported);

if (Error Err = DyLib.define(absoluteSymbols(InjectedSymbols))) {
Copy link
Contributor

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

logAllUnhandledErrors(std::move(Err), errs(),

Check warning on line 3359 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L3359 

Added line #L3359 was not covered by tests
"DefineAbsoluteSymbol error: ");
return true;

Check warning on line 3361 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L3361 

Added line #L3361 was not covered by tests
}
return false;
}
#endif

static std::string MakeResourcesPath() {
StringRef Dir;
#ifdef LLVM_BINARY_DIR
Expand Down Expand Up @@ -3442,6 +3476,11 @@
sInterpreters->back().get()});

assert(sInterpreters->size() == sInterpreterASTMap->size());
// 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);
Copy link
Contributor

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);
                       ^

Copy link
Contributor

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);
                       ^

#endif
return I;
}

Expand Down
Loading