Rely on the placement new version provided by clang-repl. by vgvassilev · Pull Request #912 · compiler-research/CppInterOp

vgvassilev · 2026-04-22T13:35:31Z

This patch removes the requirement for JitCall to include the header to access operator placement new. It now reuses the symbol provided by clang-repl.

github-actions

clang-tidy made some suggestions

github-actions · 2026-04-22T13:59:46Z

+  // Per [new.delete.placement] the standard placement overloads are
+  // declared `noexcept`; this non-noexcept redeclaration only parses
+  // if <new> is NOT in scope.
+  ASSERT_EQ(0, Cpp::Declare("void* operator new(__SIZE_TYPE__, void*);"));


warning: too few arguments to function call, expected 2, have 1 [clang-diagnostic-error]

ASSERT_EQ(0, Cpp::Declare("void* operator new(__SIZE_TYPE__, void*);")); ^

Additional context

build/unittests/googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1913: expanded from macro 'ASSERT_EQ'

#define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2) ^

build/unittests/googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1897: expanded from macro 'GTEST_ASSERT_EQ'

ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2) ^

build/unittests/googletest-prefix/src/googletest/googletest/include/gtest/gtest_pred_impl.h:147: expanded from macro 'ASSERT_PRED_FORMAT2'

GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_) ^

build/unittests/googletest-prefix/src/googletest/googletest/include/gtest/gtest_pred_impl.h:133: expanded from macro 'GTEST_PRED_FORMAT2_'

GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2), on_failure) ^

build/unittests/googletest-prefix/src/googletest/googletest/include/gtest/gtest_pred_impl.h:78: expanded from macro 'GTEST_ASSERT_'

if (const ::testing::AssertionResult gtest_ar = (expression)) \ ^

github-actions · 2026-04-22T13:59:46Z

+
+  // One JitCall is enough — if the wrapper regresses to plain placement
+  // new, MakeFunctionCallable fails to compile without <new>.
+  Cpp::Declare("int jc_sq(int x) { return x * x; }");


warning: too few arguments to function call, expected 2, have 1 [clang-diagnostic-error]

. ^

github-actions · 2026-04-22T13:59:46Z

+  auto JC = Cpp::MakeFunctionCallable(Cpp::GetNamed("jc_sq" DFLT_NULLPTR));
+  ASSERT_TRUE(JC.isValid());
+  int arg = 5, ret = 0;
+  void* args[] = {&arg};


warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]

; ^

github-actions · 2026-04-22T13:59:46Z

+  auto JC = Cpp::MakeFunctionCallable(Cpp::GetNamed("jc_sq" DFLT_NULLPTR));
+  ASSERT_TRUE(JC.isValid());
+  int arg = 5, ret = 0;
+  void* args[] = {&arg};


warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change

void* args[] = {&arg};

;

;int arg = 5;

int ret = 0;

github-actions · 2026-04-22T13:59:46Z

+  ASSERT_TRUE(JC.isValid());
+  int arg = 5, ret = 0;
+  void* args[] = {&arg};
+  JC.Invoke(&ret, {args, 1});


warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

; ^

github-actions · 2026-04-22T13:59:46Z

+  auto JCRef = Cpp::MakeFunctionCallable(RefFD);
+  EXPECT_TRUE(JCRef.getKind() == Cpp::JitCall::kGenericCall);
+  int* ref_ret = nullptr;
+  JCRef.Invoke(&ref_ret);


warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]

; ^

github-actions · 2026-04-22T13:59:46Z

+  EXPECT_EQ(*ref_ret, 7);
+  *ref_ret = 11;
+  int* ref_ret2 = nullptr;
+  JCRef.Invoke(&ref_ret2);


warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]

; ^

github-actions

clang-tidy made some suggestions

github-actions · 2026-04-22T18:12:02Z

+  // from the arena base (not past a cookie header).
+  const size_t T = Cpp::SizeOf(scope);
+  for (size_t i = 0; i < kN; ++i) {
+    int* slot = reinterpret_cast<int*>(reinterpret_cast<char*>(arena) + i * T);


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

++i) { ^

github-actions · 2026-04-22T18:12:02Z

+  // from the arena base (not past a cookie header).
+  const size_t T = Cpp::SizeOf(scope);
+  for (size_t i = 0; i < kN; ++i) {
+    int* slot = reinterpret_cast<int*>(reinterpret_cast<char*>(arena) + i * T);


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

++i) { ^

github-actions · 2026-04-22T18:12:02Z

+  const size_t T = Cpp::SizeOf(scope);
+  for (size_t i = 0; i < kN; ++i) {
+    int* slot = reinterpret_cast<int*>(reinterpret_cast<char*>(arena) + i * T);
+    EXPECT_EQ(*slot, 0xC0DE) << "element " << i << " not at expected offset";


warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]

Suggested change

EXPECT_EQ(*slot, 0xC0DE) << "element " << i << " not at expected offset";

++i) {

* T);()

github-actions · 2026-04-22T18:12:02Z

  if (llvm::sys::RunningOnValgrind())
    GTEST_SKIP() << "XFAIL due to Valgrind report";
-  std::vector<const char*> interpreter_args = { "-include", "new", "-Xclang", "-iwithsysroot/include/compat" };
+  std::vector<const char*> interpreter_args = {"-Xclang",


warning: no header providing "std::vector" is directly included [misc-include-cleaner]

unittests/CppInterOp/InterpreterTest.cpp:2:

+ #include <vector>

codecov · 2026-04-22T18:24:08Z

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.94%. Comparing base (5197645) to head (1d338e4).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/CppInterOp/CppInterOp.cpp	96.77%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #912      +/-   ##
==========================================
+ Coverage   85.89%   85.94%   +0.04%     
==========================================
  Files          15       15              
  Lines        4971     4995      +24     
==========================================
+ Hits         4270     4293      +23     
- Misses        701      702       +1

Files with missing lines	Coverage Δ
lib/CppInterOp/CppInterOp.cpp	`89.39% <96.77%> (+0.05%)`	⬆️

Files with missing lines	Coverage Δ
lib/CppInterOp/CppInterOp.cpp	`89.39% <96.77%> (+0.05%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 16. Check the log or trigger a new build to see more.

github-actions · 2026-04-23T18:47:18Z

+#include <cstddef>
+#include <cstdint>
+#include <set>
+#include <string>


warning: included header set is not used directly [misc-include-cleaner]

Suggested change

#include <string>

#include <string>

github-actions · 2026-04-23T18:47:18Z

+#include <cstdint>
+#include <set>
+#include <string>
+#include <vector>


warning: included header string is not used directly [misc-include-cleaner]

Suggested change

#include <vector>

#include <vector>

github-actions · 2026-04-23T18:47:19Z

+#include <set>
+#include <string>
+#include <vector>
+


warning: included header vector is not used directly [misc-include-cleaner]

Suggested change

github-actions · 2026-04-23T18:47:19Z

+    void** m_Args = nullptr;
+    size_t m_ArgSize = 0;
+    // Clang struggles with =default...
+    ArgList() {}


warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]

Suggested change

ArgList() {}

ArgList() = default;

github-actions · 2026-04-23T18:47:19Z

+
+public:
+  [[nodiscard]] Kind getKind() const { return m_Kind; }
+  bool isValid() const { return getKind() != kUnknown; }


warning: function 'isValid' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change

bool isValid() const { return getKind() != kUnknown; }

[[nodiscard]] bool isValid() const { return getKind() != kUnknown; }

github-actions · 2026-04-23T18:47:19Z

+public:
+  [[nodiscard]] Kind getKind() const { return m_Kind; }
+  bool isValid() const { return getKind() != kUnknown; }
+  bool isInvalid() const { return !isValid(); }


warning: function 'isInvalid' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change

bool isInvalid() const { return !isValid(); }

[[nodiscard]] bool isInvalid() const { return !isValid(); }

github-actions · 2026-04-23T18:47:19Z

+#ifndef NDEBUG
+    ReportInvokeStart(object, nary, withFree);
+#endif // NDEBUG
+    m_DestructorCall(object, nary, withFree);


warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

m_DestructorCall(object, nary, withFree); ^

github-actions · 2026-04-23T18:47:19Z

+           "Invalid args!");
+    ReportInvokeStart(result, args, nullptr);
+#endif // NDEBUG
+    m_ConstructorCall(result, nary, args.m_ArgSize, args.m_Args, is_arena);


warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

m_ConstructorCall(result, nary, args.m_ArgSize, args.m_Args, is_arena); ^

github-actions · 2026-04-23T18:47:19Z

+
+// FIXME: Rework GetDimensions to make this enum redundant.
+namespace DimensionValue {
+enum : long int {


warning: enum '(unnamed enum at /github/workspace/include/CppInterOp/CppInterOpTypes.h:338:1)' uses a larger base type ('long', size: 8 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

enum : long int { ^

github-actions · 2026-04-23T18:47:19Z

+// symbol is always resolvable there, even when the host's dynamic
+// linker does not expose libclangInterpreter's copy.
+namespace {
+void* CppInterOpPlacementNew(std::size_t, void* __p,


warning: declaration uses identifier '__p', which is a reserved identifier [bugprone-reserved-identifier]

void* CppInterOpPlacementNew(std::size_t, void* __p, ^

this fix will not be applied because it overlaps with another fix

github-actions

clang-tidy made some suggestions

github-actions · 2026-04-23T19:29:04Z

+// symbol is always resolvable there, even when the host's dynamic
+// linker does not expose libclangInterpreter's copy.
+namespace {
+void* CppInterOpPlacementNew(std::size_t, void* __p,


warning: invalid case style for parameter '__p' [readability-identifier-naming]

void* CppInterOpPlacementNew(std::size_t, void* __p, ^

this fix will not be applied because it overlaps with another fix

github-actions · 2026-04-23T19:29:04Z

+  // Apply the target-triple's symbol prefix (empty on ELF, `_` on macOS
+  // Mach-O / x86 Windows) so the interned name matches what the JIT
+  // will look up when resolving references from compiled modules.
+  MangleAndInterner Mangle(ES, Jit.getDataLayout());


warning: no header providing "llvm::orc::MangleAndInterner" is directly included [misc-include-cleaner]

lib/CppInterOp/CppInterOp.cpp:84:

- #include <map> + #include <llvm/ExecutionEngine/Orc/Mangling.h> + #include <map>

github-actions · 2026-04-23T19:29:04Z

+      compat::maybeMangleDeclName(GlobalDecl(FD), mangled);
+      DefineAbsoluteSymbol(
+          *I, mangled.c_str(),
+          reinterpret_cast<uint64_t>(&CppInterOpPlacementNew));


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

^

github-actions · 2026-04-23T19:29:04Z

+  ASSERT_NE(arena, nullptr);
+
+  EXPECT_EQ(Cpp::Construct(scope, arena), arena);
+  EXPECT_EQ(*reinterpret_cast<int*>(arena), 42);


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

EXPECT_EQ(*reinterpret_cast<int*>(arena), 42); ^

github-actions · 2026-04-23T19:29:04Z

+      << "Wrapper's placement-new expression failed to compile for a "
+         "class that declares its own operator new; add `::` to force "
+         "global-scope lookup.";
+  EXPECT_EQ(*reinterpret_cast<int*>(arena), 123);


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

; ^

github-actions

clang-tidy made some suggestions

github-actions · 2026-04-23T20:05:42Z

+      std::string mangled;
+      compat::maybeMangleDeclName(GlobalDecl(FD), mangled);
+      DefineAbsoluteSymbol(*I, mangled.c_str(),
+                           reinterpret_cast<uint64_t>(&CppInterOpPlacementNew));


warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

^

The JitCall wrapper generator emits placement-new expressions on two hot paths (`make_narg_call_with_return` and the `make_narg_ctor_with_return` scalar branch), and the array-ctor branch emitted `new (p) T[n]`. Each of those forms binds against the standard placement operators declared in `<new>`, so every JitCall consumer had to pull `<new>` into the interpreter's TU and every ctor-using test passed `-include new` on the command line. Emit `::new (p, __ci_newtag) T(...)` instead. The tagged scalar overload `operator new(size_t, void*, __clang_Interpreter_NewTag)` is preloaded by clang-repl's Runtimes string (LLVM 18+, llvm/llvm-project@1566f1ffc6b5), and its definition is registered in the JIT dylib by a local forwarder `CppInterOpPlacementNew` plumbed through `DefineAbsoluteSymbol` -- otherwise embedders that dlopen libclangCppInterOp with RTLD_LOCAL (cppyy, the DispatchTests binary) cannot resolve `_ZnwmPv26__clang_Interpreter_NewTag` at JIT link time. `DefineAbsoluteSymbol` now uses `Jit.mangleAndIntern` so the registered key carries the target DataLayout's global-symbol prefix (empty on ELF, `_` on Mach-O / 32-bit COFF). The array default-ctor branch becomes a loop of scalar tagged placements: a tagged `operator new[]` would insert an Itanium ABI array cookie ([expr.new]/8, Itanium C++ ABI S2.7) and break the `Construct(scope, arena, n) == arena` contract. The unary `::` qualifier on every placement-new emission forces name lookup straight to global scope; per [class.free]/2, if the allocated type has any class-scope `operator new`, global scope is otherwise not consulted as a fallback and the wrapper refuses to compile (cppyy's test14_new_overloader was the regression). The non-tagged `: new T(args)` arm of the is_arena ternary stays unqualified so `Cpp::Construct(scope)` still routes through the class allocator. Regression coverage: DispatchSmokeTest.TaggedPlacementNewResolvable pins the JIT-link path under dlopen-RTLD_LOCAL; DispatchSmokeTest. PlacementConstructTaggedNew pins the wrapper-emit path; and the JitCallNoNewHeader / ArrayConstructNoCookie / ConstructClassWith OperatorNew tests under FunctionReflectionTest pin the no-`<new>` premise, the no-cookie array contract, and the global-vs-class allocator routing respectively. Valgrind-XFAIL skips at the new test sites are gated `#if CLANG_VERSION_MAJOR < 22`; LLVM 22 dropped `llvm/Support/Valgrind.h`.

vgvassilev requested a review from Vipul-Cariappa April 22, 2026 13:35

github-actions Bot reviewed Apr 22, 2026

View reviewed changes

vgvassilev force-pushed the jitcall-drop-new-header branch 4 times, most recently from b972a1d to b16bb04 Compare April 22, 2026 17:41

github-actions Bot reviewed Apr 22, 2026

View reviewed changes

vgvassilev force-pushed the jitcall-drop-new-header branch from b16bb04 to 3add78b Compare April 22, 2026 18:17

vgvassilev force-pushed the jitcall-drop-new-header branch 4 times, most recently from ad9a0fb to 3abcaec Compare April 23, 2026 18:21

vgvassilev mentioned this pull request Apr 23, 2026

Make sure <new> is included before testing compiler-research/cppyy#206

Merged

github-actions Bot reviewed Apr 23, 2026

View reviewed changes

vgvassilev force-pushed the jitcall-drop-new-header branch from 6593d5e to 9352072 Compare April 23, 2026 19:10

github-actions Bot reviewed Apr 23, 2026

View reviewed changes

vgvassilev force-pushed the jitcall-drop-new-header branch from 9352072 to 302308c Compare April 23, 2026 19:47

github-actions Bot reviewed Apr 23, 2026

View reviewed changes

vgvassilev force-pushed the jitcall-drop-new-header branch from dd4fecf to f66e448 Compare May 13, 2026 19:47

vgvassilev force-pushed the jitcall-drop-new-header branch from f66e448 to 1d338e4 Compare May 14, 2026 17:48

-  void* args[] = {&arg};
+;
+;int arg = 5;
+  int ret = 0;

	EXPECT_EQ(*slot, 0xC0DE) << "element " << i << " not at expected offset";
	++i) {
	* T);()

	bool isValid() const { return getKind() != kUnknown; }
	[[nodiscard]] bool isValid() const { return getKind() != kUnknown; }

	bool isInvalid() const { return !isValid(); }
	[[nodiscard]] bool isInvalid() const { return !isValid(); }

Uh oh!

Conversation

vgvassilev commented Apr 22, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 22, 2026 •

edited

Loading