Skip to content
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

[smart_holder] Remove -fvisibility-hidden experiment #4069

Draft
wants to merge 9 commits into
base: smart_holder
Choose a base branch
from
4 changes: 3 additions & 1 deletion include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pybind11/setup_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/pybind11_cross_module_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
});
Expand Down
4 changes: 2 additions & 2 deletions tests/test_class_sh_trampoline_basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include <memory>

namespace pybind11_tests {
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace class_sh_trampoline_basic {

template <int SerNo> // Using int as a trick to easily generate a series of types.
Expand Down Expand Up @@ -74,7 +74,7 @@ void wrap(py::module_ m, const char *py_class_name) {
}

} // namespace class_sh_trampoline_basic
} // namespace pybind11_tests
PYBIND11_NAMESPACE_END(pybind11_tests)

PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<0>)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<1>)
Expand Down
6 changes: 5 additions & 1 deletion tests/test_class_sh_trampoline_self_life_support.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>
#include <utility>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace {

struct Big5 { // Also known as "rule of five".
Expand Down Expand Up @@ -42,10 +43,13 @@ struct Big5Trampoline : Big5, py::trampoline_self_life_support {
};

} // namespace
PYBIND11_NAMESPACE_END(pybind11_tests)

PYBIND11_SMART_HOLDER_TYPE_CASTERS(Big5)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::Big5)

TEST_SUBMODULE(class_sh_trampoline_self_life_support, m) {
using namespace pybind11_tests;

py::classh<Big5, Big5Trampoline>(m, "Big5")
.def(py::init<std::string>())
.def_readonly("history", &Big5::history);
Expand Down
8 changes: 6 additions & 2 deletions tests/test_class_sh_trampoline_shared_from_this.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <string>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace {

struct Sft : std::enable_shared_from_this<Sft> {
Expand Down Expand Up @@ -99,11 +100,14 @@ std::shared_ptr<Sft> make_pure_cpp_sft_shd_ptr(const std::string &history_seed)
std::shared_ptr<Sft> pass_through_shd_ptr(const std::shared_ptr<Sft> &obj) { return obj; }

} // namespace
PYBIND11_NAMESPACE_END(pybind11_tests)

PYBIND11_SMART_HOLDER_TYPE_CASTERS(Sft)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(SftSharedPtrStash)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::Sft)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::SftSharedPtrStash)

TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {
using namespace pybind11_tests;

py::classh<Sft, SftTrampoline>(m, "Sft")
.def(py::init<const std::string &>())
.def(py::init([](const std::string &history, int) {
Expand Down
8 changes: 7 additions & 1 deletion tests/test_class_sh_trampoline_unique_ptr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <cstdint>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace {

class Class {
Expand All @@ -31,9 +32,11 @@ class Class {
};

} // namespace
PYBIND11_NAMESPACE_END(pybind11_tests)

PYBIND11_SMART_HOLDER_TYPE_CASTERS(Class)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::Class)

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace {

class PyClass : public Class, public py::trampoline_self_life_support {
Expand All @@ -46,8 +49,11 @@ class PyClass : public Class, public py::trampoline_self_life_support {
};

} // namespace
PYBIND11_NAMESPACE_END(pybind11_tests)

TEST_SUBMODULE(class_sh_trampoline_unique_ptr, m) {
using namespace pybind11_tests;

py::classh<Class, PyClass>(m, "Class")
.def(py::init<>())
.def("set_val", &Class::setVal)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_class_sh_virtual_py_cpp_mix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include <memory>

namespace pybind11_tests {
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
namespace class_sh_virtual_py_cpp_mix {

class Base {
Expand Down Expand Up @@ -44,7 +44,7 @@ struct CppDerivedVirtualOverrider : CppDerived, py::trampoline_self_life_support
};

} // namespace class_sh_virtual_py_cpp_mix
} // namespace pybind11_tests
PYBIND11_NAMESPACE_END(pybind11_tests)

PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::Base)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::CppDerivedPlain)
Expand Down
5 changes: 5 additions & 0 deletions tests/test_custom_type_setup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_<OwnsPythonObjects> cls(
m, "OwnsPythonObjects", py::custom_type_setup([](PyHeapTypeObject *heap_type) {
auto *type = &heap_type->ht_type;
Expand Down
6 changes: 6 additions & 0 deletions tests/test_exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <stdexcept>
#include <utility>

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:
Expand Down Expand Up @@ -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."); });

Expand Down
4 changes: 4 additions & 0 deletions tests/test_exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 6 additions & 0 deletions tests/test_numpy_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <cstdint>
#include <utility>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))

// Size / dtype checks.
struct DtypeCheck {
py::dtype numpy{};
Expand Down Expand Up @@ -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 &) {
Expand Down
4 changes: 2 additions & 2 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

#include <utility>

namespace external {
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(external))
namespace detail {
bool check(PyObject *o) { return PyFloat_Check(o) != 0; }

Expand All @@ -37,7 +37,7 @@ class float_ : public py::object {

double get_value() const { return PyFloat_AsDouble(this->ptr()); }
};
} // namespace external
PYBIND11_NAMESPACE_END(external)

TEST_SUBMODULE(pytypes, m) {
// test_bool
Expand Down
13 changes: 0 additions & 13 deletions tools/pybind11NewTools.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
13 changes: 0 additions & 13 deletions tools/pybind11Tools.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down