Skip to content

Commit

Permalink
[Smart holder] Support void pointer capsules (google#3633)
Browse files Browse the repository at this point in the history
* Make smart holder type casters support void pointer capsules.

* Fix warnings

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix checks

* Fix check failures under CentOS

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove unused regex module

* Resolve comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Resolve comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix clangtidy

* Resolve comments

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
wangxf123456 and pre-commit-ci[bot] authored Jan 25, 2022
1 parent 152bb10 commit da058a2
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 1 deletion.
48 changes: 47 additions & 1 deletion include/pybind11/detail/smart_holder_type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ struct is_smart_holder_type<smart_holder> : std::true_type {};
inline void register_instance(instance *self, void *valptr, const type_info *tinfo);
inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);

// Replace all occurrences of a character in string.
inline void replace_all(std::string& str, const std::string& from, char to) {
size_t pos = str.find(from);
while (pos != std::string::npos) {
str.replace(pos, from.length(), 1, to);
pos = str.find(from, pos);
}
}

// The modified_type_caster_generic_load_impl could replace type_caster_generic::load_impl but not
// vice versa. The main difference is that the original code only propagates a reference to the
// held value, while the modified implementation propagates value_and_holder.
Expand Down Expand Up @@ -106,6 +115,28 @@ class modified_type_caster_generic_load_impl {
return false;
}

bool try_as_void_ptr_capsule(handle src) {
std::string type_name = cpptype->name();
detail::clean_type_id(type_name);

// Convert `a::b::c` to `a_b_c`
replace_all(type_name, "::", '_');

std::string as_void_ptr_function_name("as_");
as_void_ptr_function_name += type_name;
if (hasattr(src, as_void_ptr_function_name.c_str())) {
auto as_void_ptr_function = function(
src.attr(as_void_ptr_function_name.c_str()));
auto void_ptr_capsule = as_void_ptr_function();
if (isinstance<capsule>(void_ptr_capsule)) {
unowned_void_ptr_from_void_ptr_capsule = reinterpret_borrow<capsule>(
void_ptr_capsule).get_pointer();
return true;
}
}
return false;
}

PYBIND11_NOINLINE static void *local_load(PyObject *src, const type_info *ti) {
std::unique_ptr<modified_type_caster_generic_load_impl> loader(
new modified_type_caster_generic_load_impl(ti));
Expand Down Expand Up @@ -161,6 +192,9 @@ class modified_type_caster_generic_load_impl {
template <typename ThisT>
PYBIND11_NOINLINE bool load_impl(handle src, bool convert) {
if (!src) return false;
if (cpptype && try_as_void_ptr_capsule(src)) {
return true;
}
if (!typeinfo) return try_load_foreign_module_local(src);

auto &this_ = static_cast<ThisT &>(*this);
Expand Down Expand Up @@ -250,6 +284,7 @@ class modified_type_caster_generic_load_impl {
const type_info *typeinfo = nullptr;
const std::type_info *cpptype = nullptr;
void *unowned_void_ptr_from_direct_conversion = nullptr;
void *unowned_void_ptr_from_void_ptr_capsule = nullptr;
const std::type_info *loaded_v_h_cpptype = nullptr;
void *(*implicit_cast)(void *) = nullptr;
value_and_holder loaded_v_h;
Expand Down Expand Up @@ -366,7 +401,10 @@ struct smart_holder_type_caster_load {
}

T *loaded_as_raw_ptr_unowned() const {
void *void_ptr = load_impl.unowned_void_ptr_from_direct_conversion;
void *void_ptr = load_impl.unowned_void_ptr_from_void_ptr_capsule;
if (void_ptr == nullptr) {
void_ptr = load_impl.unowned_void_ptr_from_direct_conversion;
}
if (void_ptr == nullptr) {
if (have_holder()) {
throw_if_uninitialized_or_disowned_holder();
Expand All @@ -387,6 +425,10 @@ struct smart_holder_type_caster_load {
}

std::shared_ptr<T> loaded_as_shared_ptr() const {
if (load_impl.unowned_void_ptr_from_void_ptr_capsule) {
throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a"
" std::shared_ptr.");
}
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr)
throw cast_error("Unowned pointer from direct conversion cannot be converted to a"
" std::shared_ptr.");
Expand Down Expand Up @@ -441,6 +483,10 @@ struct smart_holder_type_caster_load {

template <typename D>
std::unique_ptr<T, D> loaded_as_unique_ptr(const char *context = "loaded_as_unique_ptr") {
if (load_impl.unowned_void_ptr_from_void_ptr_capsule) {
throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a"
" std::unique_ptr.");
}
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr)
throw cast_error("Unowned pointer from direct conversion cannot be converted to a"
" std::unique_ptr.");
Expand Down
146 changes: 146 additions & 0 deletions tests/test_class_sh_void_ptr_capsule.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
#include "pybind11_tests.h"

#include <pybind11/smart_holder.h>

#include <memory>

namespace pybind11_tests {
namespace class_sh_void_ptr_capsule {

// Without the helper we will run into a type_caster::load recursion.
// This is because whenever the type_caster::load is called, it checks
// whether the object defines an `as_` method that returns the void pointer
// capsule. If yes, it calls the method. But in the following testcases, those
// `as_` methods are defined with pybind11, which implicitly takes the object
// itself as the first parameter. Therefore calling those methods causes loading
// the object again, which causes infinite recursion.
// This test is unusual in the sense that the void pointer capsules are meant to
// be provided by objects wrapped with systems other than pybind11
// (i.e. having to avoid the recursion is an artificial problem, not the norm).
// Conveniently, the helper also serves to keep track of `capsule_generated`.
struct HelperBase {
HelperBase() = default;
virtual ~HelperBase() = default;

bool capsule_generated = false;
virtual int get() const { return 100; }
};

struct Valid: public HelperBase {
int get() const override { return 101; }

PyObject* as_pybind11_tests_class_sh_void_ptr_capsule_Valid() {
void* vptr = dynamic_cast<void*>(this);
capsule_generated = true;
// We assume vptr out lives the capsule, so we use nullptr for the
// destructor.
return PyCapsule_New(
vptr, "::pybind11_tests::class_sh_void_ptr_capsule::Valid",
nullptr);
}
};

struct NoConversion: public HelperBase {
int get() const override { return 102; }
};

struct NoCapsuleReturned: public HelperBase {
int get() const { return 103; }

PyObject* as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned() {
capsule_generated = true;
Py_XINCREF(Py_None);
return Py_None;
}
};

struct AsAnotherObject: public HelperBase {
int get() const override { return 104; }

PyObject* as_pybind11_tests_class_sh_void_ptr_capsule_Valid() {
void* vptr = dynamic_cast<void*>(this);
capsule_generated = true;
// We assume vptr out lives the capsule, so we use nullptr for the
// destructor.
return PyCapsule_New(
vptr, "::pybind11_tests::class_sh_void_ptr_capsule::Valid",
nullptr);
}
};

int get_from_valid_capsule(const Valid* c) {
return c->get();
}

int get_from_shared_ptr_valid_capsule(std::shared_ptr<Valid> c) {
return c->get();
}

int get_from_unique_ptr_valid_capsule(std::unique_ptr<Valid> c) {
return c->get();
}

int get_from_no_conversion_capsule(const NoConversion* c) {
return c->get();
}

int get_from_no_capsule_returned(const NoCapsuleReturned* c) {
return c->get();
}

} // namespace class_sh_void_ptr_capsule
} // namespace pybind11_tests

PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::HelperBase)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Valid)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::NoConversion)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::NoCapsuleReturned)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::AsAnotherObject)

TEST_SUBMODULE(class_sh_void_ptr_capsule, m) {
using namespace pybind11_tests::class_sh_void_ptr_capsule;

py::classh<HelperBase>(m, "HelperBase")
.def(py::init<>())
.def("get", &HelperBase::get)
.def_readonly("capsule_generated", &HelperBase::capsule_generated);

py::classh<Valid, HelperBase>(m, "Valid")
.def(py::init<>())
.def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid",
[](HelperBase* self) {
Valid *obj = dynamic_cast<Valid *>(self);
assert(obj != nullptr);
PyObject* capsule = obj->as_pybind11_tests_class_sh_void_ptr_capsule_Valid();
return pybind11::reinterpret_steal<py::capsule>(capsule);
});

py::classh<NoConversion, HelperBase>(m, "NoConversion")
.def(py::init<>());

py::classh<NoCapsuleReturned, HelperBase>(m, "NoCapsuleReturned")
.def(py::init<>())
.def("as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned",
[](HelperBase* self) {
NoCapsuleReturned *obj = dynamic_cast<NoCapsuleReturned *>(self);
assert(obj != nullptr);
PyObject* capsule = obj->as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned();
return pybind11::reinterpret_steal<py::capsule>(capsule);
});

py::classh<AsAnotherObject, HelperBase>(m, "AsAnotherObject")
.def(py::init<>())
.def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid",
[](HelperBase* self) {
AsAnotherObject *obj = dynamic_cast<AsAnotherObject *>(self);
assert(obj != nullptr);
PyObject* capsule = obj->as_pybind11_tests_class_sh_void_ptr_capsule_Valid();
return pybind11::reinterpret_steal<py::capsule>(capsule);
});

m.def("get_from_valid_capsule", &get_from_valid_capsule);
m.def("get_from_shared_ptr_valid_capsule", &get_from_shared_ptr_valid_capsule);
m.def("get_from_unique_ptr_valid_capsule", &get_from_unique_ptr_valid_capsule);
m.def("get_from_no_conversion_capsule", &get_from_no_conversion_capsule);
m.def("get_from_no_capsule_returned", &get_from_no_capsule_returned);
}
34 changes: 34 additions & 0 deletions tests/test_class_sh_void_ptr_capsule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# -*- coding: utf-8 -*-
import pytest

from pybind11_tests import class_sh_void_ptr_capsule as m


@pytest.mark.parametrize(
"ctor, caller, expected, capsule_generated",
[
(m.Valid, m.get_from_valid_capsule, 101, True),
(m.NoConversion, m.get_from_no_conversion_capsule, 102, False),
(m.NoCapsuleReturned, m.get_from_no_capsule_returned, 103, True),
(m.AsAnotherObject, m.get_from_valid_capsule, 104, True),
],
)
def test_as_void_ptr_capsule(ctor, caller, expected, capsule_generated):
obj = ctor()
assert caller(obj) == expected
assert obj.capsule_generated == capsule_generated


@pytest.mark.parametrize(
"ctor, caller, pointer_type, capsule_generated",
[
(m.AsAnotherObject, m.get_from_shared_ptr_valid_capsule, "shared_ptr", True),
(m.AsAnotherObject, m.get_from_unique_ptr_valid_capsule, "unique_ptr", True),
],
)
def test_as_void_ptr_capsule_unsupported(ctor, caller, pointer_type, capsule_generated):
obj = ctor()
with pytest.raises(RuntimeError) as excinfo:
caller(obj)
assert pointer_type in str(excinfo.value)
assert obj.capsule_generated == capsule_generated

0 comments on commit da058a2

Please sign in to comment.