Define (non-empty) PYBIND11_EXPORT_EXCEPTION only under macOS.
#4298
Conversation
Background: pybind#2999, pybind#4105, pybind#4283, pybind#4284 In a nutshell: * Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (pybind#4284). * Evidently (pybind#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations, * but evidently (pybind#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used. * Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
|
I think this would be good to have in 2.10.1, too. Nobody complained about ODR violations, or the need to deal with the Windows warnings, but this gets rid of both stumbling stones. We may just be saving a handful of people a few hours of unhappiness. |
|
@oraluben JIC you want to look or comment. |
LGTM |
|
I'm a little worried about changes like this hours or minutes before a release, but it looks pretty safe. If we do wait, we can put it in immediately after release. I'll leave it up to @Skylion007. |
We have used that quite frequently to work around issues with Magnum / Habitat-Sim bindings playing nicely together... To clarify, are you saying this PR would cause issues there? Or fix a potential regression and therefore issues? I don't think we worry that much about pybind11 exceptions propogating between the two though. |
Oh!
Did you also mix extensions built with different pybind11 versions? In that case I'd expect pretty terrible undefined behavior caused by the ODR violation for Now thinking: We really need to have this in 2.10.1. In case you used pybind11 from sources, but a mix of source versions between extension: you're really on thin ice with In case you only used releases: mixing any two release that have #2999 is still on thin ice (I don't have time to look up the release history right now). But especially after #1895 was merged, it's not just thin ice, ODR violations are a certainty, on Linux, with |
…#4298) Background: #2999, #4105, #4283, #4284 In a nutshell: * Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284). * Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations, * but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used. * Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
Description
Background: #2999, #4105, #4283, #4284
In a nutshell:
Only macOS actually needs
PYBIND11_EXPORT_EXCEPTION(Effectively undoing #2999, to see what platforms are affected. #4284).Evidently (WIP cross module exception ODR experiments #4283), under macOS
PYBIND11_EXPORT_EXCEPTIONdoes not run the risk of introducing ODR violations,but evidently (WIP cross module exception ODR experiments #4283) under Linux it does, in the presumably rare/unusual situation that
RTLD_GLOBALis used.Windows does no have the equivalent of
RTLD_GLOBAL, thereforePYBIND11_EXPORT_EXCEPTIONhas no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.Suggested changelog entry:
`PYBIND11_EXPORT_EXCEPTION` was made non-empty only under macOS. This makes Linux builds safer, and enables the removal of warning suppression pragmas for Windows.