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] Simplification: Enable smart_holder functionality unconditionally. #5531

Merged
merged 15 commits into from
Feb 22, 2025
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
14 changes: 0 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,6 @@ 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"
exclude:
# The setup-python action currently doesn't have graalpy for windows
- python: 'graalpy-24.1'
Expand Down
2 changes: 0 additions & 2 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,7 @@ struct type_record {
/// Solves pybind/pybind11#1446
bool release_gil_before_calling_cpp_dtor : 1;

#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
holder_enum_t holder_enum_v = holder_enum_t::undefined;
#endif

PYBIND11_NOINLINE void add_base(const std::type_info &base, void *(*caster)(void *) ) {
auto *base_info = detail::get_type_info(base, false);
Expand Down
16 changes: 3 additions & 13 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -836,8 +836,6 @@ struct copyable_holder_caster : public type_caster_base<type> {
holder_type holder;
};

#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 @@ -928,13 +926,13 @@ struct copyable_holder_caster<
return;
}
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
# if !defined(PYBIND11_DETAILED_ERROR_MESSAGES)
#if !defined(PYBIND11_DETAILED_ERROR_MESSAGES)
"(#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for "
"type information)");
# else
#else
"of type '"
+ type_id<std::shared_ptr<type>>() + "''");
# endif
#endif
}

template <typename T = std::shared_ptr<type>,
Expand Down Expand Up @@ -968,8 +966,6 @@ struct copyable_holder_caster<
std::shared_ptr<type> shared_ptr_storage;
};

#endif // PYBIND11_SMART_HOLDER_ENABLED

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

#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,8 +1123,6 @@ struct move_only_holder_caster<
std::shared_ptr<std::unique_ptr<type, deleter>> unique_ptr_storage;
};

#endif // PYBIND11_SMART_HOLDER_ENABLED

template <typename type, typename deleter>
class type_caster<std::unique_ptr<type, deleter>>
: public move_only_holder_caster<type, std::unique_ptr<type, deleter>> {};
Expand Down Expand Up @@ -1169,10 +1161,8 @@ struct is_holder_type
template <typename base, typename deleter>
struct is_holder_type<base, std::unique_ptr<base, deleter>> : std::true_type {};

#ifdef PYBIND11_SMART_HOLDER_ENABLED
template <typename base>
struct is_holder_type<base, smart_holder> : std::true_type {};
#endif

#ifdef PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION // See PR #4888

Expand Down
3 changes: 0 additions & 3 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,8 @@ struct instance {
bool simple_instance_registered : 1;
/// If true, get_internals().patients has an entry for this object
bool has_patients : 1;
// Cannot use PYBIND11_INTERNALS_VERSION >= 106 here without refactoring.
#if PYBIND11_VERSION_MAJOR >= 3
/// If true, this Python object needs to be kept alive for the lifetime of the C++ value.
bool is_alias : 1;
#endif

/// Initializes all of the above type/values/holders data (but not the instance values
/// themselves)
Expand Down
4 changes: 0 additions & 4 deletions include/pybind11/detail/init.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ void construct(value_and_holder &v_h, Alias<Class> &&result, bool) {
v_h.value_ptr() = new Alias<Class>(std::move(result));
}

#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,
bool void_cast_raw_ptr) {
Expand Down Expand Up @@ -268,8 +266,6 @@ void construct(value_and_holder &v_h,
v_h.type->init_instance(v_h.inst, &smhldr);
}

#endif // PYBIND11_SMART_HOLDER_ENABLED

// Implementing class for py::init<...>()
template <typename... Args>
struct constructor {
Expand Down
23 changes: 3 additions & 20 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,11 @@
/// further ABI-incompatible changes may be made before the ABI is officially
/// changed to the new version.
#ifndef PYBIND11_INTERNALS_VERSION
# if PYBIND11_VERSION_MAJOR >= 3
# define PYBIND11_INTERNALS_VERSION 106
# else
# define PYBIND11_INTERNALS_VERSION 6
# endif
# define PYBIND11_INTERNALS_VERSION 106
#endif

#if PYBIND11_INTERNALS_VERSION < 6
# error "PYBIND11_INTERNALS_VERSION 6 is the minimum for all platforms for pybind11v3."
#if PYBIND11_INTERNALS_VERSION < 106
# error "PYBIND11_INTERNALS_VERSION 106 is the minimum (SPECIAL SITUATION)."
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
Expand Down Expand Up @@ -215,10 +211,6 @@ struct internals {
}
};

#if PYBIND11_INTERNALS_VERSION >= 106

# define PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT

enum class holder_enum_t : uint8_t {
undefined,
std_unique_ptr, // Default, lacking interop with std::shared_ptr.
Expand All @@ -227,13 +219,6 @@ enum class holder_enum_t : uint8_t {
custom_holder,
};

#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 Expand Up @@ -262,9 +247,7 @@ struct type_info {
bool default_holder : 1;
/* true if this is a type registered with py::module_local */
bool module_local : 1;
#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
holder_enum_t holder_enum_v = holder_enum_t::undefined;
#endif
};

#define PYBIND11_INTERNALS_ID \
Expand Down
6 changes: 0 additions & 6 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,6 @@ inline PyThreadState *get_thread_state_unchecked() {
void keep_alive_impl(handle nurse, handle patient);
inline PyObject *make_new_instance(PyTypeObject *type);

#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 @@ -868,8 +866,6 @@ struct load_helper : value_and_holder_helper {

PYBIND11_NAMESPACE_END(smart_holder_type_caster_support)

#endif // PYBIND11_SMART_HOLDER_ENABLED

class type_caster_generic {
public:
PYBIND11_NOINLINE explicit type_caster_generic(const std::type_info &type_info)
Expand Down Expand Up @@ -974,7 +970,6 @@ 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_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 All @@ -984,7 +979,6 @@ class type_caster_generic {
return;
}
}
#endif
auto *&vptr = v_h.value_ptr();
// Lazy allocation for unallocated values:
if (vptr == nullptr) {
Expand Down
13 changes: 1 addition & 12 deletions include/pybind11/detail/using_smart_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,18 @@
#pragma once

#include "common.h"
#include "internals.h"
#include "struct_smart_holder.h"

#include <type_traits>

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

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

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

PYBIND11_NAMESPACE_BEGIN(detail)

#ifdef PYBIND11_SMART_HOLDER_ENABLED
template <typename H>
using is_smart_holder = std::is_same<H, smart_holder>;
#else
template <typename>
struct is_smart_holder : std::false_type {};
#endif

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
1 change: 1 addition & 0 deletions include/pybind11/detail/value_and_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "common.h"

#include <cstddef>
#include <cstdint>
#include <typeinfo>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
Expand Down
19 changes: 1 addition & 18 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1448,9 +1448,7 @@ class generic_type : public object {
tinfo->simple_ancestors = true;
tinfo->default_holder = rec.default_holder;
tinfo->module_local = rec.module_local;
#ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
tinfo->holder_enum_v = rec.holder_enum_v;
#endif

with_internals([&](internals &internals) {
auto tindex = std::type_index(*rec.type);
Expand Down Expand Up @@ -1665,8 +1663,6 @@ 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_SMART_HOLDER_ENABLED

PYBIND11_NAMESPACE_BEGIN(detail)

template <typename T, typename D, typename SFINAE = void>
Expand Down Expand Up @@ -1844,17 +1840,14 @@ 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_SMART_HOLDER_ENABLED

#if defined(PYBIND11_USE_SMART_HOLDER_AS_DEFAULT) && defined(PYBIND11_SMART_HOLDER_ENABLED)
#if defined(PYBIND11_USE_SMART_HOLDER_AS_DEFAULT)
// 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:
// Simply use `py::classh` (see below) instead of `py::class_` as needed.
// Running the pybind11 unit tests with smart_holder as the default holder is to ensure
// that `py::smart_holder` / `py::classh` is backward-compatible with all pre-existing
// functionality.
# define PYBIND11_ACTUALLY_USING_SMART_HOLDER_AS_DEFAULT
template <typename>
using default_holder_type = smart_holder;
#else
Expand Down Expand Up @@ -1913,7 +1906,6 @@ 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_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 All @@ -1923,7 +1915,6 @@ class class_ : public detail::generic_type {
} else {
record.holder_enum_v = detail::holder_enum_t::custom_holder;
}
#endif

set_operator_new<type>(&record);

Expand Down Expand Up @@ -2265,8 +2256,6 @@ class class_ : public detail::generic_type {
init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr<type>());
}

#ifdef PYBIND11_SMART_HOLDER_ENABLED

template <typename WrappedType>
static bool try_initialization_using_shared_from_this(holder_type *, WrappedType *, ...) {
return false;
Expand Down Expand Up @@ -2326,8 +2315,6 @@ class class_ : public detail::generic_type {
v_h.set_holder_constructed();
}

#endif // PYBIND11_SMART_HOLDER_ENABLED

// Deallocates an instance; via holder, if constructed; otherwise via operator delete.
// NOTE: The Python error indicator needs to cleared BEFORE this function is called.
// This is because we could be deallocating while cleaning up after a Python exception.
Expand Down Expand Up @@ -2393,8 +2380,6 @@ class class_ : public detail::generic_type {
}
};

#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.
template <typename type_, typename... options>
Expand All @@ -2403,8 +2388,6 @@ class classh : public class_<type_, smart_holder, options...> {
using class_<type_, smart_holder, options...>::class_;
};

#endif

/// Binds an existing constructor taking arguments Args...
template <typename... Args>
detail::initimpl::constructor<Args...> init() {
Expand Down
12 changes: 3 additions & 9 deletions include/pybind11/trampoline_self_life_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@

#pragma once

#include "detail/internals.h"

#ifdef PYBIND11_SMART_HOLDER_ENABLED

# include "detail/common.h"
# include "detail/using_smart_holder.h"
# include "detail/value_and_holder.h"
#include "detail/common.h"
#include "detail/using_smart_holder.h"
#include "detail/value_and_holder.h"

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

Expand Down Expand Up @@ -62,5 +58,3 @@ struct trampoline_self_life_support {
};

PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

#endif // PYBIND11_SMART_HOLDER_ENABLED
6 changes: 1 addition & 5 deletions tests/test_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,7 @@ TEST_SUBMODULE(class_, m) {
struct ToBeHeldByUniquePtr {};
py::class_<ToBeHeldByUniquePtr, std::unique_ptr<ToBeHeldByUniquePtr>>(m, "ToBeHeldByUniquePtr")
.def(py::init<>());
#ifdef PYBIND11_SMART_HOLDER_ENABLED
m.def("pass_unique_ptr", [](std::unique_ptr<ToBeHeldByUniquePtr> &&) {});
#else
m.attr("pass_unique_ptr") = py::none();
#endif

// test_inheritance
class Pet {
Expand Down Expand Up @@ -636,7 +632,7 @@ CHECK_NOALIAS(8);
CHECK_HOLDER(1, unique);
CHECK_HOLDER(2, unique);
CHECK_HOLDER(3, unique);
#ifndef PYBIND11_ACTUALLY_USING_SMART_HOLDER_AS_DEFAULT
#ifndef PYBIND11_USE_SMART_HOLDER_AS_DEFAULT
CHECK_HOLDER(4, unique);
CHECK_HOLDER(5, unique);
#endif
Expand Down
2 changes: 0 additions & 2 deletions tests/test_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ def test_instance_new():

def test_pass_unique_ptr():
obj = m.ToBeHeldByUniquePtr()
if m.pass_unique_ptr is None:
pytest.skip("smart_holder not available.")
with pytest.raises(RuntimeError) as execinfo:
m.pass_unique_ptr(obj)
assert str(execinfo.value).startswith(
Expand Down
Loading
Loading