-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[orc-rt] Ensure EH/RTTI=On overrides LLVM opts, applies to unit tests. #172155
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.
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
Conversation
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.
3631ded to
5f8fe07
Compare
| span-test.cpp | ||
| DISABLE_LLVM_LINK_LLVM_DYLIB | ||
| ) | ||
| target_compile_options(CoreTests PRIVATE ${ORC_RT_COMPILE_FLAGS}) |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
llvm#172155) 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.
llvm#172155) 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.
llvm#172155) 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.