Skip to content

Commit

Permalink
[smart_holder] Bake smart_holder functionality into class_ and `typ…
Browse files Browse the repository at this point in the history
…e_caster_base` (#5257)

* Put bakein branch @ 18b72c0 on top of smart_holder branch:

Commands used:

```
git checkout bakein
git diff smart_holder > ~/zd
git checkout smart_holder
git checkout -b bakein_sh
patch -p 1 < ~/zd
git checkout smart_holder \
MANIFEST.in \
README.rst \
README_smart_holder.rst \
docs/advanced/smart_ptrs.rst \
ubench/holder_comparison.cpp \
ubench/holder_comparison.py \
ubench/holder_comparison_extract_sheet_data.py \
ubench/number_bucket.h \
ubench/python/number_bucket.clif
git add -A
```

* Add back README_smart_holder.rst in tests/extra_python_package/test_files.py

* Restore smart_holder_poc.h as-is on smart_holder branch (i.e. undo `PYBIND11_SMART_HOLDER_PADDING`, which was meant for stress-testing only).

* Insert `std::move()` as suggested by @laramiel

* `property_cpp_function_sh_*` named specializations, as suggested by @laramiel (#5257 (comment))

* Call `property_cpp_function_classic` member functions, rather than inlining the implementations.

* Use `PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT` in holder_comparison.cpp (holder_comparison.py is NOT changed accordingly in this commit, i.e. can still only be run if the smart_holder functionality is available).

* Systematically rename `loaded_as` to `load_as` (`shared_ptr`, `unique_ptr`) as suggested by @laramiel

* Make change as suggested by @laramiel. This makes it much more obvious that the latest implementation of `smart_holder_from_unique_ptr()` accepts all existing `return_value_policy` enum values except `copy`.

* Resolve `BAKEIN_WIP: Rewrite comment.` for `property_cpp_function_*` specializations.

* Resolve `BAKEIN_WIP: Add comment to explain: This is meant for stress-testing only.`

* Resolve all remaining BAKEIN_WIP (in pybind11/cast.h).

Leave only two pairs of SMART_HOLDER_BAKEIN_FOLLOW_ON comments: refactoring of copyable_holder_caster, move_only_holder_caster. This is best left until after the smart_holder branch is merged into the master branch.

* Remove obsolete `using holder_type = smart_holder;` in `load_helper`

* Add SMART_HOLDER_BAKEIN_FOLLOW_ON comment for `internals::default_holder`

* README_smart_holder.rst update (line count reduced from 356 to 123).
  • Loading branch information
Ralf W. Grosse-Kunstleve committed Jul 31, 2024
1 parent 6765d4e commit 48f2527
Show file tree
Hide file tree
Showing 68 changed files with 1,334 additions and 1,768 deletions.
3 changes: 1 addition & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,9 @@ set(PYBIND11_HEADERS
include/pybind11/detail/init.h
include/pybind11/detail/internals.h
include/pybind11/detail/smart_holder_poc.h
include/pybind11/detail/smart_holder_sfinae_hooks_only.h
include/pybind11/detail/smart_holder_type_casters.h
include/pybind11/detail/type_caster_base.h
include/pybind11/detail/typeid.h
include/pybind11/detail/using_smart_holder.h
include/pybind11/detail/value_and_holder.h
include/pybind11/attr.h
include/pybind11/buffer_info.h
Expand Down
341 changes: 54 additions & 287 deletions README_smart_holder.rst

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,10 @@ struct type_record {
/// Is the class inheritable from python classes?
bool is_final : 1;

#ifdef PYBIND11_HAVE_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);
if (!base_info) {
Expand Down
281 changes: 249 additions & 32 deletions include/pybind11/cast.h

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@

#pragma once

#define PYBIND11_VERSION_MAJOR 2
#define PYBIND11_VERSION_MINOR 14
#define PYBIND11_VERSION_MAJOR 3
#define PYBIND11_VERSION_MINOR 0
#define PYBIND11_VERSION_PATCH 0.dev1

// Similar to Python's convention: https://docs.python.org/3/c-api/apiabiversion.html
// Additional convention: 0xD = dev
#define PYBIND11_VERSION_HEX 0x020E00D1
#define PYBIND11_VERSION_HEX 0x030000D1

// Define some generic pybind11 helper macros for warning management.
//
Expand Down
41 changes: 22 additions & 19 deletions include/pybind11/detail/init.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#pragma once

#include "class.h"
#include "smart_holder_sfinae_hooks_only.h"
#include "using_smart_holder.h"

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

Expand Down Expand Up @@ -156,9 +156,7 @@ void construct(value_and_holder &v_h, Alias<Class> *alias_ptr, bool) {
// holder. This also handles types like std::shared_ptr<T> and std::unique_ptr<T> where T is a
// derived type (through those holder's implicit conversion from derived class holder
// constructors).
template <typename Class,
detail::enable_if_t<!detail::type_uses_smart_holder_type_caster<Cpp<Class>>::value, int>
= 0>
template <typename Class, detail::enable_if_t<!is_smart_holder<Holder<Class>>::value, int> = 0>
void construct(value_and_holder &v_h, Holder<Class> holder, bool need_alias) {
PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
auto *ptr = holder_helper<Holder<Class>>::get(holder);
Expand Down Expand Up @@ -200,10 +198,18 @@ void construct(value_and_holder &v_h, Alias<Class> &&result, bool) {
v_h.value_ptr() = new Alias<Class>(std::move(result));
}

#ifdef PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT

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) {
void *void_ptr = void_cast_raw_ptr ? static_cast<void *>(unq_ptr.get()) : nullptr;
return smart_holder::from_unique_ptr(std::move(unq_ptr), void_ptr);
}

template <typename Class,
typename D = std::default_delete<Cpp<Class>>,
detail::enable_if_t<detail::type_uses_smart_holder_type_caster<Cpp<Class>>::value, int>
= 0>
detail::enable_if_t<is_smart_holder<Holder<Class>>::value, int> = 0>
void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr, bool need_alias) {
PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
auto *ptr = unq_ptr.get();
Expand All @@ -217,30 +223,27 @@ void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr,
// trampoline Python object alive. For types that don't inherit from enable_shared_from_this
// it does not matter if void_cast_raw_ptr is true or false, therefore it's not necessary
// to also inspect the type.
auto smhldr = type_caster<Cpp<Class>>::smart_holder_from_unique_ptr(
auto smhldr = init_smart_holder_from_unique_ptr(
std::move(unq_ptr), /*void_cast_raw_ptr*/ Class::has_alias && is_alias<Class>(ptr));
v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr);
}

template <typename Class,
typename D = std::default_delete<Alias<Class>>,
detail::enable_if_t<detail::type_uses_smart_holder_type_caster<Alias<Class>>::value, int>
= 0>
detail::enable_if_t<is_smart_holder<Holder<Class>>::value, int> = 0>
void construct(value_and_holder &v_h,
std::unique_ptr<Alias<Class>, D> &&unq_ptr,
bool /*need_alias*/) {
auto *ptr = unq_ptr.get();
no_nullptr(ptr);
auto smhldr = type_caster<Alias<Class>>::smart_holder_from_unique_ptr(
std::move(unq_ptr), /*void_cast_raw_ptr*/ true);
auto smhldr
= init_smart_holder_from_unique_ptr(std::move(unq_ptr), /*void_cast_raw_ptr*/ true);
v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr);
}

template <typename Class,
detail::enable_if_t<detail::type_uses_smart_holder_type_caster<Cpp<Class>>::value, int>
= 0>
template <typename Class, detail::enable_if_t<is_smart_holder<Holder<Class>>::value, int> = 0>
void construct(value_and_holder &v_h, std::shared_ptr<Cpp<Class>> &&shd_ptr, bool need_alias) {
PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
auto *ptr = shd_ptr.get();
Expand All @@ -249,24 +252,24 @@ void construct(value_and_holder &v_h, std::shared_ptr<Cpp<Class>> &&shd_ptr, boo
throw type_error("pybind11::init(): construction failed: returned std::shared_ptr pointee "
"is not an alias instance");
}
auto smhldr = type_caster<Cpp<Class>>::smart_holder_from_shared_ptr(shd_ptr);
auto smhldr = smart_holder::from_shared_ptr(shd_ptr);
v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr);
}

template <typename Class,
detail::enable_if_t<detail::type_uses_smart_holder_type_caster<Alias<Class>>::value, int>
= 0>
template <typename Class, detail::enable_if_t<is_smart_holder<Holder<Class>>::value, int> = 0>
void construct(value_and_holder &v_h,
std::shared_ptr<Alias<Class>> &&shd_ptr,
bool /*need_alias*/) {
auto *ptr = shd_ptr.get();
no_nullptr(ptr);
auto smhldr = type_caster<Alias<Class>>::smart_holder_from_shared_ptr(shd_ptr);
auto smhldr = smart_holder::from_shared_ptr(shd_ptr);
v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr);
}

#endif // PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT

// Implementing class for py::init<...>()
template <typename... Args>
struct constructor {
Expand Down
42 changes: 26 additions & 16 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#endif

#include "../pytypes.h"
#include "smart_holder_sfinae_hooks_only.h"

#include <exception>
#include <mutex>
Expand All @@ -37,7 +36,9 @@
/// further ABI-incompatible changes may be made before the ABI is officially
/// changed to the new version.
#ifndef PYBIND11_INTERNALS_VERSION
# if PY_VERSION_HEX >= 0x030C0000 || defined(_MSC_VER)
# if PYBIND11_VERSION_MAJOR >= 3
# define PYBIND11_INTERNALS_VERSION 6
# elif PY_VERSION_HEX >= 0x030C0000 || defined(_MSC_VER)
// Version bump for Python 3.12+, before first 3.12 beta release.
// Version bump for MSVC piggy-backed on PR #4779. See comments there.
# define PYBIND11_INTERNALS_VERSION 5
Expand Down Expand Up @@ -237,6 +238,20 @@ struct internals {
}
};

#if PYBIND11_INTERNALS_VERSION >= 6

# define PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT

enum class holder_enum_t : uint8_t {
undefined,
std_unique_ptr, // Default, lacking interop with std::shared_ptr.
std_shared_ptr, // Lacking interop with std::unique_ptr.
smart_holder, // Full std::unique_ptr / std::shared_ptr interop.
custom_holder,
};

#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 All @@ -260,9 +275,14 @@ struct type_info {
/* True if there is no multiple inheritance in this type's inheritance tree */
bool simple_ancestors : 1;
/* for base vs derived holder_type checks */
// SMART_HOLDER_BAKEIN_FOLLOW_ON: Remove default_holder member here and
// produce better error messages in the places where it is currently used.
bool default_holder : 1;
/* true if this is a type registered with py::module_local */
bool module_local : 1;
#ifdef PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT
holder_enum_t holder_enum_v = holder_enum_t::undefined;
#endif
};

/// On MSVC, debug and release builds are not ABI-compatible!
Expand Down Expand Up @@ -322,25 +342,15 @@ struct type_info {
# define PYBIND11_INTERNALS_KIND ""
#endif

/// See README_smart_holder.rst:
/// Classic / Conservative / Progressive cross-module compatibility
#ifndef PYBIND11_INTERNALS_SH_DEF
# if defined(PYBIND11_USE_SMART_HOLDER_AS_DEFAULT)
# define PYBIND11_INTERNALS_SH_DEF "_sh_def"
# else
# define PYBIND11_INTERNALS_SH_DEF ""
# endif
#endif

#define PYBIND11_INTERNALS_ID \
"__pybind11_internals_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
PYBIND11_BUILD_TYPE PYBIND11_INTERNALS_SH_DEF "__"
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB \
PYBIND11_BUILD_ABI PYBIND11_BUILD_TYPE "__"

#define PYBIND11_MODULE_LOCAL_ID \
"__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
PYBIND11_BUILD_TYPE PYBIND11_INTERNALS_SH_DEF "__"
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB \
PYBIND11_BUILD_ABI PYBIND11_BUILD_TYPE "__"

/// Each module locally stores a pointer to the `internals` data. The data
/// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`.
Expand Down
46 changes: 0 additions & 46 deletions include/pybind11/detail/smart_holder_sfinae_hooks_only.h

This file was deleted.

Loading

0 comments on commit 48f2527

Please sign in to comment.