Make PYBIND11_OBJECT_CVT only convert if the type check fails
Currently types that are capable of conversion always call their convert
function when invoked with a `py::object` which is actually the correct
type. This means that code such as `py::cast<py::list>(obj)` and
`py::list l(obj.attr("list"))` make copies, which was an oversight
rather than an intentional feature.
While at first glance there might be something behind having
`py::list(obj)` make a copy (as it would in Python), this would be
inconsistent when you dig a little deeper because `py::list(l)`
*doesn't* make a copy for an existing `py::list l`, and having an
inconsistency within C++ would be worse than a C++ <-> Python
inconsistency.
It is possible to get around the copying using a
`reinterpret_borrow<list>(o)` (and this commit fixes one place, in
`embed.h`, that does so), but that seems a misuse of
`reinterpret_borrow`, which is really supposed to be just for dealing
with raw python-returned values, not `py::object`-derived wrappers which
are supposed to be higher level.
This changes the constructor of such converting types (i.e. anything
using PYBIND11_OBJECT_CVT -- `str`, `bool_`, `int_`, `float_`, `tuple`,
`dict`, `list`, `set`, `memoryview`) to reference rather than copy when
the check function passes.
It also adds an `object &&` constructor that is slightly more efficient
by avoiding an inc_ref when the check function passes.
diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h
index 0eb656b..98bf769 100644
--- a/include/pybind11/embed.h
+++ b/include/pybind11/embed.h
@@ -99,8 +99,7 @@
Py_InitializeEx(init_signal_handlers ? 1 : 0);
// Make .py files in the working directory available by default
- auto sys_path = reinterpret_borrow<list>(module::import("sys").attr("path"));
- sys_path.append(".");
+ module::import("sys").attr("path").cast<list>().append(".");
}
/** \rst
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 095d40f..b652b8e 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -731,7 +731,12 @@
#define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
- Name(const object &o) : Parent(ConvertFun(o.ptr()), stolen_t{}) { if (!m_ptr) throw error_already_set(); }
+ Name(const object &o) \
+ : Parent(check_(o) ? o.inc_ref().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
+ { if (!m_ptr) throw error_already_set(); } \
+ Name(object &&o) \
+ : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
+ { if (!m_ptr) throw error_already_set(); }
#define PYBIND11_OBJECT(Name, Parent, CheckFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 1c75c47..b8dad84 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -174,9 +174,20 @@
}
inputs = {k.__name__: v for k, v in data.items()}
expected = {k.__name__: k(v) for k, v in data.items()}
+
assert m.converting_constructors(inputs) == expected
assert m.cast_functions(inputs) == expected
+ # Converting constructors and cast functions should just reference rather
+ # than copy when no conversion is needed:
+ noconv1 = m.converting_constructors(expected)
+ for k in noconv1:
+ assert noconv1[k] is expected[k]
+
+ noconv2 = m.cast_functions(expected)
+ for k in noconv2:
+ assert noconv2[k] is expected[k]
+
def test_implicit_casting():
"""Tests implicit casting when assigning or appending to dicts and lists."""