Skip to content

Commit

Permalink
Split out non-functional changes from pybind/pybind11#5332 (#5333)
Browse files Browse the repository at this point in the history
PREPARATION for:

PR #5332 — Fix handling of const unique_ptr<T, D> & (do not disown).

Splitting out so that the functional changes under PR #5332 will be more obvious.
  • Loading branch information
Ralf W. Grosse-Kunstleve authored Aug 25, 2024
1 parent d58cd0d commit bf54ecd
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 11 deletions.
16 changes: 8 additions & 8 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -871,14 +871,14 @@ struct copyable_holder_caster<
pybind11_fail("Passing `std::shared_ptr<T> *` from Python to C++ is not supported "
"(inherently unsafe).");
}
return std::addressof(shared_ptr_holder);
return std::addressof(shared_ptr_storage);
}

explicit operator std::shared_ptr<type> &() {
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
shared_ptr_holder = sh_load_helper.load_as_shared_ptr(value);
shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value);
}
return shared_ptr_holder;
return shared_ptr_storage;
}

static handle
Expand Down Expand Up @@ -924,7 +924,7 @@ struct copyable_holder_caster<
}
if (v_h.holder_constructed()) {
value = v_h.value_ptr();
shared_ptr_holder = v_h.template holder<std::shared_ptr<type>>();
shared_ptr_storage = v_h.template holder<std::shared_ptr<type>>();
return;
}
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
Expand Down Expand Up @@ -953,8 +953,8 @@ struct copyable_holder_caster<
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
sh_load_helper.loaded_v_h = sub_caster.sh_load_helper.loaded_v_h;
} else {
shared_ptr_holder
= std::shared_ptr<type>(sub_caster.shared_ptr_holder, (type *) value);
shared_ptr_storage
= std::shared_ptr<type>(sub_caster.shared_ptr_storage, (type *) value);
}
return true;
}
Expand All @@ -964,8 +964,8 @@ struct copyable_holder_caster<

static bool try_direct_conversions(handle) { return false; }

std::shared_ptr<type> shared_ptr_holder;
smart_holder_type_caster_support::load_helper<remove_cv_t<type>> sh_load_helper; // Const2Mutbl
std::shared_ptr<type> shared_ptr_storage;
};

#endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
Expand Down Expand Up @@ -1040,7 +1040,7 @@ struct move_only_holder_caster<
policy = return_value_policy::reference_internal;
}
if (policy != return_value_policy::reference_internal) {
throw cast_error("Invalid return_value_policy for unique_ptr&");
throw cast_error("Invalid return_value_policy for const unique_ptr&");
}
return type_caster_base<type>::cast(src.get(), policy, parent);
}
Expand Down
6 changes: 4 additions & 2 deletions tests/test_class_sh_trampoline_shared_from_this.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ long pass_shared_ptr(const std::shared_ptr<Sft> &obj) {
return sft.use_count();
}

void pass_unique_ptr(const std::unique_ptr<Sft> &) {}
void pass_unique_ptr_cref(const std::unique_ptr<Sft> &) {
throw std::runtime_error("Expected to not be reached.");
}

Sft *make_pure_cpp_sft_raw_ptr(const std::string &history_seed) { return new Sft{history_seed}; }

Expand Down Expand Up @@ -135,7 +137,7 @@ TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {

m.def("use_count", use_count);
m.def("pass_shared_ptr", pass_shared_ptr);
m.def("pass_unique_ptr", pass_unique_ptr);
m.def("pass_unique_ptr_cref", pass_unique_ptr_cref);
m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr);
m.def("make_pure_cpp_sft_unq_ptr", make_pure_cpp_sft_unq_ptr);
m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr);
Expand Down
3 changes: 2 additions & 1 deletion tests/test_class_sh_trampoline_shared_from_this.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,11 @@ def test_pass_released_shared_ptr_as_unique_ptr():
stash1 = m.SftSharedPtrStash(1)
stash1.Add(obj) # Releases shared_ptr to C++.
with pytest.raises(ValueError) as exc_info:
m.pass_unique_ptr(obj)
m.pass_unique_ptr_cref(obj)
assert str(exc_info.value) == (
"Python instance is currently owned by a std::shared_ptr."
)
assert obj.history == "PySft_Stash1Add"


@pytest.mark.parametrize(
Expand Down

0 comments on commit bf54ecd

Please sign in to comment.