Skip to content

Commit

Permalink
only check ref_count for temporary values
Browse files Browse the repository at this point in the history
  • Loading branch information
EricCousineau-TRI committed Oct 17, 2023
1 parent e61bb56 commit 265b71b
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 7 deletions.
14 changes: 8 additions & 6 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1219,17 +1219,19 @@ move(T &&value) {
}

template <typename T>
detail::enable_if_t<!detail::move_never<T>::value, T> move(object &&obj) {
if (obj.ref_count() > 1) {
detail::enable_if_t<!detail::move_never<T>::value, T> move(object &&obj) {\
if constexpr (detail::cast_is_temporary_value_reference<T>::value) {
if (obj.ref_count() > 1) {
#if !defined(PYBIND11_DETAILED_ERROR_MESSAGES)
throw cast_error(
throw cast_error(
"Unable to cast Python instance to C++ rvalue: instance has multiple references"
" (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)");
" (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)");
#else
throw cast_error("Unable to move from Python " + (std::string) str(type::handle_of(obj))
+ " instance to C++ " + type_id<T>()
throw cast_error("Unable to move from Python " + (std::string) str(type::handle_of(obj))
+ " instance to C++ " + type_id<T>()
+ " instance: instance has multiple references");
#endif
}
}

// Move into a temporary and return that, because the reference may be a local value of `conv`
Expand Down
11 changes: 11 additions & 0 deletions tests/test_smart_ptr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "object.h"
#include "pybind11_tests.h"
#include "pybind11/functional.h"

namespace {

Expand Down Expand Up @@ -531,6 +532,16 @@ TEST_SUBMODULE(smart_ptr, m) {
return out;
});

using Callback = std::function<std::unique_ptr<UniquePtrHeld>()>;
m.def("unique_ptr_callback",
[](const Callback& callback) {
auto obj = callback();
if (obj->value() != 10) {
throw std::runtime_error("obj.value() must be 10");
}
return callback();
});

// Ensure class is non-empty, so it's easier to detect double-free
// corruption. (If empty, this may be harder to see easily.)
struct SharedPtrHeld {
Expand Down
13 changes: 13 additions & 0 deletions tests/test_smart_ptr.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,3 +378,16 @@ def check_reset(obj_new):

check_reset(obj_new=m.UniquePtrHeld(10))
check_reset(obj_new=None)


def test_unique_ptr_self_reference():
obj = None

def make_unique_ptr_held():
nonlocal obj
# This simulates a self-reference for a derived type.
obj = m.UniquePtrHeld(10)
return obj

out = m.unique_ptr_callback(make_unique_ptr_held)
assert obj is out
5 changes: 4 additions & 1 deletion tests/test_virtual_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@ def get_movable(self, a, b):
ncv2 = NCVirtExt2()
assert ncv2.print_movable(7, 7) == "14"
# Don't check the exception message here because it differs under debug/non-debug mode
with pytest.raises(RuntimeError):

# N.B. Drake fork allows this behavior.
if True:
# with pytest.raises(RuntimeError):
ncv2.print_nc(9, 9)

nc_stats = ConstructorStats.get(m.NonCopyable)
Expand Down

0 comments on commit 265b71b

Please sign in to comment.