Skip to content

Commit d78b4da

Browse files
aaronj0Martin Foll
authored andcommitted
[interop] Upgrade version to bring in upstreamed patches
Adds various upstreamed developments: compiler-research/CppInterOp#650 streamlines the logic in Cpp::Construct to be more generic. compiler-research/CppInterOp#643 by @hageboeck making CppInterOp work with new gtest target names Fixed wrapper name conflicts with ROOT that also uses `__cf`, allowing us to integrate JitCall incrementally and co-exist with CallFunc Fixes related to the cppyy forks Unrelated work on Emscripten and WebAssembly
1 parent b0222b8 commit d78b4da

16 files changed

+194
-38
lines changed

interpreter/CppInterOp/.clang-format

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
BasedOnStyle: LLVM
22

33
Language: Cpp
4-
Standard: Cpp11
4+
Standard: c++17
55
PointerAlignment: Left
66

77
IncludeCategories:

interpreter/CppInterOp/Emscripten-build-instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ $env:CMAKE_SYSTEM_PREFIX_PATH=$env:PREFIX
173173
```
174174

175175
on Windows. Now to build and test your Emscripten build of CppInterOp using node on Linux and osx execute the following
176+
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)
176177

177178
```bash
178179
mkdir build
@@ -190,6 +191,7 @@ emmake make -j $(nproc --all) check-cppinterop
190191
```
191192

192193
To build and test your Emscripten build of CppInterOp on using node Windows execute the following
194+
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)
193195

194196
```powershell
195197
emcmake cmake -DCMAKE_BUILD_TYPE=Release `

interpreter/CppInterOp/cmake/modules/GoogleTest.cmake

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,18 @@ include(ExternalProject)
2222
if (EMSCRIPTEN)
2323
if (CMAKE_C_COMPILER MATCHES ".bat")
2424
set(config_cmd emcmake.bat cmake)
25-
set(build_cmd emmake.bat make)
25+
if(CMAKE_GENERATOR STREQUAL "Ninja")
26+
set(build_cmd emmake.bat ninja)
27+
else()
28+
set(build_cmd emmake.bat make)
29+
endif()
2630
else()
2731
set(config_cmd emcmake cmake)
28-
set(build_cmd emmake make)
32+
if(CMAKE_GENERATOR STREQUAL "Ninja")
33+
set(build_cmd emmake ninja)
34+
else()
35+
set(build_cmd emmake make)
36+
endif()
2937
endif()
3038
else()
3139
set(config_cmd ${CMAKE_COMMAND})
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
130dd30548b59d8f618339158ab917c083184968
1+
9c9423069529caea5aee79b61580fe9a7b1eb73b

interpreter/CppInterOp/docs/DebuggingCppInterOp.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ Here's a comprehensive example that demonstrates common CppInterOp usage pattern
4848

4949
.. code-block:: cpp
5050
51-
#include "clang/Interpreter/CppInterOp.h"
51+
#include <CppInterOp/CppInterOp.h>
5252
#include <iostream>
5353
5454
void run_code(std::string code) {

interpreter/CppInterOp/docs/Emscripten-build-instructions.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ and
197197
$env:CMAKE_SYSTEM_PREFIX_PATH=$env:PREFIX
198198
199199
on Windows. Now to build and test your Emscripten build of CppInterOp on Linux and osx execute the following
200+
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)
200201

201202
.. code:: bash
202203
@@ -214,6 +215,7 @@ on Windows. Now to build and test your Emscripten build of CppInterOp on Linux a
214215
emmake make -j $(nproc --all) check-cppinterop
215216
216217
To build and test your Emscripten build of CppInterOp on Windows execute the following
218+
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)
217219

218220
.. code:: powershell
219221

interpreter/CppInterOp/include/CppInterOp/CppInterOp.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,8 @@ class JitCall {
224224
///\param[in] nary - Use array new if we have to construct an array of
225225
/// objects (nary > 1).
226226
///\param[in] args - a pointer to a argument list and argument size.
227+
///\param[in] is_arena - a pointer that indicates if placement new is to be
228+
/// used
227229
// FIXME: Change the type of withFree from int to bool in the wrapper code.
228230
void InvokeConstructor(void* result, unsigned long nary = 1,
229231
ArgList args = {}, void* is_arena = nullptr) const {
@@ -849,7 +851,7 @@ CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,
849851

850852
/// Creates one or more objects of class \c scope by calling its default
851853
/// constructor.
852-
/// \param[in] scope Class to construct
854+
/// \param[in] scope Class to construct, or handle to Constructor
853855
/// \param[in] arena If set, this API uses placement new to construct at this
854856
/// address.
855857
/// \param[in] is used to indicate the number of objects to construct.

interpreter/CppInterOp/lib/CppInterOp/CMakeLists.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ endif()
114114

115115

116116
if(EMSCRIPTEN)
117+
if(BUILD_SHARED_LIBS)
117118
# FIXME: When dynamically linking the Emscripten shared library to the
118119
# unit tests main_module you get errors due to undefined symbols. The reading of the file
119120
# below into a SYMBOLS_LIST variable is a temporary workaround that exports the undefined
@@ -131,8 +132,13 @@ if(EMSCRIPTEN)
131132
PRIVATE "SHELL: -s SIDE_MODULE=1"
132133
PRIVATE "SHELL: ${SYMBOLS_LIST}"
133134
)
135+
else()
136+
target_link_options(clangCppInterOp
137+
PRIVATE "SHELL: -s WASM_BIGINT"
138+
)
139+
endif()
134140
if (CPPINTEROP_ENABLE_TESTING)
135-
# When compiling Emscripten tests the shared library it links to is expected to be in the same folder as the compiled Javascript
141+
# When compiling Emscripten tests the CppInterOp library it links to is expected to be in the same folder as the compiled Javascript
136142
add_custom_command(TARGET clangCppInterOp POST_BUILD
137143
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:clangCppInterOp> ${CMAKE_BINARY_DIR}/unittests/CppInterOp/
138144
)

interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -616,8 +616,6 @@ static Decl* GetScopeFromType(QualType QT) {
616616
Type = Type->getUnqualifiedDesugaredType();
617617
if (auto* ET = llvm::dyn_cast<EnumType>(Type))
618618
return ET->getDecl();
619-
if (auto* FnType = llvm::dyn_cast<FunctionProtoType>(Type))
620-
Type = const_cast<clang::Type*>(FnType->getReturnType().getTypePtr());
621619
return Type->getAsCXXRecordDecl();
622620
}
623621
return 0;
@@ -1327,6 +1325,8 @@ void GetDatamembers(TCppScope_t scope, std::vector<TCppScope_t>& datamembers) {
13271325

13281326
if (auto* CXXRD = llvm::dyn_cast_or_null<CXXRecordDecl>(D)) {
13291327
getSema().ForceDeclarationOfImplicitMembers(CXXRD);
1328+
if (CXXRD->hasDefinition())
1329+
CXXRD = CXXRD->getDefinition();
13301330

13311331
llvm::SmallVector<RecordDecl::decl_iterator, 2> stack_begin;
13321332
llvm::SmallVector<RecordDecl::decl_iterator, 2> stack_end;
@@ -1447,7 +1447,7 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D,
14471447
}
14481448
offset += C.toCharUnitsFromBits(C.getFieldOffset(FD)).getQuantity();
14491449
}
1450-
if (BaseCXXRD && BaseCXXRD != FieldParentRecordDecl) {
1450+
if (BaseCXXRD && BaseCXXRD != FieldParentRecordDecl->getCanonicalDecl()) {
14511451
// FieldDecl FD belongs to some class C, but the base class BaseCXXRD is
14521452
// not C. That means BaseCXXRD derives from C. Offset needs to be
14531453
// calculated for Derived class
@@ -1476,7 +1476,7 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D,
14761476
}
14771477
if (auto* RD = llvm::dyn_cast<CXXRecordDecl>(FieldParentRecordDecl)) {
14781478
// add in the offsets for the (multi level) base classes
1479-
while (BaseCXXRD != RD) {
1479+
while (BaseCXXRD != RD->getCanonicalDecl()) {
14801480
CXXRecordDecl* Parent = direction.at(RD);
14811481
offset +=
14821482
C.getASTRecordLayout(Parent).getBaseClassOffset(RD).getQuantity();
@@ -1689,6 +1689,10 @@ std::string GetTypeAsString(TCppType_t var) {
16891689
PrintingPolicy Policy((LangOptions()));
16901690
Policy.Bool = true; // Print bool instead of _Bool.
16911691
Policy.SuppressTagKeyword = true; // Do not print `class std::string`.
1692+
#if CLANG_VERSION_MAJOR > 16
1693+
Policy.SuppressElaboration = true;
1694+
#endif
1695+
Policy.FullyQualifiedName = true;
16921696
return compat::FixTypeName(QT.getAsString(Policy));
16931697
}
16941698

@@ -1833,6 +1837,7 @@ void get_type_as_string(QualType QT, std::string& type_name, ASTContext& C,
18331837
#if CLANG_VERSION_MAJOR > 16
18341838
Policy.SuppressElaboration = true;
18351839
#endif
1840+
Policy.SuppressTagKeyword = !QT->isEnumeralType();
18361841
Policy.FullyQualifiedName = true;
18371842
QT.getAsStringInternal(type_name, Policy);
18381843
}
@@ -2103,6 +2108,30 @@ void make_narg_call(const FunctionDecl* FD, const std::string& return_type,
21032108
std::string template_args = complete_name.substr(idx);
21042109
name = name_without_template_args +
21052110
(template_args.empty() ? "" : " " + template_args);
2111+
2112+
// If a template has consecutive parameter packs, then it is impossible to
2113+
// use the explicit name in the wrapper, since the type deduction is what
2114+
// determines the split of the packs. Instead, we'll revert to the
2115+
// non-templated function name and hope that the type casts in the wrapper
2116+
// will suffice.
2117+
if (FD->isTemplateInstantiation() && FD->getPrimaryTemplate()) {
2118+
const FunctionTemplateDecl* FTDecl =
2119+
llvm::dyn_cast<FunctionTemplateDecl>(FD->getPrimaryTemplate());
2120+
if (FTDecl) {
2121+
auto* templateParms = FTDecl->getTemplateParameters();
2122+
int numPacks = 0;
2123+
for (size_t iParam = 0, nParams = templateParms->size();
2124+
iParam < nParams; ++iParam) {
2125+
if (templateParms->getParam(iParam)->isTemplateParameterPack())
2126+
numPacks += 1;
2127+
else
2128+
numPacks = 0;
2129+
}
2130+
if (numPacks > 1) {
2131+
name = name_without_template_args;
2132+
}
2133+
}
2134+
}
21062135
}
21072136
if (op_flag || N <= 1)
21082137
callbuf << name;
@@ -2707,7 +2736,7 @@ int get_wrapper_code(compat::Interpreter& I, const FunctionDecl* FD,
27072736
//
27082737
{
27092738
std::ostringstream buf;
2710-
buf << "__cf";
2739+
buf << "__jc";
27112740
// const NamedDecl* ND = dyn_cast<NamedDecl>(FD);
27122741
// std::string mn;
27132742
// fInterp->maybeMangleDeclName(ND, mn);
@@ -3422,6 +3451,8 @@ static Decl* InstantiateTemplate(TemplateDecl* TemplateD,
34223451
// This will instantiate tape<T> type and return it.
34233452
SourceLocation noLoc;
34243453
QualType TT = S.CheckTemplateIdType(TemplateName(TemplateD), noLoc, TLI);
3454+
if (TT.isNull())
3455+
return nullptr;
34253456

34263457
// Perhaps we can extract this into a new interface.
34273458
S.RequireCompleteType(fakeLoc, TT, diag::err_tentative_def_incomplete_type);
@@ -3753,22 +3784,25 @@ void Deallocate(TCppScope_t scope, TCppObject_t address, TCppIndex_t count) {
37533784
// FIXME: Add optional arguments to the operator new.
37543785
TCppObject_t Construct(compat::Interpreter& interp, TCppScope_t scope,
37553786
void* arena /*=nullptr*/, TCppIndex_t count /*=1UL*/) {
3756-
auto* Class = (Decl*)scope;
3757-
// FIXME: Diagnose.
3758-
if (!HasDefaultConstructor(Class))
3759-
return nullptr;
37603787

3761-
auto* const Ctor = GetDefaultConstructor(interp, Class);
3762-
if (JitCall JC = MakeFunctionCallable(&interp, Ctor)) {
3763-
if (arena) {
3764-
JC.InvokeConstructor(&arena, count, {},
3765-
(void*)~0); // Tell Invoke to use placement new.
3766-
return arena;
3767-
}
3788+
if (!Cpp::IsConstructor(scope) && !Cpp::IsClass(scope))
3789+
return nullptr;
3790+
if (Cpp::IsClass(scope) && !HasDefaultConstructor(scope))
3791+
return nullptr;
37683792

3769-
void* obj = nullptr;
3770-
JC.InvokeConstructor(&obj, count, {}, nullptr);
3771-
return obj;
3793+
TCppFunction_t ctor = nullptr;
3794+
if (Cpp::IsClass(scope))
3795+
ctor = Cpp::GetDefaultConstructor(scope);
3796+
else // a ctor
3797+
ctor = scope;
3798+
3799+
if (JitCall JC = MakeFunctionCallable(&interp, ctor)) {
3800+
// invoke the constructor (placement/heap) in one shot
3801+
// flag is non-null for placement new, null for normal new
3802+
void* is_arena = arena ? reinterpret_cast<void*>(1) : nullptr;
3803+
void* result = arena;
3804+
JC.InvokeConstructor(&result, count, /*args=*/{}, is_arena);
3805+
return result;
37723806
}
37733807
return nullptr;
37743808
}

interpreter/CppInterOp/unittests/CMakeLists.txt

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,23 @@ set(CTEST_BUILD_NAME
33
enable_testing()
44

55
# LLVM builds (not installed llvm) provides gtest.
6-
if (NOT TARGET gtest)
6+
if (NOT TARGET GTest::gtest AND NOT TARGET gtest)
77
include(GoogleTest)
88
endif()
99

1010
if(EMSCRIPTEN)
11-
set(gtest_libs gtest gmock)
11+
if (TARGET GTest::gtest)
12+
# Target names in CMake >= v3.23
13+
set(gtest_libs GTest::gtest GTest::gmock)
14+
else()
15+
set(gtest_libs gtest gmock)
16+
endif()
1217
else()
13-
set(gtest_libs gtest gtest_main)
14-
if (TARGET gmock)
15-
list(APPEND gtest_libs gmock gmock_main)
18+
if (TARGET GTest::gtest)
19+
# Target names in CMake >= v3.23
20+
set(gtest_libs GTest::gtest GTest::gmock GTest::gtest_main)
21+
else()
22+
set(gtest_libs gtest gtest_main gmock)
1623
endif()
1724
set(link_pthreads_lib pthread)
1825
endif()

0 commit comments

Comments
 (0)