-
Notifications
You must be signed in to change notification settings - Fork 2.2k
PYBIND11_NAMESPACE
consistency fixes.
#4043
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Thanks! |
Just to note, the one post-merge CI failure is a flake I saw several times before already.
|
To probe environment/toolchain/platform-specific behavior under the exact same conditions as normal tests. (An earlier version of this code was used to inform PR pybind#4043.)
Pure record keeping, JIC it's useful for something later: (For reasons that are difficult to explain) I compared
Due to the flake mentioned in my previous comment, one result is missing. Apart from that, the Dumping the current
|
* Add test_namespace_visibility To probe environment/toolchain/platform-specific behavior under the exact same conditions as normal tests. (An earlier version of this code was used to inform PR #4043.) * Disable flake8 in ubench/holder_comparison_*.py, to suppress new & useless diagnostics. * Disable namespace_visibility_1s.cpp (tosee if that resolves the MSVC and CUDA `test_cross_module_exception_translator` failures). * Turn off flake8 completely for ubench (the Strip unnecessary `# noqa`s action un-helpfully removed the added noqa). * Disable test_namespace_visibility completely. Just keep the two .cpp files, only setting the module docstring and doing nothing else. * Rename test_namespace_visibility.py to test_exc_namespace_visibility.py, so that it is imported by pytest before test_exceptions.py * Add `set_property(SOURCE namespace_visibility_1s.cpp PROPERTY LANGUAGE CUDA)` * Add reference to PR #4054 * Complete the documentation (comments in test_exc_namespace_visibility.py). * Rename namespace_visibility.h to namespace_visibility.inl, as suggested by @charlesbeattie
Description
This PR fixes a couple long-standing
PYBIND11_NAMESPACE
__attribute__((visibility("hidden")))
inconsistencies. The fixes only affect builds with__GNUG__
defined.Based on experiments under PR #4044, this (currently) only makes a difference in an unusual situation: macOS, NOT using
-fvisibility=hidden
. However, if someone ventures there, surprises due to inconsistencies can be extremely time-consuming to troubleshoot.(Discovered while experimenting related to PR #4022.)
Suggested changelog entry: