Skip to content

[smart_holder] Introduce PYBIND11_SMART_HOLDER_DISABLE option. #5348

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

Merged
merged 4 commits into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ jobs:
# Extra ubuntu latest job
- runs-on: ubuntu-latest
python: '3.11'
# Exercise PYBIND11_USE_SMART_HOLDER_AS_DEFAULT
# with recent (or ideally latest) released Python version.
- runs-on: ubuntu-latest
python: '3.12'
args: >
Expand All @@ -80,6 +82,20 @@ jobs:
python: '3.12'
args: >
-DCMAKE_CXX_FLAGS="/DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT /GR /EHsc"
# Exercise PYBIND11_SMART_HOLDER_DISABLE
# with recent (or ideally latest) released Python version.
- runs-on: ubuntu-latest
python: '3.12'
args: >
-DCMAKE_CXX_FLAGS="-DPYBIND11_SMART_HOLDER_DISABLE"
- runs-on: macos-13
python: '3.12'
args: >
-DCMAKE_CXX_FLAGS="-DPYBIND11_SMART_HOLDER_DISABLE"
- runs-on: windows-2022
python: '3.12'
args: >
-DCMAKE_CXX_FLAGS="/DPYBIND11_SMART_HOLDER_DISABLE /GR /EHsc"

name: "🐍 ${{ matrix.python }} • ${{ matrix.runs-on }} • x64 ${{ matrix.args }}"
runs-on: ${{ matrix.runs-on }}
Expand Down
10 changes: 5 additions & 5 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ struct copyable_holder_caster : public type_caster_base<type> {
holder_type holder;
};

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

template <typename, typename SFINAE = void>
struct copyable_holder_caster_shared_ptr_with_smart_holder_support_enabled : std::true_type {};
Expand Down Expand Up @@ -968,7 +968,7 @@ struct copyable_holder_caster<
std::shared_ptr<type> shared_ptr_storage;
};

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

/// Specialize for the common std::shared_ptr, so users don't need to
template <typename T>
Expand All @@ -990,7 +990,7 @@ struct move_only_holder_caster {
static constexpr auto name = type_caster_base<type>::name;
};

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

template <typename, typename SFINAE = void>
struct move_only_holder_caster_unique_ptr_with_smart_holder_support_enabled : std::true_type {};
Expand Down Expand Up @@ -1129,7 +1129,7 @@ struct move_only_holder_caster<
std::shared_ptr<std::unique_ptr<type, deleter>> unique_ptr_storage;
};

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

template <typename type, typename deleter>
class type_caster<std::unique_ptr<type, deleter>>
Expand Down Expand Up @@ -1167,7 +1167,7 @@ struct is_holder_type
template <typename base, typename deleter>
struct is_holder_type<base, std::unique_ptr<base, deleter>> : std::true_type {};

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
template <typename base>
struct is_holder_type<base, smart_holder> : std::true_type {};
#endif
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/init.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void construct(value_and_holder &v_h, Alias<Class> &&result, bool) {
v_h.value_ptr() = new Alias<Class>(std::move(result));
}

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

template <typename T, typename D>
smart_holder init_smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&unq_ptr,
Expand Down Expand Up @@ -268,7 +268,7 @@ void construct(value_and_holder &v_h,
v_h.type->init_instance(v_h.inst, &smhldr);
}

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

// Implementing class for py::init<...>()
template <typename... Args>
Expand Down
5 changes: 5 additions & 0 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,11 @@ enum class holder_enum_t : uint8_t {

#endif

#if defined(PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT) \
&& !defined(PYBIND11_SMART_HOLDER_DISABLE)
# define PYBIND11_SMART_HOLDER_ENABLED
#endif

/// Additional type information which does not fit into the PyTypeObject.
/// Changes to this struct also require bumping `PYBIND11_INTERNALS_VERSION`.
struct type_info {
Expand Down
6 changes: 3 additions & 3 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ inline PyThreadState *get_thread_state_unchecked() {
void keep_alive_impl(handle nurse, handle patient);
inline PyObject *make_new_instance(PyTypeObject *type);

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

// PYBIND11:REMINDER: Needs refactoring of existing pybind11 code.
inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
Expand Down Expand Up @@ -830,7 +830,7 @@ struct load_helper : value_and_holder_helper {

PYBIND11_NAMESPACE_END(smart_holder_type_caster_support)

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

class type_caster_generic {
public:
Expand Down Expand Up @@ -936,7 +936,7 @@ class type_caster_generic {

// Base methods for generic caster; there are overridden in copyable_holder_caster
void load_value(value_and_holder &&v_h) {
#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
smart_holder_type_caster_support::value_and_holder_helper v_h_helper;
v_h_helper.loaded_v_h = v_h;
Expand Down
6 changes: 3 additions & 3 deletions include/pybind11/detail/using_smart_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@

#include <type_traits>

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
# include "struct_smart_holder.h"
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
using pybindit::memory::smart_holder;
#endif

PYBIND11_NAMESPACE_BEGIN(detail)

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
template <typename H>
using is_smart_holder = std::is_same<H, smart_holder>;
#else
Expand Down
15 changes: 7 additions & 8 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1631,7 +1631,7 @@ PYBIND11_NAMESPACE_END(detail)
template <typename T, typename D, typename SFINAE = void>
struct property_cpp_function : detail::property_cpp_function_classic<T, D> {};

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

PYBIND11_NAMESPACE_BEGIN(detail)

Expand Down Expand Up @@ -1810,10 +1810,9 @@ struct property_cpp_function<
detail::both_t_and_d_use_type_caster_base<T, typename D::element_type>>::value>>
: detail::property_cpp_function_sh_unique_ptr_member<T, D> {};

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

#if defined(PYBIND11_USE_SMART_HOLDER_AS_DEFAULT) \
&& defined(PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT)
#if defined(PYBIND11_USE_SMART_HOLDER_AS_DEFAULT) && defined(PYBIND11_SMART_HOLDER_ENABLED)
// NOTE: THIS IS MEANT FOR STRESS-TESTING ONLY!
// As of PR #5257, for production use, there is no longer a strong reason to make
// smart_holder the default holder:
Expand Down Expand Up @@ -1881,7 +1880,7 @@ class class_ : public detail::generic_type {
// A more fitting name would be uses_unique_ptr_holder.
record.default_holder = detail::is_instantiation<std::unique_ptr, holder_type>::value;

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
if (detail::is_instantiation<std::unique_ptr, holder_type>::value) {
record.holder_enum_v = detail::holder_enum_t::std_unique_ptr;
} else if (detail::is_instantiation<std::shared_ptr, holder_type>::value) {
Expand Down Expand Up @@ -2226,7 +2225,7 @@ class class_ : public detail::generic_type {
init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr<type>());
}

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

template <typename WrappedType>
static bool try_initialization_using_shared_from_this(holder_type *, WrappedType *, ...) {
Expand Down Expand Up @@ -2287,7 +2286,7 @@ class class_ : public detail::generic_type {
v_h.set_holder_constructed();
}

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED

/// Deallocates an instance; via holder, if constructed; otherwise via operator delete.
static void dealloc(detail::value_and_holder &v_h) {
Expand Down Expand Up @@ -2329,7 +2328,7 @@ class class_ : public detail::generic_type {
}
};

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

// Supports easier switching between py::class_<T> and py::class_<T, py::smart_holder>:
// users can simply replace the `_` in `class_` with `h` or vice versa.
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/trampoline_self_life_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#include "detail/internals.h"

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED

# include "detail/common.h"
# include "detail/using_smart_holder.h"
Expand Down Expand Up @@ -63,4 +63,4 @@ struct trampoline_self_life_support {

PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
2 changes: 1 addition & 1 deletion tests/test_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ TEST_SUBMODULE(class_, m) {
struct ToBeHeldByUniquePtr {};
py::class_<ToBeHeldByUniquePtr, std::unique_ptr<ToBeHeldByUniquePtr>>(m, "ToBeHeldByUniquePtr")
.def(py::init<>());
#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#ifdef PYBIND11_SMART_HOLDER_ENABLED
m.def("pass_unique_ptr", [](std::unique_ptr<ToBeHeldByUniquePtr> &&) {});
#else
m.attr("pass_unique_ptr") = py::none();
Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ namespace pybind11_tests {
namespace class_sh_basic {

TEST_SUBMODULE(class_sh_basic, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand Down Expand Up @@ -260,7 +260,7 @@ TEST_SUBMODULE(class_sh_basic, m) {
[]() { return CastUnusualOpRefConstRef(LocalUnusualOpRef()); });
m.def("CallCastUnusualOpRefMovable",
[]() { return CastUnusualOpRefMovable(LocalUnusualOpRef()); });
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}

} // namespace class_sh_basic
Expand Down
2 changes: 1 addition & 1 deletion tests/test_class_sh_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pybind11_tests import class_sh_basic as m

if not m.defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT:
if not m.defined_PYBIND11_SMART_HOLDER_ENABLED:
pytest.skip("smart_holder not available.", allow_module_level=True)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_disowning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning::Atype<1>)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning::Atype<2>)

TEST_SUBMODULE(class_sh_disowning, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand All @@ -49,5 +49,5 @@ TEST_SUBMODULE(class_sh_disowning, m) {

m.def("overloaded", (int (*)(std::unique_ptr<Atype<1>>, int)) & overloaded);
m.def("overloaded", (int (*)(std::unique_ptr<Atype<2>>, int)) & overloaded);
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}
2 changes: 1 addition & 1 deletion tests/test_class_sh_disowning.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from pybind11_tests import class_sh_disowning as m

if not m.defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT:
if not m.defined_PYBIND11_SMART_HOLDER_ENABLED:
pytest.skip("smart_holder not available.", allow_module_level=True)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_disowning_mi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning_mi::Base1)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning_mi::Base2)

TEST_SUBMODULE(class_sh_disowning_mi, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand Down Expand Up @@ -98,5 +98,5 @@ TEST_SUBMODULE(class_sh_disowning_mi, m) {
py::classh<Base2>(m, "Base2").def(py::init<int>()).def("bar", &Base2::bar);
m.def("disown_base1", disown_base1);
m.def("disown_base2", disown_base2);
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}
2 changes: 1 addition & 1 deletion tests/test_class_sh_disowning_mi.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import env # noqa: F401
from pybind11_tests import class_sh_disowning_mi as m

if not m.defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT:
if not m.defined_PYBIND11_SMART_HOLDER_ENABLED:
pytest.skip("smart_holder not available.", allow_module_level=True)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_factory_constructors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::with_alias)

TEST_SUBMODULE(class_sh_factory_constructors, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand Down Expand Up @@ -183,5 +183,5 @@ TEST_SUBMODULE(class_sh_factory_constructors, m) {
[](int, int, int, int, int) {
return std::make_shared<with_alias>(); // Invalid alias factory.
}));
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}
2 changes: 1 addition & 1 deletion tests/test_class_sh_factory_constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from pybind11_tests import class_sh_factory_constructors as m

if not m.defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT:
if not m.defined_PYBIND11_SMART_HOLDER_ENABLED:
pytest.skip("smart_holder not available.", allow_module_level=True)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_inheritance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ namespace pybind11_tests {
namespace class_sh_inheritance {

TEST_SUBMODULE(class_sh_inheritance, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand Down Expand Up @@ -105,7 +105,7 @@ TEST_SUBMODULE(class_sh_inheritance, m) {
m.def("pass_cptr_base1", pass_cptr_base1);
m.def("pass_cptr_base2", pass_cptr_base2);
m.def("pass_cptr_drvd2", pass_cptr_drvd2);
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}

} // namespace class_sh_inheritance
Expand Down
2 changes: 1 addition & 1 deletion tests/test_class_sh_inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from pybind11_tests import class_sh_inheritance as m

if not m.defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT:
if not m.defined_PYBIND11_SMART_HOLDER_ENABLED:
pytest.skip("smart_holder not available.", allow_module_level=True)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_class_sh_mi_thunks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_mi_thunks::Base1)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_mi_thunks::Derived)

TEST_SUBMODULE(class_sh_mi_thunks, m) {
m.attr("defined_PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT") =
#ifndef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
m.attr("defined_PYBIND11_SMART_HOLDER_ENABLED") =
#ifndef PYBIND11_SMART_HOLDER_ENABLED
false;
#else
true;
Expand Down Expand Up @@ -103,5 +103,5 @@ TEST_SUBMODULE(class_sh_mi_thunks, m) {
}
return obj_der->vec.size();
});
#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
#endif // PYBIND11_SMART_HOLDER_ENABLED
}
Loading