Skip to content

Commit cfeb6c7

Browse files
committed
Avoid unnecessary Python type checks and conversions where appropriate
The pytype converting constructors are convenient and safe for user code, but for library internals the additional type checks and possible conversions are sometimes not desired. This commit adds `reinterpret_borrow<T>()` and `reinterpret_steal<T>()` which serve as the low-level unsafe counterparts of `cast<T>()`.
1 parent f7f73cd commit cfeb6c7

File tree

4 files changed

+28
-16
lines changed

4 files changed

+28
-16
lines changed

include/pybind11/cast.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class type_caster_generic {
199199
auto const &type_dict = get_internals().registered_types_py;
200200
bool new_style_class = PyType_Check(tobj);
201201
if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) {
202-
tuple parents(tobj->tp_bases, true);
202+
auto parents = reinterpret_borrow<tuple>(tobj->tp_bases);
203203
for (handle parent : parents) {
204204
bool result = load(src, convert, (PyTypeObject *) parent.ptr());
205205
if (result)
@@ -868,7 +868,7 @@ template <typename type, typename holder_type> class type_caster_holder : public
868868
auto const &type_dict = get_internals().registered_types_py;
869869
bool new_style_class = PyType_Check(tobj);
870870
if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) {
871-
tuple parents(tobj->tp_bases, true);
871+
auto parents = reinterpret_borrow<tuple>(tobj->tp_bases);
872872
for (handle parent : parents) {
873873
bool result = load(src, convert, (PyTypeObject *) parent.ptr());
874874
if (result)
@@ -963,7 +963,7 @@ struct pyobject_caster {
963963
bool load(handle src, bool /* convert */) {
964964
if (!isinstance<type>(src))
965965
return false;
966-
value = type(src, true);
966+
value = reinterpret_borrow<type>(src);
967967
return true;
968968
}
969969

@@ -1298,7 +1298,7 @@ class unpacking_collector {
12981298
void process(list &/*args_list*/, detail::kwargs_proxy kp) {
12991299
if (!kp)
13001300
return;
1301-
for (const auto &k : dict(kp, true)) {
1301+
for (const auto &k : reinterpret_borrow<dict>(kp)) {
13021302
if (m_kwargs.contains(k.first)) {
13031303
#if defined(NDEBUG)
13041304
multiple_values_error();

include/pybind11/pybind11.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ class cpp_function : public function {
379379
result = PYBIND11_TRY_NEXT_OVERLOAD;
380380
try {
381381
for (; it != nullptr; it = it->next) {
382-
tuple args_(args, true);
382+
auto args_ = reinterpret_borrow<tuple>(args);
383383
size_t kwargs_consumed = 0;
384384

385385
/* For each overload:
@@ -499,7 +499,7 @@ class cpp_function : public function {
499499
msg += "\n";
500500
}
501501
msg += "\nInvoked with: ";
502-
tuple args_(args, true);
502+
auto args_ = reinterpret_borrow<tuple>(args);
503503
for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) {
504504
msg += static_cast<std::string>(pybind11::str(args_[ti]));
505505
if ((ti + 1) != args_.size() )
@@ -764,7 +764,7 @@ class generic_type : public object {
764764

765765
/// Helper function which tags all parents of a type using mult. inheritance
766766
void mark_parents_nonsimple(PyTypeObject *value) {
767-
tuple t(value->tp_bases, true);
767+
auto t = reinterpret_borrow<tuple>(value->tp_bases);
768768
for (handle h : t) {
769769
auto tinfo2 = get_type_info((PyTypeObject *) h.ptr());
770770
if (tinfo2)
@@ -932,7 +932,7 @@ class class_ : public detail::generic_type {
932932
static_assert(detail::all_of_t<is_valid_class_option, options...>::value,
933933
"Unknown/invalid class_ template parameters provided");
934934

935-
PYBIND11_OBJECT(class_, detail::generic_type, PyType_Check)
935+
PYBIND11_OBJECT(class_, generic_type, PyType_Check)
936936

937937
template <typename... Extra>
938938
class_(handle scope, const char *name, const Extra &... extra) {

include/pybind11/pytypes.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ using sequence_accessor = accessor<accessor_policies::sequence_item>;
4141
using list_accessor = accessor<accessor_policies::list_item>;
4242
using tuple_accessor = accessor<accessor_policies::tuple_item>;
4343

44+
/// Tag for choosing the non-converting py::object constructor
45+
struct noconvert { };
46+
4447
/// Tag and check to identify a class which implements the Python object API
4548
class pyobject_tag { };
4649
template <typename T> using is_pyobject = std::is_base_of<pyobject_tag, T>;
@@ -102,9 +105,11 @@ class handle : public detail::object_api<handle> {
102105
class object : public handle {
103106
public:
104107
object() = default;
108+
// Derived classes may override this constructor for Python type conversions
109+
object(handle h, bool borrowed) : handle(h) { if (borrowed) inc_ref(); }
110+
// This one must be inherited as is -- it's always just a pure pointer assignment
111+
object(handle h, bool borrowed, detail::noconvert) : object(h, borrowed) { }
105112
object(const object &o) : handle(o) { inc_ref(); }
106-
object(const handle &h, bool borrowed) : handle(h) { if (borrowed) inc_ref(); }
107-
object(PyObject *ptr, bool borrowed) : handle(ptr) { if (borrowed) inc_ref(); }
108113
object(object &&other) noexcept { m_ptr = other.m_ptr; other.m_ptr = nullptr; }
109114
~object() { dec_ref(); }
110115

@@ -137,6 +142,13 @@ class object : public handle {
137142
template <typename T> T cast() &&;
138143
};
139144

145+
/** Python type casts of the form `obj.cast<pytype>()` and `pytype(obj)` are equivalent
146+
and they do actual convertions, e.g. `list` -> `tuple`. In contrast, the following
147+
functions don't do any kind of conversion, they simply declare that a PyObject is a
148+
certain type and borrow or steal the reference. */
149+
template <typename T> T reinterpret_borrow(handle h) { return T(h, true, detail::noconvert()); }
150+
template <typename T> T reinterpret_steal(handle h) { return T(h, false, detail::noconvert()); }
151+
140152
/// Check if `obj` is an instance of type `T`
141153
template <typename T, detail::enable_if_t<std::is_base_of<object, T>::value, int> = 0>
142154
bool isinstance(handle obj) { return T::_check(obj); }
@@ -392,6 +404,7 @@ NAMESPACE_END(detail)
392404

393405
#define PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
394406
public: \
407+
using Parent::Parent; \
395408
PYBIND11_DEPRECATED("Use py::isinstance<py::python_type>(obj) instead") \
396409
bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \
397410
static bool _check(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); }
@@ -404,7 +417,6 @@ NAMESPACE_END(detail)
404417

405418
#define PYBIND11_OBJECT(Name, Parent, CheckFun) \
406419
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
407-
Name(handle h, bool borrowed) : Parent(h, borrowed) { } \
408420
/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
409421
Name(const object &o) : Parent(o) { }
410422

@@ -833,7 +845,7 @@ inline str repr(handle h) {
833845
Py_XDECREF(str_value); str_value = unicode;
834846
if (!str_value) throw error_already_set();
835847
#endif
836-
return {str_value, false};
848+
return reinterpret_steal<str>(str_value);
837849
}
838850

839851
NAMESPACE_BEGIN(detail)

include/pybind11/stl.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ template <typename Type, typename Key> struct set_caster {
3232
bool load(handle src, bool convert) {
3333
if (!isinstance<pybind11::set>(src))
3434
return false;
35-
pybind11::set s(src, true);
35+
auto s = reinterpret_borrow<pybind11::set>(src);
3636
value.clear();
3737
key_conv conv;
3838
for (auto entry : s) {
@@ -64,7 +64,7 @@ template <typename Type, typename Key, typename Value> struct map_caster {
6464
bool load(handle src, bool convert) {
6565
if (!isinstance<dict>(src))
6666
return false;
67-
dict d(src, true);
67+
auto d = reinterpret_borrow<dict>(src);
6868
key_conv kconv;
6969
value_conv vconv;
7070
value.clear();
@@ -99,7 +99,7 @@ template <typename Type, typename Value> struct list_caster {
9999
bool load(handle src, bool convert) {
100100
if (!isinstance<sequence>(src))
101101
return false;
102-
sequence s(src, true);
102+
auto s = reinterpret_borrow<sequence>(src);
103103
value_conv conv;
104104
value.clear();
105105
reserve_maybe(s, &value);
@@ -144,7 +144,7 @@ template <typename Type, size_t Size> struct type_caster<std::array<Type, Size>>
144144
bool load(handle src, bool convert) {
145145
if (!isinstance<list>(src))
146146
return false;
147-
list l(src, true);
147+
auto l = reinterpret_borrow<list>(src);
148148
if (l.size() != Size)
149149
return false;
150150
value_conv conv;

0 commit comments

Comments
 (0)