From 4672f2bd151381afd16d5bfcb6544ffd8d2afbc5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 19 Aug 2024 03:07:29 +0700 Subject: [PATCH 1/2] Add `instance::is_alias` and migrate from using `smart_holder::pointee_depends_on_holder_owner` to that. (#5318) --- include/pybind11/cast.h | 18 ++++++++++++++---- include/pybind11/detail/common.h | 5 +++++ include/pybind11/detail/struct_smart_holder.h | 4 +--- include/pybind11/detail/type_caster_base.h | 13 +++++++++++-- include/pybind11/pybind11.h | 7 +++---- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index d3b3fe07..0262296a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -858,8 +858,12 @@ struct copyable_holder_caster< using base::value; bool load(handle src, bool convert) { - return base::template load_impl>>( - src, convert); + if (base::template load_impl>>( + src, convert)) { + sh_load_helper.maybe_set_python_instance_is_alias(src); + return true; + } + return false; } explicit operator std::shared_ptr *() { @@ -914,6 +918,7 @@ struct copyable_holder_caster< void load_value(value_and_holder &&v_h) { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { sh_load_helper.loaded_v_h = v_h; + sh_load_helper.was_populated = true; value = sh_load_helper.get_void_ptr_or_nullptr(); return; } @@ -1041,14 +1046,19 @@ struct move_only_holder_caster< } bool load(handle src, bool convert) { - return base::template load_impl< - move_only_holder_caster>>(src, convert); + if (base::template load_impl< + move_only_holder_caster>>(src, convert)) { + sh_load_helper.maybe_set_python_instance_is_alias(src); + return true; + } + return false; } void load_value(value_and_holder &&v_h) { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { sh_load_helper.loaded_v_h = v_h; sh_load_helper.loaded_v_h.type = typeinfo; + sh_load_helper.was_populated = true; value = sh_load_helper.get_void_ptr_or_nullptr(); return; } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 7b02e83c..4d2cc70a 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -637,6 +637,11 @@ 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 >= 6 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) diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 6fdd95f4..ccd28971 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -136,7 +136,6 @@ struct smart_holder { bool vptr_is_external_shared_ptr : 1; bool is_populated : 1; bool is_disowned : 1; - bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839. // Design choice: smart_holder is movable but not copyable. smart_holder(smart_holder &&) = default; @@ -146,8 +145,7 @@ struct smart_holder { smart_holder() : vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, - vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, - pointee_depends_on_holder_owner{false} {} + vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {} bool has_pointee() const { return vptr != nullptr; } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index cc62cfa8..a3453334 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -704,6 +704,15 @@ inline std::unique_ptr unique_with_deleter(T *raw_ptr, std::unique_ptr template struct load_helper : value_and_holder_helper { + bool was_populated = false; + bool python_instance_is_alias = false; + + void maybe_set_python_instance_is_alias(handle src) { + if (was_populated) { + python_instance_is_alias = reinterpret_cast(src.ptr())->is_alias; + } + } + static std::shared_ptr make_shared_ptr_with_responsible_parent(T *raw_ptr, handle parent) { return std::shared_ptr(raw_ptr, shared_ptr_parent_life_support(parent.ptr())); } @@ -724,7 +733,7 @@ struct load_helper : value_and_holder_helper { throw std::runtime_error("Non-owning holder (load_as_shared_ptr)."); } auto *type_raw_ptr = static_cast(void_raw_ptr); - if (hld.pointee_depends_on_holder_owner) { + if (python_instance_is_alias) { auto *vptr_gd_ptr = std::get_deleter(hld.vptr); if (vptr_gd_ptr != nullptr) { std::shared_ptr released_ptr = vptr_gd_ptr->released_ptr.lock(); @@ -778,7 +787,7 @@ struct load_helper : value_and_holder_helper { auto *self_life_support = dynamic_raw_ptr_cast_if_possible(raw_type_ptr); - if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) { + if (self_life_support == nullptr && python_instance_is_alias) { throw value_error("Alias class (also known as trampoline) does not inherit from " "py::trampoline_self_life_support, therefore the ownership of this " "instance cannot safely be transferred to C++."); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 2a88a94a..9f356828 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2267,7 +2267,8 @@ class class_ : public detail::generic_type { } auto *uninitialized_location = std::addressof(v_h.holder()); auto *value_ptr_w_t = v_h.value_ptr(); - bool pointee_depends_on_holder_owner + // Try downcast from `type` to `type_alias`: + inst->is_alias = detail::dynamic_raw_ptr_cast_if_possible(value_ptr_w_t) != nullptr; if (holder_void_ptr) { // Note: inst->owned ignored. @@ -2277,14 +2278,12 @@ class class_ : public detail::generic_type { uninitialized_location, value_ptr_w_t, value_ptr_w_t)) { if (inst->owned) { new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership( - value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner)); + value_ptr_w_t, /*void_cast_raw_ptr*/ inst->is_alias)); } else { new (uninitialized_location) holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t)); } } - v_h.holder().pointee_depends_on_holder_owner - = pointee_depends_on_holder_owner; v_h.set_holder_constructed(); } From 7d85baa6a194e6c5da46252c49f42ec1d87597c7 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 21 Aug 2024 13:16:49 -0400 Subject: [PATCH 2/2] fix: never use `..` in a header include (#5321) * fix: never use `..` in a header include Signed-off-by: Henry Schreiner * fix: one more parent include Signed-off-by: Henry Schreiner --------- Signed-off-by: Henry Schreiner --- include/pybind11/detail/class.h | 4 ++-- include/pybind11/detail/internals.h | 4 ++-- include/pybind11/detail/type_caster_base.h | 3 ++- include/pybind11/eigen/matrix.h | 3 ++- include/pybind11/eigen/tensor.h | 3 ++- include/pybind11/stl/filesystem.h | 10 +++++----- 6 files changed, 15 insertions(+), 12 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 485f7ac6..454c186b 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -9,8 +9,8 @@ #pragma once -#include "../attr.h" -#include "../options.h" +#include +#include PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 329d89b9..17e6df82 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -12,10 +12,10 @@ #include "common.h" #if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) -# include "../gil.h" +# include #endif -#include "../pytypes.h" +#include #include #include diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 481b7c78..bc42252f 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -9,7 +9,8 @@ #pragma once -#include "../pytypes.h" +#include + #include "common.h" #include "descr.h" #include "internals.h" diff --git a/include/pybind11/eigen/matrix.h b/include/pybind11/eigen/matrix.h index 8d4342f8..5cf1f0a2 100644 --- a/include/pybind11/eigen/matrix.h +++ b/include/pybind11/eigen/matrix.h @@ -9,7 +9,8 @@ #pragma once -#include "../numpy.h" +#include + #include "common.h" /* HINT: To suppress warnings originating from the Eigen headers, use -isystem. diff --git a/include/pybind11/eigen/tensor.h b/include/pybind11/eigen/tensor.h index 14bb91c4..0a9d7c25 100644 --- a/include/pybind11/eigen/tensor.h +++ b/include/pybind11/eigen/tensor.h @@ -7,7 +7,8 @@ #pragma once -#include "../numpy.h" +#include + #include "common.h" #if defined(__GNUC__) && !defined(__clang__) && !defined(__INTEL_COMPILER) diff --git a/include/pybind11/stl/filesystem.h b/include/pybind11/stl/filesystem.h index 935d7948..c16a9ae5 100644 --- a/include/pybind11/stl/filesystem.h +++ b/include/pybind11/stl/filesystem.h @@ -4,11 +4,11 @@ #pragma once -#include "../pybind11.h" -#include "../detail/common.h" -#include "../detail/descr.h" -#include "../cast.h" -#include "../pytypes.h" +#include +#include +#include +#include +#include #include