Skip to content

Commit 371b096

Browse files
authored
[interop] Fix build errors with -Dtesting=off (#17791)
* [interop] Bump to latest version, fixes cmake overriding testing flag * Patch in InterOp that fixes testing mode issues(to be upstreamed) * [interpreter] Configure the flags for InterOp correctly Also moves the addition of `CppInterOpUnitTests` to the build chain, from metacling, to the interpreter cmake
1 parent bec2612 commit 371b096

File tree

6 files changed

+56
-30
lines changed

6 files changed

+56
-30
lines changed

.github/workflows/cppinterop-diff.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
uses: actions/checkout@v4
1717
with:
1818
repository: compiler-research/CppInterOp
19-
ref: 6c6f94a22bd971520a249e2c02e4259cdd3a5be6
19+
ref: b0c3e360bdbea9618dd2744836671667c108bf97
2020
path: CppInterOp
2121
- name: Drop directories that are not added to ROOT
2222
working-directory: CppInterOp

core/metacling/src/CMakeLists.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ ROOT_LINKER_LIBRARY(Cling
125125
# When these two link at the same time, they can exhaust the RAM on many machines, since they both link against llvm.
126126
add_dependencies(Cling rootcling_stage1)
127127

128-
if(testing)
129-
set_target_properties(CppInterOpUnitTests PROPERTIES EXCLUDE_FROM_ALL OFF)
130-
endif()
131-
132128
if(MSVC)
133129
set_target_properties(Cling PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
134130
set(cling_exports ${cling_exports}

interpreter/CMakeLists.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,17 @@ set(CPPINTEROP_INCLUDE_DIRS
523523
CACHE STRING "CppInterOp include directories.")
524524

525525
#---Set InterOp to build on Cling, this can be toggled to use Clang-REPL-------------------------------------------------------
526-
set(CPPINTEROP_USE_CLING ON BOOL "Use Cling as backend")
526+
set(CPPINTEROP_USE_CLING ON CACHE BOOL "" FORCE)
527+
set(CPPINTEROP_USE_REPL OFF CACHE BOOL "" FORCE)
528+
529+
#---Disable testing on InterOp if ROOT's testing is OFF (InterOp tests are enabled by default) -------------------------------------------------------
530+
if(testing)
531+
set(CPPINTEROP_ENABLE_TESTING ON CACHE BOOL "" FORCE)
532+
else()
533+
set(CPPINTEROP_ENABLE_TESTING OFF CACHE BOOL "")
534+
endif()
527535

528536
add_subdirectory(CppInterOp)
537+
if(testing)
538+
set_target_properties(CppInterOpUnitTests PROPERTIES EXCLUDE_FROM_ALL OFF)
539+
endif()

interpreter/CppInterOp/CMakeLists.txt

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ set(CMAKE_MODULE_PATH
66
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
77
)
88

9+
enable_language(CXX)
10+
set(CMAKE_CXX_EXTENSIONS NO)
11+
12+
option(CPPINTEROP_USE_CLING "Use Cling as backend" OFF)
13+
option(CPPINTEROP_USE_REPL "Use clang-repl as backend" ON)
14+
option(CPPINTEROP_ENABLE_TESTING "Enable the CppInterOp testing infrastructure." ON)
15+
16+
if (CPPINTEROP_USE_CLING AND CPPINTEROP_USE_REPL)
17+
message(FATAL_ERROR "We can only use Cling (${CPPINTEROP_USE_CLING}=On) or Repl (CPPINTEROP_USE_REPL=On), but not both of them.")
18+
endif()
919
# If we are not building as a part of LLVM, build CppInterOp as a standalone
1020
# project, using LLVM as an external library:
1121
if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
@@ -51,19 +61,10 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
5161
endif()
5262
if (NOT DEFINED Clang_DIR)
5363
set(Clang_DIR ${Cling_DIR})
54-
endif()
5564
endif()
65+
endif()
5666

57-
enable_language(CXX)
58-
set(CMAKE_CXX_EXTENSIONS NO)
5967
include(GNUInstallDirs)
60-
option(CPPINTEROP_USE_CLING "Use Cling as backend" OFF)
61-
option(CPPINTEROP_USE_REPL "Use clang-repl as backend" ON)
62-
63-
if (CPPINTEROP_USE_CLING AND CPPINTEROP_USE_REPL)
64-
message(FATAL_ERROR "We can only use Cling (CPPINTEROP_USE_CLING=On) or Repl (CPPINTEROP_USE_REPL=On), but not both of them.")
65-
endif()
66-
6768
## Define supported version of clang and llvm
6869

6970
set(CLANG_MIN_SUPPORTED 13.0)
@@ -292,16 +293,15 @@ set(CMAKE_INCLUDE_CURRENT_DIR ON)
292293
# In rare cases we might want to have clang installed in a different place
293294
# than llvm and the header files should be found first (even though the
294295
# LLVM_INCLUDE_DIRS) contain clang headers, too.
295-
if (CPPINTEROP_USE_CLING)
296+
if(CPPINTEROP_USE_CLING)
296297
add_definitions(-DCPPINTEROP_USE_CLING)
297298
include_directories(SYSTEM ${CLING_INCLUDE_DIRS})
298-
else()
299-
if (NOT CPPINTEROP_USE_REPL)
300-
message(FATAL_ERROR "We need either CPPINTEROP_USE_CLING or CPPINTEROP_USE_REPL")
301-
endif()
299+
elseif(CPPINTEROP_USE_REPL)
302300
add_definitions(-DCPPINTEROP_USE_REPL)
301+
else()
302+
message(FATAL_ERROR "We need either CPPINTEROP_USE_CLING or CPPINTEROP_USE_REPL")
303+
endif()
303304

304-
endif(CPPINTEROP_USE_CLING)
305305
include_directories(SYSTEM ${CLANG_INCLUDE_DIRS})
306306
include_directories(SYSTEM ${LLVM_INCLUDE_DIRS})
307307
separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
@@ -453,10 +453,7 @@ option(CPPINTEROP_ENABLE_SPHINX "Use sphinx to generage CppInterOp user document
453453

454454
if(EMSCRIPTEN)
455455
message("Build with emscripten")
456-
option(CPPINTEROP_ENABLE_TESTING "Enables the testing infrastructure." OFF)
457-
else()
458-
message("Build with cmake")
459-
option(CPPINTEROP_ENABLE_TESTING "Enables the testing infrastructure." ON)
456+
set(CPPINTEROP_ENABLE_TESTING OFF)
460457
endif()
461458

462459
if(MSVC)

interpreter/CppInterOp/lib/Interpreter/CppInterOp.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,6 +1675,9 @@ namespace Cpp {
16751675
//
16761676
ASTContext& C = FD->getASTContext();
16771677
PrintingPolicy Policy(C.getPrintingPolicy());
1678+
#if CLANG_VERSION_MAJOR > 16
1679+
Policy.SuppressElaboration = true;
1680+
#endif
16781681
refType = kNotReference;
16791682
if (QT->isRecordType() && forArgument) {
16801683
get_type_as_string(QT, type_name, C, Policy);
@@ -1989,18 +1992,22 @@ namespace Cpp {
19891992
EReferenceType refType = kNotReference;
19901993
bool isPointer = false;
19911994

1995+
std::ostringstream typedefbuf;
1996+
std::ostringstream callbuf;
1997+
1998+
collect_type_info(FD, QT, typedefbuf, callbuf, type_name, refType,
1999+
isPointer, indent_level, false);
2000+
2001+
buf << typedefbuf.str();
2002+
19922003
buf << "if (ret) {\n";
19932004
++indent_level;
19942005
{
1995-
std::ostringstream typedefbuf;
1996-
std::ostringstream callbuf;
19972006
//
19982007
// Write the placement part of the placement new.
19992008
//
20002009
indent(callbuf, indent_level);
20012010
callbuf << "new (ret) ";
2002-
collect_type_info(FD, QT, typedefbuf, callbuf, type_name, refType,
2003-
isPointer, indent_level, false);
20042011
//
20052012
// Write the type part of the placement new.
20062013
//

interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,9 @@ TEST(FunctionReflectionTest, GetFunctionCallWrapper) {
888888
int f3() { return 3; }
889889
890890
extern "C" int f4() { return 4; }
891+
892+
typedef int(*int_func)(void);
893+
int_func f5() { return f3; }
891894
}
892895
)");
893896

@@ -923,6 +926,18 @@ TEST(FunctionReflectionTest, GetFunctionCallWrapper) {
923926
FCI4.Invoke(&ret4);
924927
EXPECT_EQ(ret4, 4);
925928

929+
#if CLANG_VERSION_MAJOR > 16
930+
Cpp::JitCall FCI5 =
931+
Cpp::MakeFunctionCallable(Cpp::GetNamed("f5", Cpp::GetNamed("NS")));
932+
EXPECT_TRUE(FCI5.getKind() == Cpp::JitCall::kGenericCall);
933+
934+
typedef int (*int_func)();
935+
int_func callback = nullptr;
936+
FCI5.Invoke((void*)&callback);
937+
EXPECT_TRUE(callback);
938+
EXPECT_EQ(callback(), 3);
939+
#endif
940+
926941
// FIXME: Do we need to support private ctors?
927942
Interp->process(R"(
928943
class C {

0 commit comments

Comments
 (0)