Skip to content

Commit 511e8c3

Browse files
committed
Fix roundtrip unit test for derived Python objects
As expected, the unit test was failing because args were passed by-value and not by reference. However, it wasn't possible to just change the policy to reference or reference_internal. reference gave errors, because const unique_ptr& only accepts reference_internal. reference_internal gives an error, because the parent required for keep_alive is null. Hence, I decided to introduce a new policy to explicitly indicate passing from from Python to C++ via an overridden method. That's the cleanest solution to correctly react to the special needs of this use case.
1 parent d0b8c3f commit 511e8c3

File tree

4 files changed

+21
-11
lines changed

4 files changed

+21
-11
lines changed

include/pybind11/detail/common.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,11 @@ enum class return_value_policy : uint8_t {
434434
collected while Python is still using the child. More advanced
435435
variations of this scheme are also possible using combinations of
436436
return_value_policy::reference and the keep_alive call policy */
437-
reference_internal
437+
reference_internal,
438+
439+
/* This internally-only used policy applies to C++ arguments passed
440+
to virtual methods overridden in Python to allow reference passing. */
441+
automatic_override
438442
};
439443

440444
PYBIND11_NAMESPACE_BEGIN(detail)

include/pybind11/detail/smart_holder_type_casters.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,10 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
472472
}
473473

474474
static handle cast(T const &src, return_value_policy policy, handle parent) {
475+
return cast(const_cast<T &>(src), policy, parent);
476+
}
477+
478+
static handle cast(T &src, return_value_policy policy, handle parent) {
475479
// type_caster_base BEGIN
476480
// clang-format off
477481
if (policy == return_value_policy::automatic || policy == return_value_policy::automatic_reference)
@@ -481,11 +485,13 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
481485
// type_caster_base END
482486
}
483487

484-
static handle cast(T &src, return_value_policy policy, handle parent) {
485-
return cast(const_cast<T const &>(src), policy, parent); // Mutbl2Const
488+
static handle cast(T const *src, return_value_policy policy, handle parent) {
489+
return cast(const_cast<T *>(src), policy, parent);
486490
}
487491

488-
static handle cast(T const *src, return_value_policy policy, handle parent) {
492+
static handle cast(T *src, return_value_policy policy, handle parent) {
493+
if (policy == return_value_policy::automatic_override)
494+
policy = return_value_policy::reference;
489495
auto st = type_caster_base<T>::src_and_type(src);
490496
return cast_const_raw_ptr( // Originally type_caster_generic::cast.
491497
st.first,
@@ -496,10 +502,6 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
496502
make_constructor::make_move_constructor(src));
497503
}
498504

499-
static handle cast(T *src, return_value_policy policy, handle parent) {
500-
return cast(const_cast<T const *>(src), policy, parent); // Mutbl2Const
501-
}
502-
503505
#if defined(_MSC_VER) && _MSC_VER < 1910
504506
// Working around MSVC 2015 bug. const-correctness is lost.
505507
// SMART_HOLDER_WIP: IMPROVABLE: make common code work with MSVC 2015.
@@ -723,7 +725,9 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
723725
return none().release();
724726
if (policy == return_value_policy::automatic)
725727
policy = return_value_policy::reference_internal;
726-
if (policy != return_value_policy::reference_internal)
728+
else if (policy == return_value_policy::automatic_override)
729+
;
730+
else if (policy != return_value_policy::reference_internal)
727731
throw cast_error("Invalid return_value_policy for unique_ptr&");
728732
return smart_holder_type_caster<T>::cast(src.get(), policy, parent);
729733
}

include/pybind11/detail/type_caster_base.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,8 @@ template <typename type> class type_caster_base : public type_caster_generic {
899899
}
900900

901901
static handle cast(const itype *src, return_value_policy policy, handle parent) {
902+
if (policy == return_value_policy::automatic_override)
903+
policy = return_value_policy::reference;
902904
auto st = src_and_type(src);
903905
return type_caster_generic::cast(
904906
st.first, policy, parent, st.second,

include/pybind11/pytypes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ class object_api : public pyobject_tag {
105105
function will throw a `cast_error` exception. When the Python function
106106
call fails, a `error_already_set` exception is thrown.
107107
\endrst */
108-
template <return_value_policy policy = return_value_policy::automatic_reference, typename... Args>
108+
template <return_value_policy policy = return_value_policy::automatic_override, typename... Args>
109109
object operator()(Args &&...args) const;
110-
template <return_value_policy policy = return_value_policy::automatic_reference, typename... Args>
110+
template <return_value_policy policy = return_value_policy::automatic_override, typename... Args>
111111
PYBIND11_DEPRECATED("call(...) was deprecated in favor of operator()(...)")
112112
object call(Args&&... args) const;
113113

0 commit comments

Comments
 (0)