Skip to content

[smart_holder] Auto select return value policy for clif_automatic #4381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Dec 5, 2022
Merged
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
19 changes: 17 additions & 2 deletions include/pybind11/detail/smart_holder_type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -630,19 +630,30 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
static handle cast(T const &src, return_value_policy policy, handle parent) {
// type_caster_base BEGIN
if (policy == return_value_policy::automatic
|| policy == return_value_policy::automatic_reference) {
|| policy == return_value_policy::automatic_reference
|| policy == return_value_policy::_clif_automatic) {
policy = return_value_policy::copy;
}
return cast(&src, policy, parent);
// type_caster_base END
}

static handle cast(T &src, return_value_policy policy, handle parent) {
if (policy == return_value_policy::_clif_automatic) {
if (std::is_move_constructible<T>::value) {
policy = return_value_policy::move;
} else {
policy = return_value_policy::automatic;
}
}
return cast(const_cast<T const &>(src), policy, parent); // Mutbl2Const
}

static handle cast(T const *src, return_value_policy policy, handle parent) {
auto st = type_caster_base<T>::src_and_type(src);
if (policy == return_value_policy::_clif_automatic) {
policy = return_value_policy::copy;
}
return cast_const_raw_ptr( // Originally type_caster_generic::cast.
st.first,
policy,
Expand All @@ -653,6 +664,9 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
}

static handle cast(T *src, return_value_policy policy, handle parent) {
if (policy == return_value_policy::_clif_automatic) {
policy = return_value_policy::reference;
}
return cast(const_cast<T const *>(src), policy, parent); // Mutbl2Const
}

Expand Down Expand Up @@ -867,7 +881,8 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
static handle cast(std::unique_ptr<T, D> &&src, return_value_policy policy, handle parent) {
if (policy != return_value_policy::automatic
&& policy != return_value_policy::reference_internal
&& policy != return_value_policy::move) {
&& policy != return_value_policy::move
&& policy != return_value_policy::_clif_automatic) {
// SMART_HOLDER_WIP: IMPROVABLE: Error message.
throw cast_error("Invalid return_value_policy for unique_ptr.");
}
Expand Down
241 changes: 241 additions & 0 deletions tests/test_return_value_policy_override.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,166 @@
#include <pybind11/smart_holder.h>

#include "pybind11_tests.h"

namespace test_return_value_policy_override {

struct some_type {};

// cp = copyable, mv = movable, 1 = yes, 0 = no
struct type_cp1_mv1 {
std::string mtxt;
explicit type_cp1_mv1(const std::string &mtxt_) : mtxt(mtxt_) {}
type_cp1_mv1(const type_cp1_mv1 &other) { mtxt = other.mtxt + "_CpCtor"; }
type_cp1_mv1(type_cp1_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; }
type_cp1_mv1 &operator=(const type_cp1_mv1 &other) {
mtxt = other.mtxt + "_CpCtor";
return *this;
}
type_cp1_mv1 &operator=(type_cp1_mv1 &&other) noexcept {
mtxt = other.mtxt + "_MvCtor";
return *this;
}
};

// nocopy
struct type_cp0_mv1 {
std::string mtxt;
explicit type_cp0_mv1(const std::string &mtxt_) : mtxt(mtxt_) {}
type_cp0_mv1(const type_cp0_mv1 &) = delete;
type_cp0_mv1(type_cp0_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; }
type_cp0_mv1 &operator=(const type_cp0_mv1 &) = delete;
type_cp0_mv1 &operator=(type_cp0_mv1 &&other) noexcept {
mtxt = other.mtxt + "_MvCtor";
return *this;
}
};

// nomove
struct type_cp1_mv0 {
std::string mtxt;
explicit type_cp1_mv0(const std::string &mtxt_) : mtxt(mtxt_) {}
type_cp1_mv0(const type_cp1_mv0 &other) { mtxt = other.mtxt + "_CpCtor"; }
type_cp1_mv0(type_cp1_mv0 &&other) = delete;
type_cp1_mv0 &operator=(const type_cp1_mv0 &other) {
mtxt = other.mtxt + "_CpCtor";
return *this;
}
type_cp1_mv0 &operator=(type_cp1_mv0 &&other) = delete;
};

// nocopy_nomove
struct type_cp0_mv0 {
std::string mtxt;
explicit type_cp0_mv0(const std::string &mtxt_) : mtxt(mtxt_) {}
type_cp0_mv0(const type_cp0_mv0 &) = delete;
type_cp0_mv0(type_cp0_mv0 &&other) = delete;
type_cp0_mv0 &operator=(const type_cp0_mv0 &other) = delete;
type_cp0_mv0 &operator=(type_cp0_mv0 &&other) = delete;
};

type_cp1_mv1 return_value() { return type_cp1_mv1{"value"}; }

type_cp1_mv1 *return_pointer() {
static type_cp1_mv1 value("pointer");
return &value;
}

const type_cp1_mv1 *return_const_pointer() {
static type_cp1_mv1 value("const_pointer");
return &value;
}

type_cp1_mv1 &return_reference() {
static type_cp1_mv1 value("reference");
return value;
}

const type_cp1_mv1 &return_const_reference() {
static type_cp1_mv1 value("const_reference");
return value;
}

std::shared_ptr<type_cp1_mv1> return_shared_pointer() {
return std::make_shared<type_cp1_mv1>("shared_pointer");
}

std::unique_ptr<type_cp1_mv1> return_unique_pointer() {
return std::unique_ptr<type_cp1_mv1>(new type_cp1_mv1("unique_pointer"));
}

type_cp0_mv1 return_value_nocopy() { return type_cp0_mv1{"value_nocopy"}; }

type_cp0_mv1 *return_pointer_nocopy() {
static type_cp0_mv1 value("pointer_nocopy");
return &value;
}

const type_cp0_mv1 *return_const_pointer_nocopy() {
static type_cp0_mv1 value("const_pointer_nocopy");
return &value;
}

type_cp0_mv1 &return_reference_nocopy() {
static type_cp0_mv1 value("reference_nocopy");
return value;
}

std::shared_ptr<type_cp0_mv1> return_shared_pointer_nocopy() {
return std::make_shared<type_cp0_mv1>("shared_pointer_nocopy");
}

std::unique_ptr<type_cp0_mv1> return_unique_pointer_nocopy() {
return std::unique_ptr<type_cp0_mv1>(new type_cp0_mv1("unique_pointer_nocopy"));
}

type_cp1_mv0 *return_pointer_nomove() {
static type_cp1_mv0 value("pointer_nomove");
return &value;
}

const type_cp1_mv0 *return_const_pointer_nomove() {
static type_cp1_mv0 value("const_pointer_nomove");
return &value;
}

type_cp1_mv0 &return_reference_nomove() {
static type_cp1_mv0 value("reference_nomove");
return value;
}

const type_cp1_mv0 &return_const_reference_nomove() {
static type_cp1_mv0 value("const_reference_nomove");
return value;
}

std::shared_ptr<type_cp1_mv0> return_shared_pointer_nomove() {
return std::make_shared<type_cp1_mv0>("shared_pointer_nomove");
}

std::unique_ptr<type_cp1_mv0> return_unique_pointer_nomove() {
return std::unique_ptr<type_cp1_mv0>(new type_cp1_mv0("unique_pointer_nomove"));
}

type_cp0_mv0 *return_pointer_nocopy_nomove() {
static type_cp0_mv0 value("pointer_nocopy_nomove");
return &value;
}

std::shared_ptr<type_cp0_mv0> return_shared_pointer_nocopy_nomove() {
return std::make_shared<type_cp0_mv0>("shared_pointer_nocopy_nomove");
}

std::unique_ptr<type_cp0_mv0> return_unique_pointer_nocopy_nomove() {
return std::unique_ptr<type_cp0_mv0>(new type_cp0_mv0("unique_pointer_nocopy_nomove"));
}

} // namespace test_return_value_policy_override

using test_return_value_policy_override::some_type;
using test_return_value_policy_override::type_cp0_mv0;
using test_return_value_policy_override::type_cp0_mv1;
using test_return_value_policy_override::type_cp1_mv0;
using test_return_value_policy_override::type_cp1_mv1;

namespace pybind11 {
namespace detail {
Expand Down Expand Up @@ -51,6 +205,11 @@ struct type_caster<some_type> : type_caster_base<some_type> {
} // namespace detail
} // namespace pybind11

PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv1)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv1)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv0)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv0)

TEST_SUBMODULE(return_value_policy_override, m) {
m.def("return_value_with_default_policy", []() { return some_type(); });
m.def(
Expand Down Expand Up @@ -79,4 +238,86 @@ TEST_SUBMODULE(return_value_policy_override, m) {
return &value;
},
py::return_value_policy::_clif_automatic);

py::classh<type_cp1_mv1>(m, "type_cp1_mv1")
.def(py::init<std::string>())
.def_readonly("mtxt", &type_cp1_mv1::mtxt);
m.def("return_value",
&test_return_value_policy_override::return_value,
py::return_value_policy::_clif_automatic);
m.def("return_pointer",
&test_return_value_policy_override::return_pointer,
py::return_value_policy::_clif_automatic);
m.def("return_const_pointer",
&test_return_value_policy_override::return_const_pointer,
py::return_value_policy::_clif_automatic);
m.def("return_reference",
&test_return_value_policy_override::return_reference,
py::return_value_policy::_clif_automatic);
m.def("return_const_reference",
&test_return_value_policy_override::return_const_reference,
py::return_value_policy::_clif_automatic);
m.def("return_unique_pointer",
&test_return_value_policy_override::return_unique_pointer,
py::return_value_policy::_clif_automatic);
m.def("return_shared_pointer",
&test_return_value_policy_override::return_shared_pointer,
py::return_value_policy::_clif_automatic);

py::classh<type_cp0_mv1>(m, "type_cp0_mv1")
.def(py::init<std::string>())
.def_readonly("mtxt", &type_cp0_mv1::mtxt);
m.def("return_value_nocopy",
&test_return_value_policy_override::return_value_nocopy,
py::return_value_policy::_clif_automatic);
m.def("return_pointer_nocopy",
&test_return_value_policy_override::return_pointer_nocopy,
py::return_value_policy::_clif_automatic);
m.def("return_const_pointer_nocopy",
&test_return_value_policy_override::return_const_pointer_nocopy,
py::return_value_policy::_clif_automatic);
m.def("return_reference_nocopy",
&test_return_value_policy_override::return_reference_nocopy,
py::return_value_policy::_clif_automatic);
m.def("return_shared_pointer_nocopy",
&test_return_value_policy_override::return_shared_pointer_nocopy,
py::return_value_policy::_clif_automatic);
m.def("return_unique_pointer_nocopy",
&test_return_value_policy_override::return_unique_pointer_nocopy,
py::return_value_policy::_clif_automatic);

py::classh<type_cp1_mv0>(m, "type_cp1_mv0")
.def(py::init<std::string>())
.def_readonly("mtxt", &type_cp1_mv0::mtxt);
m.def("return_pointer_nomove",
&test_return_value_policy_override::return_pointer_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_const_pointer_nomove",
&test_return_value_policy_override::return_const_pointer_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_reference_nomove",
&test_return_value_policy_override::return_reference_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_const_reference_nomove",
&test_return_value_policy_override::return_const_reference_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_shared_pointer_nomove",
&test_return_value_policy_override::return_shared_pointer_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_unique_pointer_nomove",
&test_return_value_policy_override::return_unique_pointer_nomove,
py::return_value_policy::_clif_automatic);

py::classh<type_cp0_mv0>(m, "type_cp0_mv0")
.def(py::init<std::string>())
.def_readonly("mtxt", &type_cp0_mv0::mtxt);
m.def("return_pointer_nocopy_nomove",
&test_return_value_policy_override::return_pointer_nocopy_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_shared_pointer_nocopy_nomove",
&test_return_value_policy_override::return_shared_pointer_nocopy_nomove,
py::return_value_policy::_clif_automatic);
m.def("return_unique_pointer_nocopy_nomove",
&test_return_value_policy_override::return_unique_pointer_nocopy_nomove,
py::return_value_policy::_clif_automatic);
}
34 changes: 34 additions & 0 deletions tests/test_return_value_policy_override.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import re

import pytest

from pybind11_tests import return_value_policy_override as m


Expand All @@ -11,3 +15,33 @@ def test_return_pointer():
assert m.return_pointer_with_default_policy() == "automatic"
assert m.return_pointer_with_policy_move() == "move"
assert m.return_pointer_with_policy_clif_automatic() == "_clif_automatic"


@pytest.mark.parametrize(
"func, expected",
[
(m.return_value, "value(_MvCtor)*_MvCtor"),
(m.return_pointer, "pointer"),
(m.return_const_pointer, "const_pointer_CpCtor"),
(m.return_reference, "reference_MvCtor"),
(m.return_const_reference, "const_reference_CpCtor"),
(m.return_unique_pointer, "unique_pointer"),
(m.return_shared_pointer, "shared_pointer"),
(m.return_value_nocopy, "value_nocopy(_MvCtor)*_MvCtor"),
(m.return_pointer_nocopy, "pointer_nocopy"),
(m.return_reference_nocopy, "reference_nocopy_MvCtor"),
(m.return_unique_pointer_nocopy, "unique_pointer_nocopy"),
(m.return_shared_pointer_nocopy, "shared_pointer_nocopy"),
(m.return_pointer_nomove, "pointer_nomove"),
(m.return_const_pointer_nomove, "const_pointer_nomove_CpCtor"),
(m.return_reference_nomove, "reference_nomove_CpCtor"),
(m.return_const_reference_nomove, "const_reference_nomove_CpCtor"),
(m.return_unique_pointer_nomove, "unique_pointer_nomove"),
(m.return_shared_pointer_nomove, "shared_pointer_nomove"),
(m.return_pointer_nocopy_nomove, "pointer_nocopy_nomove"),
(m.return_unique_pointer_nocopy_nomove, "unique_pointer_nocopy_nomove"),
(m.return_shared_pointer_nocopy_nomove, "shared_pointer_nocopy_nomove"),
],
)
def test_clif_automatic_return_value_policy_override(func, expected):
assert re.match(expected, func().mtxt)