From 3d1a7f36b83b600cba0dc6597d56763fe494d5b9 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 16 Jul 2022 09:25:42 -0700 Subject: [PATCH] Remove cmake `PROPERTIES CXX_VISIBILITY_PRESET "hidden"`, add `PYBIND11_NS_VISIBILITY`, use to resolve all "... declared with greater visibility than ..." warnings across all platforms. --- include/pybind11/detail/common.h | 4 +++- pybind11/setup_helpers.py | 1 - tests/pybind11_cross_module_tests.cpp | 2 +- tests/test_custom_type_setup.cpp | 5 +++++ tests/test_exceptions.cpp | 6 ++++++ tests/test_exceptions.h | 4 ++++ tests/test_numpy_array.cpp | 6 ++++++ tests/test_pytypes.cpp | 4 ++-- tools/pybind11NewTools.cmake | 13 ------------- tools/pybind11Tools.cmake | 13 ------------- 10 files changed, 27 insertions(+), 31 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 05370108fb..94b911f14b 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -25,8 +25,10 @@ // on the main `pybind11` namespace. #if !defined(PYBIND11_NAMESPACE) # ifdef __GNUG__ -# define PYBIND11_NAMESPACE pybind11 __attribute__((visibility("hidden"))) +# define PYBIND11_NS_VISIBILITY(...) __VA_ARGS__ __attribute__((visibility("hidden"))) +# define PYBIND11_NAMESPACE PYBIND11_NS_VISIBILITY(pybind11) # else +# define PYBIND11_NS_VISIBILITY(...) __VA_ARGS__ # define PYBIND11_NAMESPACE pybind11 # endif #endif diff --git a/pybind11/setup_helpers.py b/pybind11/setup_helpers.py index 1fd04b9154..1172406b4b 100644 --- a/pybind11/setup_helpers.py +++ b/pybind11/setup_helpers.py @@ -149,7 +149,6 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: if WIN: cflags += ["/EHsc", "/bigobj"] else: - cflags += ["-fvisibility=hidden"] env_cflags = os.environ.get("CFLAGS", "") env_cppflags = os.environ.get("CPPFLAGS", "") c_cpp_flags = shlex.split(env_cflags) + shlex.split(env_cppflags) diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 9379f3f259..92182a646a 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -48,7 +48,7 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { if (p) { std::rethrow_exception(p); } - } catch (const shared_exception &e) { + } catch (const pybind11_tests::shared_exception &e) { PyErr_SetString(PyExc_KeyError, e.what()); } }); diff --git a/tests/test_custom_type_setup.cpp b/tests/test_custom_type_setup.cpp index 42fae05d5d..057f4ad1f1 100644 --- a/tests/test_custom_type_setup.cpp +++ b/tests/test_custom_type_setup.cpp @@ -13,14 +13,19 @@ namespace py = pybind11; +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests)) namespace { struct OwnsPythonObjects { py::object value = py::none(); }; + } // namespace +PYBIND11_NAMESPACE_END(pybind11_tests) TEST_SUBMODULE(custom_type_setup, m) { + using namespace pybind11_tests; + py::class_ cls( m, "OwnsPythonObjects", py::custom_type_setup([](PyHeapTypeObject *heap_type) { auto *type = &heap_type->ht_type; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 3ec999d1dc..25f48f7802 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -15,6 +15,8 @@ #include #include +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests)) + // A type that should be raised as an exception in Python class MyException : public std::exception { public: @@ -110,7 +112,11 @@ std::string error_already_set_what(const py::object &exc_type, const py::object return py::error_already_set().what(); } +PYBIND11_NAMESPACE_END(pybind11_tests) + TEST_SUBMODULE(exceptions, m) { + using namespace pybind11_tests; + m.def("throw_std_exception", []() { throw std::runtime_error("This exception was intentionally thrown."); }); diff --git a/tests/test_exceptions.h b/tests/test_exceptions.h index 03684b89fa..adad7c6c4e 100644 --- a/tests/test_exceptions.h +++ b/tests/test_exceptions.h @@ -5,9 +5,13 @@ // shared exceptions for cross_module_tests +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests)) + class PYBIND11_EXPORT_EXCEPTION shared_exception : public pybind11::builtin_exception { public: using builtin_exception::builtin_exception; explicit shared_exception() : shared_exception("") {} void set_error() const override { PyErr_SetString(PyExc_RuntimeError, what()); } }; + +PYBIND11_NAMESPACE_END(pybind11_tests) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 69ddbe1ef2..81639f06ec 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -15,6 +15,8 @@ #include #include +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests)) + // Size / dtype checks. struct DtypeCheck { py::dtype numpy{}; @@ -159,7 +161,11 @@ py::handle auxiliaries(T &&r, T2 &&r2) { // note: declaration at local scope would create a dangling reference! static int data_i = 42; +PYBIND11_NAMESPACE_END(pybind11_tests) + TEST_SUBMODULE(numpy_array, sm) { + using namespace pybind11_tests; + try { py::module_::import("numpy"); } catch (const py::error_already_set &) { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index f532e26085..a2625b103b 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -11,7 +11,7 @@ #include -namespace external { +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(external)) namespace detail { bool check(PyObject *o) { return PyFloat_Check(o) != 0; } @@ -37,7 +37,7 @@ class float_ : public py::object { double get_value() const { return PyFloat_AsDouble(this->ptr()); } }; -} // namespace external +PYBIND11_NAMESPACE_END(external) namespace implicit_conversion_from_0_to_handle { // Uncomment to trigger compiler error. Note: Before PR #4008 this used to compile successfully. diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake index abba0fe0e2..52306d0cfa 100644 --- a/tools/pybind11NewTools.cmake +++ b/tools/pybind11NewTools.cmake @@ -203,19 +203,6 @@ function(pybind11_add_module target_name) target_link_libraries(${target_name} PRIVATE pybind11::windows_extras) endif() - # -fvisibility=hidden is required to allow multiple modules compiled against - # different pybind versions to work properly, and for some features (e.g. - # py::module_local). We force it on everything inside the `pybind11` - # namespace; also turning it on for a pybind module compilation here avoids - # potential warnings or issues from having mixed hidden/non-hidden types. - if(NOT DEFINED CMAKE_CXX_VISIBILITY_PRESET) - set_target_properties(${target_name} PROPERTIES CXX_VISIBILITY_PRESET "hidden") - endif() - - if(NOT DEFINED CMAKE_CUDA_VISIBILITY_PRESET) - set_target_properties(${target_name} PROPERTIES CUDA_VISIBILITY_PRESET "hidden") - endif() - # If we don't pass a WITH_SOABI or WITHOUT_SOABI, use our own default handling of extensions if(NOT ARG_WITHOUT_SOABI AND NOT "WITH_SOABI" IN_LIST ARG_UNPARSED_ARGUMENTS) pybind11_extension(${target_name}) diff --git a/tools/pybind11Tools.cmake b/tools/pybind11Tools.cmake index 5535e872f3..542fd92d68 100644 --- a/tools/pybind11Tools.cmake +++ b/tools/pybind11Tools.cmake @@ -183,19 +183,6 @@ function(pybind11_add_module target_name) pybind11_extension(${target_name}) - # -fvisibility=hidden is required to allow multiple modules compiled against - # different pybind versions to work properly, and for some features (e.g. - # py::module_local). We force it on everything inside the `pybind11` - # namespace; also turning it on for a pybind module compilation here avoids - # potential warnings or issues from having mixed hidden/non-hidden types. - if(NOT DEFINED CMAKE_CXX_VISIBILITY_PRESET) - set_target_properties(${target_name} PROPERTIES CXX_VISIBILITY_PRESET "hidden") - endif() - - if(NOT DEFINED CMAKE_CUDA_VISIBILITY_PRESET) - set_target_properties(${target_name} PROPERTIES CUDA_VISIBILITY_PRESET "hidden") - endif() - if(ARG_NO_EXTRAS) return() endif()