Skip to content

Commit c1f14f0

Browse files
wangxf123456wangxf123456
authored
[smart_holder] Auto select return value policy for clif_automatic (#4381)
* Auto select return value policy for clif_automatic * Try fixing test failures * Add more tests. * remove comments * Fix test failures * Fix test failures * Fix test failure for windows platform * Fix clangtidy
1 parent 33fb7a6 commit c1f14f0

File tree

3 files changed

+292
-2
lines changed

3 files changed

+292
-2
lines changed

‎include/pybind11/detail/smart_holder_type_casters.h‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,19 +630,30 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
630630
static handle cast(T const &src, return_value_policy policy, handle parent) {
631631
// type_caster_base BEGIN
632632
if (policy == return_value_policy::automatic
633-
|| policy == return_value_policy::automatic_reference) {
633+
|| policy == return_value_policy::automatic_reference
634+
|| policy == return_value_policy::_clif_automatic) {
634635
policy = return_value_policy::copy;
635636
}
636637
return cast(&src, policy, parent);
637638
// type_caster_base END
638639
}
639640

640641
static handle cast(T &src, return_value_policy policy, handle parent) {
642+
if (policy == return_value_policy::_clif_automatic) {
643+
if (std::is_move_constructible<T>::value) {
644+
policy = return_value_policy::move;
645+
} else {
646+
policy = return_value_policy::automatic;
647+
}
648+
}
641649
return cast(const_cast<T const &>(src), policy, parent); // Mutbl2Const
642650
}
643651

644652
static handle cast(T const *src, return_value_policy policy, handle parent) {
645653
auto st = type_caster_base<T>::src_and_type(src);
654+
if (policy == return_value_policy::_clif_automatic) {
655+
policy = return_value_policy::copy;
656+
}
646657
return cast_const_raw_ptr( // Originally type_caster_generic::cast.
647658
st.first,
648659
policy,
@@ -653,6 +664,9 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
653664
}
654665

655666
static handle cast(T *src, return_value_policy policy, handle parent) {
667+
if (policy == return_value_policy::_clif_automatic) {
668+
policy = return_value_policy::reference;
669+
}
656670
return cast(const_cast<T const *>(src), policy, parent); // Mutbl2Const
657671
}
658672

@@ -867,7 +881,8 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
867881
static handle cast(std::unique_ptr<T, D> &&src, return_value_policy policy, handle parent) {
868882
if (policy != return_value_policy::automatic
869883
&& policy != return_value_policy::reference_internal
870-
&& policy != return_value_policy::move) {
884+
&& policy != return_value_policy::move
885+
&& policy != return_value_policy::_clif_automatic) {
871886
// SMART_HOLDER_WIP: IMPROVABLE: Error message.
872887
throw cast_error("Invalid return_value_policy for unique_ptr.");
873888
}

‎tests/test_return_value_policy_override.cpp‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,166 @@
1+
#include <pybind11/smart_holder.h>
2+
13
#include "pybind11_tests.h"
24

35
namespace test_return_value_policy_override {
46

57
struct some_type {};
68

9+
// cp = copyable, mv = movable, 1 = yes, 0 = no
10+
struct type_cp1_mv1 {
11+
std::string mtxt;
12+
explicit type_cp1_mv1(const std::string &mtxt_) : mtxt(mtxt_) {}
13+
type_cp1_mv1(const type_cp1_mv1 &other) { mtxt = other.mtxt + "_CpCtor"; }
14+
type_cp1_mv1(type_cp1_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; }
15+
type_cp1_mv1 &operator=(const type_cp1_mv1 &other) {
16+
mtxt = other.mtxt + "_CpCtor";
17+
return *this;
18+
}
19+
type_cp1_mv1 &operator=(type_cp1_mv1 &&other) noexcept {
20+
mtxt = other.mtxt + "_MvCtor";
21+
return *this;
22+
}
23+
};
24+
25+
// nocopy
26+
struct type_cp0_mv1 {
27+
std::string mtxt;
28+
explicit type_cp0_mv1(const std::string &mtxt_) : mtxt(mtxt_) {}
29+
type_cp0_mv1(const type_cp0_mv1 &) = delete;
30+
type_cp0_mv1(type_cp0_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; }
31+
type_cp0_mv1 &operator=(const type_cp0_mv1 &) = delete;
32+
type_cp0_mv1 &operator=(type_cp0_mv1 &&other) noexcept {
33+
mtxt = other.mtxt + "_MvCtor";
34+
return *this;
35+
}
36+
};
37+
38+
// nomove
39+
struct type_cp1_mv0 {
40+
std::string mtxt;
41+
explicit type_cp1_mv0(const std::string &mtxt_) : mtxt(mtxt_) {}
42+
type_cp1_mv0(const type_cp1_mv0 &other) { mtxt = other.mtxt + "_CpCtor"; }
43+
type_cp1_mv0(type_cp1_mv0 &&other) = delete;
44+
type_cp1_mv0 &operator=(const type_cp1_mv0 &other) {
45+
mtxt = other.mtxt + "_CpCtor";
46+
return *this;
47+
}
48+
type_cp1_mv0 &operator=(type_cp1_mv0 &&other) = delete;
49+
};
50+
51+
// nocopy_nomove
52+
struct type_cp0_mv0 {
53+
std::string mtxt;
54+
explicit type_cp0_mv0(const std::string &mtxt_) : mtxt(mtxt_) {}
55+
type_cp0_mv0(const type_cp0_mv0 &) = delete;
56+
type_cp0_mv0(type_cp0_mv0 &&other) = delete;
57+
type_cp0_mv0 &operator=(const type_cp0_mv0 &other) = delete;
58+
type_cp0_mv0 &operator=(type_cp0_mv0 &&other) = delete;
59+
};
60+
61+
type_cp1_mv1 return_value() { return type_cp1_mv1{"value"}; }
62+
63+
type_cp1_mv1 *return_pointer() {
64+
static type_cp1_mv1 value("pointer");
65+
return &value;
66+
}
67+
68+
const type_cp1_mv1 *return_const_pointer() {
69+
static type_cp1_mv1 value("const_pointer");
70+
return &value;
71+
}
72+
73+
type_cp1_mv1 &return_reference() {
74+
static type_cp1_mv1 value("reference");
75+
return value;
76+
}
77+
78+
const type_cp1_mv1 &return_const_reference() {
79+
static type_cp1_mv1 value("const_reference");
80+
return value;
81+
}
82+
83+
std::shared_ptr<type_cp1_mv1> return_shared_pointer() {
84+
return std::make_shared<type_cp1_mv1>("shared_pointer");
85+
}
86+
87+
std::unique_ptr<type_cp1_mv1> return_unique_pointer() {
88+
return std::unique_ptr<type_cp1_mv1>(new type_cp1_mv1("unique_pointer"));
89+
}
90+
91+
type_cp0_mv1 return_value_nocopy() { return type_cp0_mv1{"value_nocopy"}; }
92+
93+
type_cp0_mv1 *return_pointer_nocopy() {
94+
static type_cp0_mv1 value("pointer_nocopy");
95+
return &value;
96+
}
97+
98+
const type_cp0_mv1 *return_const_pointer_nocopy() {
99+
static type_cp0_mv1 value("const_pointer_nocopy");
100+
return &value;
101+
}
102+
103+
type_cp0_mv1 &return_reference_nocopy() {
104+
static type_cp0_mv1 value("reference_nocopy");
105+
return value;
106+
}
107+
108+
std::shared_ptr<type_cp0_mv1> return_shared_pointer_nocopy() {
109+
return std::make_shared<type_cp0_mv1>("shared_pointer_nocopy");
110+
}
111+
112+
std::unique_ptr<type_cp0_mv1> return_unique_pointer_nocopy() {
113+
return std::unique_ptr<type_cp0_mv1>(new type_cp0_mv1("unique_pointer_nocopy"));
114+
}
115+
116+
type_cp1_mv0 *return_pointer_nomove() {
117+
static type_cp1_mv0 value("pointer_nomove");
118+
return &value;
119+
}
120+
121+
const type_cp1_mv0 *return_const_pointer_nomove() {
122+
static type_cp1_mv0 value("const_pointer_nomove");
123+
return &value;
124+
}
125+
126+
type_cp1_mv0 &return_reference_nomove() {
127+
static type_cp1_mv0 value("reference_nomove");
128+
return value;
129+
}
130+
131+
const type_cp1_mv0 &return_const_reference_nomove() {
132+
static type_cp1_mv0 value("const_reference_nomove");
133+
return value;
134+
}
135+
136+
std::shared_ptr<type_cp1_mv0> return_shared_pointer_nomove() {
137+
return std::make_shared<type_cp1_mv0>("shared_pointer_nomove");
138+
}
139+
140+
std::unique_ptr<type_cp1_mv0> return_unique_pointer_nomove() {
141+
return std::unique_ptr<type_cp1_mv0>(new type_cp1_mv0("unique_pointer_nomove"));
142+
}
143+
144+
type_cp0_mv0 *return_pointer_nocopy_nomove() {
145+
static type_cp0_mv0 value("pointer_nocopy_nomove");
146+
return &value;
147+
}
148+
149+
std::shared_ptr<type_cp0_mv0> return_shared_pointer_nocopy_nomove() {
150+
return std::make_shared<type_cp0_mv0>("shared_pointer_nocopy_nomove");
151+
}
152+
153+
std::unique_ptr<type_cp0_mv0> return_unique_pointer_nocopy_nomove() {
154+
return std::unique_ptr<type_cp0_mv0>(new type_cp0_mv0("unique_pointer_nocopy_nomove"));
155+
}
156+
7157
} // namespace test_return_value_policy_override
8158

9159
using test_return_value_policy_override::some_type;
160+
using test_return_value_policy_override::type_cp0_mv0;
161+
using test_return_value_policy_override::type_cp0_mv1;
162+
using test_return_value_policy_override::type_cp1_mv0;
163+
using test_return_value_policy_override::type_cp1_mv1;
10164

11165
namespace pybind11 {
12166
namespace detail {
@@ -51,6 +205,11 @@ struct type_caster<some_type> : type_caster_base<some_type> {
51205
} // namespace detail
52206
} // namespace pybind11
53207

208+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv1)
209+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv1)
210+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv0)
211+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv0)
212+
54213
TEST_SUBMODULE(return_value_policy_override, m) {
55214
m.def("return_value_with_default_policy", []() { return some_type(); });
56215
m.def(
@@ -79,4 +238,86 @@ TEST_SUBMODULE(return_value_policy_override, m) {
79238
return &value;
80239
},
81240
py::return_value_policy::_clif_automatic);
241+
242+
py::classh<type_cp1_mv1>(m, "type_cp1_mv1")
243+
.def(py::init<std::string>())
244+
.def_readonly("mtxt", &type_cp1_mv1::mtxt);
245+
m.def("return_value",
246+
&test_return_value_policy_override::return_value,
247+
py::return_value_policy::_clif_automatic);
248+
m.def("return_pointer",
249+
&test_return_value_policy_override::return_pointer,
250+
py::return_value_policy::_clif_automatic);
251+
m.def("return_const_pointer",
252+
&test_return_value_policy_override::return_const_pointer,
253+
py::return_value_policy::_clif_automatic);
254+
m.def("return_reference",
255+
&test_return_value_policy_override::return_reference,
256+
py::return_value_policy::_clif_automatic);
257+
m.def("return_const_reference",
258+
&test_return_value_policy_override::return_const_reference,
259+
py::return_value_policy::_clif_automatic);
260+
m.def("return_unique_pointer",
261+
&test_return_value_policy_override::return_unique_pointer,
262+
py::return_value_policy::_clif_automatic);
263+
m.def("return_shared_pointer",
264+
&test_return_value_policy_override::return_shared_pointer,
265+
py::return_value_policy::_clif_automatic);
266+
267+
py::classh<type_cp0_mv1>(m, "type_cp0_mv1")
268+
.def(py::init<std::string>())
269+
.def_readonly("mtxt", &type_cp0_mv1::mtxt);
270+
m.def("return_value_nocopy",
271+
&test_return_value_policy_override::return_value_nocopy,
272+
py::return_value_policy::_clif_automatic);
273+
m.def("return_pointer_nocopy",
274+
&test_return_value_policy_override::return_pointer_nocopy,
275+
py::return_value_policy::_clif_automatic);
276+
m.def("return_const_pointer_nocopy",
277+
&test_return_value_policy_override::return_const_pointer_nocopy,
278+
py::return_value_policy::_clif_automatic);
279+
m.def("return_reference_nocopy",
280+
&test_return_value_policy_override::return_reference_nocopy,
281+
py::return_value_policy::_clif_automatic);
282+
m.def("return_shared_pointer_nocopy",
283+
&test_return_value_policy_override::return_shared_pointer_nocopy,
284+
py::return_value_policy::_clif_automatic);
285+
m.def("return_unique_pointer_nocopy",
286+
&test_return_value_policy_override::return_unique_pointer_nocopy,
287+
py::return_value_policy::_clif_automatic);
288+
289+
py::classh<type_cp1_mv0>(m, "type_cp1_mv0")
290+
.def(py::init<std::string>())
291+
.def_readonly("mtxt", &type_cp1_mv0::mtxt);
292+
m.def("return_pointer_nomove",
293+
&test_return_value_policy_override::return_pointer_nomove,
294+
py::return_value_policy::_clif_automatic);
295+
m.def("return_const_pointer_nomove",
296+
&test_return_value_policy_override::return_const_pointer_nomove,
297+
py::return_value_policy::_clif_automatic);
298+
m.def("return_reference_nomove",
299+
&test_return_value_policy_override::return_reference_nomove,
300+
py::return_value_policy::_clif_automatic);
301+
m.def("return_const_reference_nomove",
302+
&test_return_value_policy_override::return_const_reference_nomove,
303+
py::return_value_policy::_clif_automatic);
304+
m.def("return_shared_pointer_nomove",
305+
&test_return_value_policy_override::return_shared_pointer_nomove,
306+
py::return_value_policy::_clif_automatic);
307+
m.def("return_unique_pointer_nomove",
308+
&test_return_value_policy_override::return_unique_pointer_nomove,
309+
py::return_value_policy::_clif_automatic);
310+
311+
py::classh<type_cp0_mv0>(m, "type_cp0_mv0")
312+
.def(py::init<std::string>())
313+
.def_readonly("mtxt", &type_cp0_mv0::mtxt);
314+
m.def("return_pointer_nocopy_nomove",
315+
&test_return_value_policy_override::return_pointer_nocopy_nomove,
316+
py::return_value_policy::_clif_automatic);
317+
m.def("return_shared_pointer_nocopy_nomove",
318+
&test_return_value_policy_override::return_shared_pointer_nocopy_nomove,
319+
py::return_value_policy::_clif_automatic);
320+
m.def("return_unique_pointer_nocopy_nomove",
321+
&test_return_value_policy_override::return_unique_pointer_nocopy_nomove,
322+
py::return_value_policy::_clif_automatic);
82323
}

‎tests/test_return_value_policy_override.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
import re
2+
3+
import pytest
4+
15
from pybind11_tests import return_value_policy_override as m
26

37

@@ -11,3 +15,33 @@ def test_return_pointer():
1115
assert m.return_pointer_with_default_policy() == "automatic"
1216
assert m.return_pointer_with_policy_move() == "move"
1317
assert m.return_pointer_with_policy_clif_automatic() == "_clif_automatic"
18+
19+
20+
@pytest.mark.parametrize(
21+
"func, expected",
22+
[
23+
(m.return_value, "value(_MvCtor)*_MvCtor"),
24+
(m.return_pointer, "pointer"),
25+
(m.return_const_pointer, "const_pointer_CpCtor"),
26+
(m.return_reference, "reference_MvCtor"),
27+
(m.return_const_reference, "const_reference_CpCtor"),
28+
(m.return_unique_pointer, "unique_pointer"),
29+
(m.return_shared_pointer, "shared_pointer"),
30+
(m.return_value_nocopy, "value_nocopy(_MvCtor)*_MvCtor"),
31+
(m.return_pointer_nocopy, "pointer_nocopy"),
32+
(m.return_reference_nocopy, "reference_nocopy_MvCtor"),
33+
(m.return_unique_pointer_nocopy, "unique_pointer_nocopy"),
34+
(m.return_shared_pointer_nocopy, "shared_pointer_nocopy"),
35+
(m.return_pointer_nomove, "pointer_nomove"),
36+
(m.return_const_pointer_nomove, "const_pointer_nomove_CpCtor"),
37+
(m.return_reference_nomove, "reference_nomove_CpCtor"),
38+
(m.return_const_reference_nomove, "const_reference_nomove_CpCtor"),
39+
(m.return_unique_pointer_nomove, "unique_pointer_nomove"),
40+
(m.return_shared_pointer_nomove, "shared_pointer_nomove"),
41+
(m.return_pointer_nocopy_nomove, "pointer_nocopy_nomove"),
42+
(m.return_unique_pointer_nocopy_nomove, "unique_pointer_nocopy_nomove"),
43+
(m.return_shared_pointer_nocopy_nomove, "shared_pointer_nocopy_nomove"),
44+
],
45+
)
46+
def test_clif_automatic_return_value_policy_override(func, expected):
47+
assert re.match(expected, func().mtxt)

0 commit comments

Comments
 (0)