Skip to content

Conversation

@lhames
Copy link
Contributor

@lhames lhames commented Dec 13, 2025

When -DORC_RT_ENABLE_EXCEPTIONS=On and -DORC_RT_ENABLE_RTTI=On are passed we need to ensure that the resulting compiler flags (e.g. -fexceptions, -frtti for clang/GCC) are appended so that we override any inherited options (e.g. -fno-exceptions, -fno-rtti) from LLVM.

Updates unit tests to ensure that these compiler options are applied to them too.

When -DORC_RT_ENABLE_EXCEPTIONS=On and -DORC_RT_ENABLE_RTTI=On are passed we
need to ensure that the resulting compiler flags (e.g. -fexceptions, -frtti
for clang/GCC) are appended so that we override any inherited options (e.g.
-fno-exceptions, -fno-rtti) from LLVM.

Updates unit tests to ensure that these compiler options are applied to them
too.
@lhames lhames force-pushed the orc-rt-eh-rtti-unittests branch from 3631ded to 5f8fe07 Compare December 13, 2025 12:52
span-test.cpp
DISABLE_LLVM_LINK_LLVM_DYLIB
)
target_compile_options(CoreTests PRIVATE ${ORC_RT_COMPILE_FLAGS})
Copy link
Contributor

@jaredwy jaredwy Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could instead use the CXX_RTTI and CXX_EXCEPTIONS properties here, and do the checks here instead of inside the the header. This is more akin to how a library consumer would compile the orc-rt.

These are cross platform and will work on windows (but we don't have that supported yet)

Copy link
Contributor

@jaredwy jaredwy Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_target_properties(CoreTests PROPERTIES
 CXX_RTTI ${ORC_RT_ENABLE_RTTI}
 CXX_EXCEPTIONS ${ORC_RT_ENABLE_EXCEPTIONS}
)

something like this should work, but you would also need to ensure its on the executor etc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It look like these are unique to a previous project.

Feel free to merge and ill introduce these later when I add windows support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants