Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,18 @@ struct copyable_holder_caster<
return smart_holder_type_caster_support::smart_holder_from_shared_ptr(
src, policy, parent, srcs.result);
}
return type_caster_base<type>::cast_holder(srcs, &src);

auto *tinfo = srcs.result.tinfo;
if (tinfo != nullptr && tinfo->holder_enum_v == holder_enum_t::std_shared_ptr) {
return type_caster_base<type>::cast_holder(srcs, &src);
}

if (parent) {
return type_caster_base<type>::cast(srcs, policy, parent);
}

throw cast_error("Unable to convert std::shared_ptr<T> to Python when the bound type "
"does not use std::shared_ptr or py::smart_holder as its holder type");
}

// This function will succeed even if the `responsible_parent` does not own the
Expand Down
35 changes: 35 additions & 0 deletions tests/test_class_sh_property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@ struct WithConstCharPtrMember {
const char *const_char_ptr_member = "ConstChar*";
};

// See PR #6008
enum class EnumAB {
A = 0,
B = 1,
};

struct ShWithEnumABMember {
EnumAB level = EnumAB::A;
};

struct SimpleStruct {
int value = 7;
};

struct ShWithSimpleStructMember {
SimpleStruct legacy;
};

} // namespace test_class_sh_property

TEST_SUBMODULE(class_sh_property, m) {
Expand Down Expand Up @@ -91,4 +109,21 @@ TEST_SUBMODULE(class_sh_property, m) {
py::classh<WithConstCharPtrMember>(m, "WithConstCharPtrMember")
.def(py::init<>())
.def_readonly("const_char_ptr_member", &WithConstCharPtrMember::const_char_ptr_member);

// See PR #6008
py::enum_<EnumAB>(m, "EnumAB").value("A", EnumAB::A).value("B", EnumAB::B);

py::classh<ShWithEnumABMember>(m, "ShWithEnumABMember")
.def(py::init<>())
.def_readwrite("level", &ShWithEnumABMember::level);

py::class_<SimpleStruct>(m, "SimpleStruct")
.def(py::init<>())
.def_readwrite("value", &SimpleStruct::value);

py::classh<ShWithSimpleStructMember>(m, "ShWithSimpleStructMember")
.def(py::init<>())
.def_readwrite("legacy", &ShWithSimpleStructMember::legacy);

m.def("getSimpleStructAsShared", []() { return std::make_shared<SimpleStruct>(); });
}
28 changes: 28 additions & 0 deletions tests/test_class_sh_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,31 @@
def test_readonly_const_char_ptr_member():
obj = m.WithConstCharPtrMember()
assert obj.const_char_ptr_member == "ConstChar*"


# See PR #6008
def test_enum_member_with_smart_holder_def_readwrite():
obj = m.ShWithEnumABMember()
assert obj.level == m.EnumAB.A
for _ in range(100):
v = obj.level
assert v == m.EnumAB.A
del v


# See PR #6008
def test_non_smart_holder_member_type_with_smart_holder_owner():
obj = m.ShWithSimpleStructMember()
for _ in range(1000):
v = obj.legacy
assert v.value == 7
del v


# See PR #6008, previously this was UB
def test_shared_ptr_return_for_unique_ptr_holder():
with pytest.raises(

Check failure on line 190 in tests/test_class_sh_property.py

View workflow job for this annotation

GitHub Actions / 🐍 (ubuntu-latest, 3.14t, -DCMAKE_CXX_STANDARD=17 -DPYBIND11_TEST_SMART_HOLDER=ON) / 🧪

test_shared_ptr_return_for_unique_ptr_holder Failed: DID NOT RAISE <class 'RuntimeError'>

Check failure on line 190 in tests/test_class_sh_property.py

View workflow job for this annotation

GitHub Actions / 🐍 (ubuntu-latest, 3.12, -DPYBIND11_TEST_SMART_HOLDER=ON -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON) / 🧪

test_shared_ptr_return_for_unique_ptr_holder Failed: DID NOT RAISE <class 'RuntimeError'>

Check failure on line 190 in tests/test_class_sh_property.py

View workflow job for this annotation

GitHub Actions / 🐍 (ubuntu-latest, 3.11, -DPYBIND11_TEST_SMART_HOLDER=ON -DCMAKE_CXX_STANDARD=17) / 🧪

test_shared_ptr_return_for_unique_ptr_holder Failed: DID NOT RAISE <class 'RuntimeError'>
RuntimeError,
match="Unable to convert std::shared_ptr<T> to Python when the bound type does not use std::shared_ptr or py::smart_holder as its holder type",
):
m.getSimpleStructAsShared()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird that this check sometimes doesn't occur on some compilers? Maybe we just omit this test.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first sight, it seems like -DPYBIND11_TEST_SMART_HOLDER=ON could be the culprit?

But hang on a sec, I'll post another comment. Maybe that one will make this moot (not sure).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JIC you find it's not moot, and JIC it helps: PYBIND11_TEST_SMART_HOLDER (cmake) triggers PYBIND11_RUN_TESTING_WITH_SMART_HOLDER_AS_DEFAULT_BUT_NEVER_USE_IN_PRODUCTION_PLEASE (C++)

Loading