Skip to content

Commit

Permalink
[smart_holder] Remove obsolete detail::type_info::default_holder me…
Browse files Browse the repository at this point in the history
…mber. (#5541)

* git merge --squash purge_internals_versions_4_5

* Remove PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT, set PYBIND11_INTERNALS_VERSION 7

* Remove all uses of PYBIND11_SMART_HOLDER_ENABLED under include/pybind11

* Remove obsolete PYBIND11_ACTUALLY_USING_SMART_HOLDER_AS_DEFAULT macro.

* Remove PYBIND11_SMART_HOLDER_ENABLED in ubench/holder_comparison.cpp

* Remove all uses of PYBIND11_SMART_HOLDER_ENABLED under tests/

* Remove `#define PYBIND11_SMART_HOLDER_ENABLED`

* Remove all uses of PYBIND11_SMART_HOLDER_TYPE_CASTERS under tests/

* Remove all uses of PYBIND11_TYPE_CASTER_BASE_HOLDER under tests/

* Add missing `#include <cstdint>`

Example error message (🐍 3.11 • ubuntu-latest • x64, GNU 13.3.0):

```
include/pybind11/detail/value_and_holder.h:56:52: error: ‘uint8_t’ is not a member of ‘std’; did you mean ‘wint_t’?
   56 |             inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_holder_constructed;
      |                                                    ^~~~~~~
```

* Remove type_info::default_holder member. DOES NOT BUILD

* Remove some obsolete default_holder code and #ifdef out uses of typeinfo->default_holder. BUILDS BUT 2 TESTS ARE FAILING.

* Replace `default_holder` with `holder_enum_v == holder_enum_t::std_unique_ptr`

Intentionally not changing error messages, because this would result in a significantly bigger change.

* Change PYBIND11_INTERNALS_VERSION to 106: It will be changed to 7 in a follow-on PR that actually changes the internals.

* Change PYBIND11_INTERNALS_VERSION to 7 (because this PR actually changes the internals).
  • Loading branch information
rwgk authored Feb 22, 2025
1 parent 5ab036b commit aed215c
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 22 deletions.
16 changes: 8 additions & 8 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,7 @@ struct function_record {
struct type_record {
PYBIND11_NOINLINE type_record()
: multiple_inheritance(false), dynamic_attr(false), buffer_protocol(false),
default_holder(true), module_local(false), is_final(false),
release_gil_before_calling_cpp_dtor(false) {}
module_local(false), is_final(false), release_gil_before_calling_cpp_dtor(false) {}

/// Handle to the parent scope
handle scope;
Expand Down Expand Up @@ -327,9 +326,6 @@ struct type_record {
/// Does the class implement the buffer protocol?
bool buffer_protocol : 1;

/// Is the default (unique_ptr) holder type used?
bool default_holder : 1;

/// Is the class definition local to the module shared object?
bool module_local : 1;

Expand All @@ -350,13 +346,17 @@ struct type_record {
+ "\" referenced unknown base type \"" + tname + "\"");
}

if (default_holder != base_info->default_holder) {
// SMART_HOLDER_BAKEIN_FOLLOW_ON: Refine holder compatibility checks.
bool this_has_unique_ptr_holder = (holder_enum_v == holder_enum_t::std_unique_ptr);
bool base_has_unique_ptr_holder
= (base_info->holder_enum_v == holder_enum_t::std_unique_ptr);
if (this_has_unique_ptr_holder != base_has_unique_ptr_holder) {
std::string tname(base.name());
detail::clean_type_id(tname);
pybind11_fail("generic_type: type \"" + std::string(name) + "\" "
+ (default_holder ? "does not have" : "has")
+ (this_has_unique_ptr_holder ? "does not have" : "has")
+ " a non-default holder type while its base \"" + tname + "\" "
+ (base_info->default_holder ? "does not" : "does"));
+ (base_has_unique_ptr_holder ? "does not" : "does"));
}

bases.append((PyObject *) base_info->type);
Expand Down
10 changes: 8 additions & 2 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,10 @@ struct copyable_holder_caster : public type_caster_base<type> {
protected:
friend class type_caster_generic;
void check_holder_compat() {
if (typeinfo->default_holder) {
// SMART_HOLDER_BAKEIN_FOLLOW_ON: Refine holder compatibility checks.
bool inst_has_unique_ptr_holder
= (typeinfo->holder_enum_v == holder_enum_t::std_unique_ptr);
if (inst_has_unique_ptr_holder) {
throw cast_error("Unable to load a custom holder type from a default-holder instance");
}
}
Expand Down Expand Up @@ -908,7 +911,10 @@ struct copyable_holder_caster<
protected:
friend class type_caster_generic;
void check_holder_compat() {
if (typeinfo->default_holder) {
// SMART_HOLDER_BAKEIN_FOLLOW_ON: Refine holder compatibility checks.
bool inst_has_unique_ptr_holder
= (typeinfo->holder_enum_v == holder_enum_t::std_unique_ptr);
if (inst_has_unique_ptr_holder) {
throw cast_error("Unable to load a custom holder type from a default-holder instance");
}
}
Expand Down
12 changes: 4 additions & 8 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
/// further ABI-incompatible changes may be made before the ABI is officially
/// changed to the new version.
#ifndef PYBIND11_INTERNALS_VERSION
# define PYBIND11_INTERNALS_VERSION 106
# define PYBIND11_INTERNALS_VERSION 7
#endif

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

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
Expand Down Expand Up @@ -234,20 +234,16 @@ struct type_info {
buffer_info *(*get_buffer)(PyObject *, void *) = nullptr;
void *get_buffer_data = nullptr;
void *(*module_local_load)(PyObject *, const type_info *) = nullptr;
holder_enum_t holder_enum_v = holder_enum_t::undefined;
/* A simple type never occurs as a (direct or indirect) parent
* of a class that makes use of multiple inheritance.
* A type can be simple even if it has non-simple ancestors as long as it has no descendants.
*/
bool simple_type : 1;
/* 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;
holder_enum_t holder_enum_v = holder_enum_t::undefined;
};

#define PYBIND11_INTERNALS_ID \
Expand Down
4 changes: 0 additions & 4 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,6 @@ class generic_type : public object {
tinfo->dealloc = rec.dealloc;
tinfo->simple_type = true;
tinfo->simple_ancestors = true;
tinfo->default_holder = rec.default_holder;
tinfo->module_local = rec.module_local;
tinfo->holder_enum_v = rec.holder_enum_v;

Expand Down Expand Up @@ -1903,9 +1902,6 @@ class class_ : public detail::generic_type {
record.holder_size = sizeof(holder_type);
record.init_instance = init_instance;

// A more fitting name would be uses_unique_ptr_holder.
record.default_holder = detail::is_instantiation<std::unique_ptr, holder_type>::value;

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

0 comments on commit aed215c

Please sign in to comment.