From 05127a36a0ebf1ac25f12ddd11c901ad72e03f42 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Sun, 23 Oct 2016 14:50:08 +0200 Subject: [PATCH 1/4] Add py::isinstance(obj) for generalized Python type checking Allows checking the Python types before creating an object instead of after. For example: ```c++ auto l = list(ptr, true); if (l.check()) // ... ``` The above is replaced with: ```c++ if (isinstance(ptr)) { auto l = reinterpret_borrow(ptr); // ... } ``` This deprecates `py::object::check()`. `py::isinstance()` covers the same use case, but it can also check for user-defined types: ```c++ class Pet { ... }; py::class_(...); m.def("is_pet", [](py::object obj) { return py::isinstance(obj); // works as expected }); ``` --- include/pybind11/cast.h | 41 +++++++++++++++++++++++++------------- include/pybind11/eigen.h | 4 ++-- include/pybind11/numpy.h | 20 ++++++++++++++++--- include/pybind11/pytypes.h | 16 ++++++++++++++- include/pybind11/stl.h | 16 +++++++-------- tests/test_inheritance.cpp | 14 +++++++++++++ tests/test_inheritance.py | 8 ++++++++ 7 files changed, 91 insertions(+), 28 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0b6ff08596..90d9d4b33b 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -40,12 +40,8 @@ PYBIND11_NOINLINE inline internals &get_internals() { return *internals_ptr; handle builtins(PyEval_GetBuiltins()); const char *id = PYBIND11_INTERNALS_ID; - capsule caps; - if (builtins.contains(id)) { - caps = builtins[id]; - } - if (caps.check()) { - internals_ptr = caps; + if (builtins.contains(id) && isinstance(builtins[id])) { + internals_ptr = capsule(builtins[id]); } else { internals_ptr = new internals(); #if defined(WITH_THREAD) @@ -111,6 +107,17 @@ PYBIND11_NOINLINE inline handle get_type_handle(const std::type_info &tp, bool t return handle(type_info ? ((PyObject *) type_info->type) : nullptr); } +PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_info &tp) { + const auto type = detail::get_type_handle(tp, false); + if (!type) + return false; + + const auto result = PyObject_IsInstance(obj.ptr(), type.ptr()); + if (result == -1) + throw error_already_set(); + return result != 0; +} + PYBIND11_NOINLINE inline std::string error_string() { if (!PyErr_Occurred()) { PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred"); @@ -536,9 +543,8 @@ template <> class type_caster : public type_caster { } /* Check if this is a capsule */ - capsule c(h, true); - if (c.check()) { - value = (void *) c; + if (isinstance(h)) { + value = capsule(h, true); return true; } @@ -986,13 +992,17 @@ template <> struct handle_type_name { static PYBIND11_DESCR name() { retur template <> struct handle_type_name { static PYBIND11_DESCR name() { return _("**kwargs"); } }; template -struct type_caster::value>> { -public: - template ::value, int> = 0> - bool load(handle src, bool /* convert */) { value = type(src); return value.check(); } +struct pyobject_caster { + template ::value, int> = 0> + bool load(handle src, bool /* convert */) { value = src; return static_cast(value); } template ::value, int> = 0> - bool load(handle src, bool /* convert */) { value = type(src, true); return value.check(); } + bool load(handle src, bool /* convert */) { + if (!isinstance(src)) + return false; + value = type(src, true); + return true; + } static handle cast(const handle &src, return_value_policy /* policy */, handle /* parent */) { return src.inc_ref(); @@ -1000,6 +1010,9 @@ struct type_caster::value>> { PYBIND11_TYPE_CASTER(type, handle_type_name::name()); }; +template +class type_caster::value>> : public pyobject_caster { }; + // Our conditions for enabling moving are quite restrictive: // At compile time: // - T needs to be a non-const, non-pointer, non-reference type diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index d8bf41cd0a..3da857b712 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -55,7 +55,7 @@ struct type_caster::value && !is_eigen_re bool load(handle src, bool) { array_t buf(src, true); - if (!buf.check()) + if (!buf) return false; if (buf.ndim() == 1) { @@ -201,7 +201,7 @@ struct type_caster::value>> { auto shape = pybind11::tuple((pybind11::object) obj.attr("shape")); auto nnz = obj.attr("nnz").cast(); - if (!values.check() || !innerIndices.check() || !outerIndices.check()) + if (!values || !innerIndices || !outerIndices) return false; value = Eigen::MappedSparseMatrix( diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 5309997c43..2ffbc5f5cb 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -324,10 +324,9 @@ class array : public buffer { int flags = 0; if (base && ptr) { - array base_array(base, true); - if (base_array.check()) + if (isinstance(base)) /* Copy flags from base (except baseship bit) */ - flags = base_array.flags() & ~detail::npy_api::NPY_ARRAY_OWNDATA_; + flags = array(base, true).flags() & ~detail::npy_api::NPY_ARRAY_OWNDATA_; else /* Writable by default, easy to downgrade later on if needed */ flags = detail::npy_api::NPY_ARRAY_WRITEABLE_; @@ -627,6 +626,21 @@ struct format_descriptor::value>> { }; NAMESPACE_BEGIN(detail) +template +struct pyobject_caster> { + using type = array_t; + + bool load(handle src, bool /* convert */) { + value = type(src, true); + return static_cast(value); + } + + static handle cast(const handle &src, return_value_policy /* policy */, handle /* parent */) { + return src.inc_ref(); + } + PYBIND11_TYPE_CASTER(type, handle_type_name::name()); +}; + template struct is_std_array : std::false_type { }; template struct is_std_array> : std::true_type { }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index bfe169a726..ced8eb0765 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -22,6 +22,7 @@ struct arg; struct arg_v; NAMESPACE_BEGIN(detail) class args_proxy; +inline bool isinstance_generic(handle obj, const std::type_info &tp); // Accessor forward declarations template class accessor; @@ -91,6 +92,7 @@ class handle : public detail::object_api { explicit operator bool() const { return m_ptr != nullptr; } bool operator==(const handle &h) const { return m_ptr == h.m_ptr; } bool operator!=(const handle &h) const { return m_ptr != h.m_ptr; } + PYBIND11_DEPRECATED("Use handle::operator bool() instead") bool check() const { return m_ptr != nullptr; } protected: PyObject *m_ptr = nullptr; @@ -135,6 +137,16 @@ class object : public handle { template T cast() &&; }; +/// Check if `obj` is an instance of type `T` +template ::value, int> = 0> +bool isinstance(handle obj) { return T::_check(obj); } + +template ::value, int> = 0> +bool isinstance(handle obj) { return detail::isinstance_generic(obj, typeid(T)); } + +template <> inline bool isinstance(handle obj) = delete; +template <> inline bool isinstance(handle obj) { return obj.ptr() != nullptr; } + inline bool hasattr(handle obj, handle name) { return PyObject_HasAttr(obj.ptr(), name.ptr()) == 1; } @@ -386,7 +398,9 @@ NAMESPACE_END(detail) Name(object&& o) noexcept : Parent(std::move(o)) { CvtStmt; } \ Name& operator=(object&& o) noexcept { (void) object::operator=(std::move(o)); CvtStmt; return *this; } \ Name& operator=(const object& o) { return static_cast(object::operator=(o)); CvtStmt; } \ - bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } + PYBIND11_DEPRECATED("Use py::isinstance(obj) instead") \ + bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \ + static bool _check(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } #define PYBIND11_OBJECT(Name, Parent, CheckFun) \ PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 04d2daad17..ed8ab5cbb9 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -45,9 +45,9 @@ template struct set_caster { using key_conv = make_caster; bool load(handle src, bool convert) { - pybind11::set s(src, true); - if (!s.check()) + if (!isinstance(src)) return false; + pybind11::set s(src, true); value.clear(); key_conv conv; for (auto entry : s) { @@ -77,9 +77,9 @@ template struct map_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - dict d(src, true); - if (!d.check()) + if (!isinstance(src)) return false; + dict d(src, true); key_conv kconv; value_conv vconv; value.clear(); @@ -112,9 +112,9 @@ template struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - sequence s(src, true); - if (!s.check()) + if (!isinstance(src)) return false; + sequence s(src, true); value_conv conv; value.clear(); reserve_maybe(s, &value); @@ -157,9 +157,9 @@ template struct type_caster> using value_conv = make_caster; bool load(handle src, bool convert) { - list l(src, true); - if (!l.check()) + if (!isinstance(src)) return false; + list l(src, true); if (l.size() != Size) return false; value_conv conv; diff --git a/tests/test_inheritance.cpp b/tests/test_inheritance.cpp index f43edc261f..2ec0b4a7a8 100644 --- a/tests/test_inheritance.cpp +++ b/tests/test_inheritance.cpp @@ -83,4 +83,18 @@ test_initializer inheritance([](py::module &m) { return new BaseClass(); }); m.def("return_none", []() -> BaseClass* { return nullptr; }); + + m.def("test_isinstance", [](py::list l) { + struct Unregistered { }; // checks missing type_info code path + + return py::make_tuple( + py::isinstance(l[0]), + py::isinstance(l[1]), + py::isinstance(l[2]), + py::isinstance(l[3]), + py::isinstance(l[4]), + py::isinstance(l[5]), + py::isinstance(l[6]) + ); + }); }); diff --git a/tests/test_inheritance.py b/tests/test_inheritance.py index 351fe6b2ce..7bb52be02b 100644 --- a/tests/test_inheritance.py +++ b/tests/test_inheritance.py @@ -45,3 +45,11 @@ def test_automatic_upcasting(): assert type(return_class_n(2)).__name__ == "DerivedClass2" assert type(return_class_n(0)).__name__ == "BaseClass" assert type(return_class_n(1)).__name__ == "DerivedClass1" + + +def test_isinstance(): + from pybind11_tests import test_isinstance, Pet, Dog + + objects = [tuple(), dict(), Pet("Polly", "parrot")] + [Dog("Molly")] * 4 + expected = (True, True, True, True, True, False, False) + assert test_isinstance(objects) == expected From 294832e5d93e426c9944fafb73bb76507b688283 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Tue, 25 Oct 2016 22:12:39 +0200 Subject: [PATCH 2/4] Add default and converting constructors for all concrete Python types * Deprecate the `py::object::str()` member function since `py::str(obj)` is now equivalent and preferred * Make `py::repr()` a free function * Make sure obj.cast() works as expected when T is a Python type `obj.cast()` should be the same as `T(obj)`, i.e. it should convert the given object to a different Python type. However, `obj.cast()` usually calls `type_caster::load()` which only checks the type without doing any actual conversion. That causes a very unexpected `cast_error`. This commit makes it so that `obj.cast()` and `T(obj)` are the same when T is a Python type. * Simplify pytypes converting constructor implementation It's not necessary to maintain a full set of converting constructors and assignment operators + const& and &&. A single converting const& constructor will work and there is no impact on binary size. On the other hand, the conversion functions can be significantly simplified. --- docs/changelog.rst | 2 +- include/pybind11/attr.h | 4 +- include/pybind11/cast.h | 22 +++--- include/pybind11/common.h | 2 - include/pybind11/numpy.h | 8 +-- include/pybind11/pybind11.h | 12 ++-- include/pybind11/pytypes.h | 138 ++++++++++++++++++++---------------- include/pybind11/stl.h | 2 +- tests/test_numpy_dtypes.cpp | 16 ++--- tests/test_python_types.cpp | 46 +++++++++++- tests/test_python_types.py | 26 +++++++ 11 files changed, 179 insertions(+), 99 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 87d38d3d83..15cf8d9598 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,7 +28,7 @@ Breaking changes queued for v2.0.0 (Not yet released) (now uses prefix increment operator); it now also accepts iterators with different begin/end types as long as they are equality comparable. * ``arg()`` now accepts a wider range of argument types for default values -* Added ``repr()`` method to the ``handle`` class. +* Added ``py::repr()`` function which is equivalent to Python's builtin ``repr()``. * Added support for registering structured dtypes via ``PYBIND11_NUMPY_DTYPE()`` macro. * Added ``PYBIND11_STR_TYPE`` macro which maps to the ``builtins.str`` type. * Added a simplified ``buffer_info`` constructor for 1-dimensional buffers. diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 1ea925c187..d728210e0e 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -265,9 +265,9 @@ template <> struct process_attribute : process_attribute_default { auto descr = "'" + std::string(a.name) + ": " + a.type + "'"; if (r->class_) { if (r->name) - descr += " in method '" + (std::string) r->class_.str() + "." + (std::string) r->name + "'"; + descr += " in method '" + (std::string) str(r->class_) + "." + (std::string) r->name + "'"; else - descr += " in method of '" + (std::string) r->class_.str() + "'"; + descr += " in method of '" + (std::string) str(r->class_) + "'"; } else if (r->name) { descr += " in function named '" + (std::string) r->name + "'"; } diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 90d9d4b33b..fa9d21d7c0 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -132,7 +132,7 @@ PYBIND11_NOINLINE inline std::string error_string() { errorString += ": "; } if (scope.value) - errorString += (std::string) handle(scope.value).str(); + errorString += (std::string) str(scope.value); PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); @@ -1056,7 +1056,7 @@ template type_caster &load_type(type_ca throw cast_error("Unable to cast Python instance to C++ type (compile in debug mode for details)"); #else throw cast_error("Unable to cast Python instance of type " + - (std::string) handle.get_type().str() + " to C++ type '" + type_id() + "''"); + (std::string) str(handle.get_type()) + " to C++ type '" + type_id() + "''"); #endif } return conv; @@ -1070,16 +1070,20 @@ template make_caster load_type(const handle &handle) { NAMESPACE_END(detail) -template T cast(const handle &handle) { +template ::value, int> = 0> +T cast(const handle &handle) { static_assert(!detail::cast_is_temporary_value_reference::value, "Unable to cast type to reference: value is local to type caster"); using type_caster = detail::make_caster; return detail::load_type(handle).operator typename type_caster::template cast_op_type(); } -template object cast(const T &value, - return_value_policy policy = return_value_policy::automatic_reference, - handle parent = handle()) { +template ::value, int> = 0> +T cast(const handle &handle) { return {handle, true}; } + +template ::value, int> = 0> +object cast(const T &value, return_value_policy policy = return_value_policy::automatic_reference, + handle parent = handle()) { if (policy == return_value_policy::automatic) policy = std::is_pointer::value ? return_value_policy::take_ownership : return_value_policy::copy; else if (policy == return_value_policy::automatic_reference) @@ -1097,7 +1101,7 @@ detail::enable_if_t::value || detail::move_if_unreference throw cast_error("Unable to cast Python instance to C++ rvalue: instance has multiple references" " (compile in debug mode for details)"); #else - throw cast_error("Unable to move from Python " + (std::string) obj.get_type().str() + + throw cast_error("Unable to move from Python " + (std::string) str(obj.get_type()) + " instance to C++ " + type_id() + " instance: instance has multiple references"); #endif @@ -1274,7 +1278,7 @@ class unpacking_collector { int _[] = { 0, (process(args_list, std::forward(values)), 0)... }; ignore_unused(_); - m_args = object(PyList_AsTuple(args_list.ptr()), false); + m_args = std::move(args_list); } const tuple &args() const & { return m_args; } @@ -1336,7 +1340,7 @@ class unpacking_collector { #if defined(NDEBUG) multiple_values_error(); #else - multiple_values_error(k.first.str()); + multiple_values_error(str(k.first)); #endif } m_kwargs[k.first] = k.second; diff --git a/include/pybind11/common.h b/include/pybind11/common.h index f6e54e7f28..e0ebedeaf9 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -111,7 +111,6 @@ #define PYBIND11_BYTES_FROM_STRING_AND_SIZE PyBytes_FromStringAndSize #define PYBIND11_BYTES_AS_STRING_AND_SIZE PyBytes_AsStringAndSize #define PYBIND11_BYTES_AS_STRING PyBytes_AsString -#define PYBIND11_BYTES_CHECK PyBytes_Check #define PYBIND11_LONG_CHECK(o) PyLong_Check(o) #define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o) #define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) PyLong_AsUnsignedLongLong(o) @@ -130,7 +129,6 @@ #define PYBIND11_BYTES_FROM_STRING_AND_SIZE PyString_FromStringAndSize #define PYBIND11_BYTES_AS_STRING_AND_SIZE PyString_AsStringAndSize #define PYBIND11_BYTES_AS_STRING PyString_AsString -#define PYBIND11_BYTES_CHECK PyString_Check #define PYBIND11_LONG_CHECK(o) (PyInt_Check(o) || PyLong_Check(o)) #define PYBIND11_LONG_AS_LONGLONG(o) (PyInt_Check(o) ? (long long) PyLong_AsLong(o) : PyLong_AsLongLong(o)) #define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) (PyInt_Check(o) ? (unsigned long long) PyLong_AsUnsignedLong(o) : PyLong_AsUnsignedLongLong(o)) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 2ffbc5f5cb..9fe4385029 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -540,10 +540,12 @@ class array : public buffer { template class array_t : public array { public: - PYBIND11_OBJECT_CVT(array_t, array, is_non_null, m_ptr = ensure_(m_ptr)); - array_t() : array() { } + array_t(handle h, bool borrowed) : array(h, borrowed) { m_ptr = ensure_(m_ptr); } + + array_t(const object &o) : array(o) { m_ptr = ensure_(m_ptr); } + explicit array_t(const buffer_info& info) : array(info) { } array_t(const std::vector &shape, @@ -588,8 +590,6 @@ template class array_t : public return *(static_cast(array::mutable_data()) + byte_offset(size_t(index)...) / itemsize()); } - static bool is_non_null(PyObject *ptr) { return ptr != nullptr; } - static PyObject *ensure_(PyObject *ptr) { if (ptr == nullptr) return nullptr; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 0cd35c9ba0..ca116eb4ce 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -504,7 +504,7 @@ class cpp_function : public function { msg += "\nInvoked with: "; tuple args_(args, true); for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) { - msg += static_cast(static_cast(args_[ti]).str()); + msg += static_cast(pybind11::str(args_[ti])); if ((ti + 1) != args_.size() ) msg += ", "; } @@ -665,11 +665,9 @@ class generic_type : public object { #endif size_t num_bases = rec->bases.size(); - tuple bases(num_bases); - for (size_t i = 0; i < num_bases; ++i) - bases[i] = rec->bases[i]; + auto bases = tuple(rec->bases); - std::string full_name = (scope_module ? ((std::string) scope_module.str() + "." + rec->name) + std::string full_name = (scope_module ? ((std::string) pybind11::str(scope_module) + "." + rec->name) : std::string(rec->name)); char *tp_doc = nullptr; @@ -1470,7 +1468,7 @@ NAMESPACE_BEGIN(detail) PYBIND11_NOINLINE inline void print(tuple args, dict kwargs) { auto strings = tuple(args.size()); for (size_t i = 0; i < args.size(); ++i) { - strings[i] = args[i].str(); + strings[i] = str(args[i]); } auto sep = kwargs.contains("sep") ? kwargs["sep"] : cast(" "); auto line = sep.attr("join")(strings); @@ -1654,7 +1652,7 @@ inline function get_type_overload(const void *this_ptr, const detail::type_info /* Don't call dispatch code if invoked from overridden function */ PyFrameObject *frame = PyThreadState_Get()->frame; - if (frame && (std::string) pybind11::handle(frame->f_code->co_name).str() == name && + if (frame && (std::string) str(frame->f_code->co_name) == name && frame->f_code->co_argcount > 0) { PyFrame_FastToLocals(frame); PyObject *self_caller = PyDict_GetItem( diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index ced8eb0765..9257643e8b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -68,8 +68,8 @@ class object_api : public pyobject_tag { object call(Args&&... args) const; bool is_none() const { return derived().ptr() == Py_None; } + PYBIND11_DEPRECATED("Instead of obj.str(), use py::str(obj)") pybind11::str str() const; - pybind11::str repr() const; int ref_count() const { return static_cast(Py_REFCNT(derived().ptr())); } handle get_type() const; @@ -222,13 +222,13 @@ class accessor : public object_api> { template PYBIND11_DEPRECATED("Use of obj.attr(...) as bool is deprecated in favor of pybind11::hasattr(obj, ...)") - operator enable_if_t::value || + explicit operator enable_if_t::value || std::is_same::value, bool>() const { return hasattr(obj, key); } template PYBIND11_DEPRECATED("Use of obj[key] as bool is deprecated in favor of obj.contains(key)") - operator enable_if_t::value, bool>() const { + explicit operator enable_if_t::value, bool>() const { return obj.contains(key); } @@ -390,20 +390,23 @@ class unpacking_collector; NAMESPACE_END(detail) -#define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, CvtStmt) \ +#define PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ public: \ - Name(const handle &h, bool borrowed) : Parent(h, borrowed) { CvtStmt; } \ - /* These are deliberately not 'explicit' to allow implicit conversion from object: */ \ - Name(const object& o): Parent(o) { CvtStmt; } \ - Name(object&& o) noexcept : Parent(std::move(o)) { CvtStmt; } \ - Name& operator=(object&& o) noexcept { (void) object::operator=(std::move(o)); CvtStmt; return *this; } \ - Name& operator=(const object& o) { return static_cast(object::operator=(o)); CvtStmt; } \ PYBIND11_DEPRECATED("Use py::isinstance(obj) instead") \ bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \ static bool _check(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } +#define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \ + PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ + Name(handle h, bool borrowed) : Name(object(h, borrowed)) { } \ + /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ + Name(const object &o) : Parent(ConvertFun(o.ptr()), false) { if (!m_ptr) throw error_already_set(); } + #define PYBIND11_OBJECT(Name, Parent, CheckFun) \ - PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ) + PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ + Name(handle h, bool borrowed) : Parent(h, borrowed) { } \ + /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ + Name(const object &o) : Parent(o) { } #define PYBIND11_OBJECT_DEFAULT(Name, Parent, CheckFun) \ PYBIND11_OBJECT(Name, Parent, CheckFun) \ @@ -411,26 +414,9 @@ NAMESPACE_END(detail) class iterator : public object { public: - PYBIND11_OBJECT_CVT(iterator, object, PyIter_Check, value = object(); ready = false) - iterator() : object(), value(object()), ready(false) { } - iterator(const iterator& it) : object(it), value(it.value), ready(it.ready) { } - iterator(iterator&& it) : object(std::move(it)), value(std::move(it.value)), ready(it.ready) { } - - /** Caveat: this copy constructor does not (and cannot) clone the internal + /** Caveat: copying an iterator does not (and cannot) clone the internal state of the Python iterable */ - iterator &operator=(const iterator &it) { - (void) object::operator=(it); - value = it.value; - ready = it.ready; - return *this; - } - - iterator &operator=(iterator &&it) noexcept { - (void) object::operator=(std::move(it)); - value = std::move(it.value); - ready = it.ready; - return *this; - } + PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check) iterator& operator++() { if (m_ptr) @@ -465,8 +451,8 @@ class iterator : public object { void advance() { value = object(PyIter_Next(m_ptr), false); } private: - object value; - bool ready; + object value = {}; + bool ready = false; }; class iterable : public object { @@ -478,7 +464,7 @@ class bytes; class str : public object { public: - PYBIND11_OBJECT_DEFAULT(str, object, detail::PyUnicode_Check_Permissive) + PYBIND11_OBJECT_CVT(str, object, detail::PyUnicode_Check_Permissive, raw_str) str(const char *c, size_t n) : object(PyUnicode_FromStringAndSize(c, (ssize_t) n), false) { @@ -486,7 +472,7 @@ class str : public object { } // 'explicit' is explicitly omitted from the following constructors to allow implicit conversion to py::str from C++ string-like objects - str(const char *c) + str(const char *c = "") : object(PyUnicode_FromString(c), false) { if (!m_ptr) pybind11_fail("Could not allocate string object!"); } @@ -495,6 +481,8 @@ class str : public object { explicit str(const bytes &b); + explicit str(handle h) : object(raw_str(h.ptr()), false) { } + operator std::string() const { object temp = *this; if (PyUnicode_Check(m_ptr)) { @@ -513,6 +501,18 @@ class str : public object { str format(Args &&...args) const { return attr("format")(std::forward(args)...); } + +private: + /// Return string representation -- always returns a new reference, even if already a str + static PyObject *raw_str(PyObject *op) { + PyObject *str_value = PyObject_Str(op); +#if PY_MAJOR_VERSION < 3 + if (!str_value) throw error_already_set(); + PyObject *unicode = PyUnicode_FromEncodedObject(str_value, "utf-8", nullptr); + Py_XDECREF(str_value); str_value = unicode; +#endif + return str_value; + } }; inline namespace literals { @@ -522,10 +522,10 @@ inline str operator"" _s(const char *s, size_t size) { return {s, size}; } class bytes : public object { public: - PYBIND11_OBJECT_DEFAULT(bytes, object, PYBIND11_BYTES_CHECK) + PYBIND11_OBJECT(bytes, object, PYBIND11_BYTES_CHECK) // Allow implicit conversion: - bytes(const char *c) + bytes(const char *c = "") : object(PYBIND11_BYTES_FROM_STRING(c), false) { if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); } @@ -585,15 +585,25 @@ class none : public object { class bool_ : public object { public: - PYBIND11_OBJECT_DEFAULT(bool_, object, PyBool_Check) + PYBIND11_OBJECT_CVT(bool_, object, PyBool_Check, raw_bool) + bool_() : object(Py_False, true) { } // Allow implicit conversion from and to `bool`: bool_(bool value) : object(value ? Py_True : Py_False, true) { } operator bool() const { return m_ptr && PyLong_AsLong(m_ptr) != 0; } + +private: + /// Return the truth value of an object -- always returns a new reference + static PyObject *raw_bool(PyObject *op) { + const auto value = PyObject_IsTrue(op); + if (value == -1) return nullptr; + return handle(value ? Py_True : Py_False).inc_ref().ptr(); + } }; class int_ : public object { public: - PYBIND11_OBJECT_DEFAULT(int_, object, PYBIND11_LONG_CHECK) + PYBIND11_OBJECT_CVT(int_, object, PYBIND11_LONG_CHECK, PyNumber_Long) + int_() : object(PyLong_FromLong(0), false) { } // Allow implicit conversion from C++ integral types: template ::value, int> = 0> @@ -631,12 +641,12 @@ class int_ : public object { class float_ : public object { public: - PYBIND11_OBJECT_DEFAULT(float_, object, PyFloat_Check) + PYBIND11_OBJECT_CVT(float_, object, PyFloat_Check, PyNumber_Float) // Allow implicit conversion from float/double: float_(float value) : object(PyFloat_FromDouble((double) value), false) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } - float_(double value) : object(PyFloat_FromDouble((double) value), false) { + float_(double value = .0) : object(PyFloat_FromDouble((double) value), false) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } operator float() const { return (float) PyFloat_AsDouble(m_ptr); } @@ -685,7 +695,7 @@ class capsule : public object { class tuple : public object { public: - PYBIND11_OBJECT(tuple, object, PyTuple_Check) + PYBIND11_OBJECT_CVT(tuple, object, PyTuple_Check, PySequence_Tuple) explicit tuple(size_t size = 0) : object(PyTuple_New((ssize_t) size), false) { if (!m_ptr) pybind11_fail("Could not allocate tuple object!"); } @@ -695,7 +705,7 @@ class tuple : public object { class dict : public object { public: - PYBIND11_OBJECT(dict, object, PyDict_Check) + PYBIND11_OBJECT_CVT(dict, object, PyDict_Check, raw_dict) dict() : object(PyDict_New(), false) { if (!m_ptr) pybind11_fail("Could not allocate dict object!"); } @@ -711,6 +721,14 @@ class dict : public object { void clear() const { PyDict_Clear(ptr()); } bool contains(handle key) const { return PyDict_Contains(ptr(), key.ptr()) == 1; } bool contains(const char *key) const { return PyDict_Contains(ptr(), pybind11::str(key).ptr()) == 1; } + +private: + /// Call the `dict` Python type -- always returns a new reference + static PyObject *raw_dict(PyObject *op) { + if (PyDict_Check(op)) + return handle(op).inc_ref().ptr(); + return PyObject_CallFunctionObjArgs((PyObject *) &PyDict_Type, op, nullptr); + } }; class sequence : public object { @@ -722,7 +740,7 @@ class sequence : public object { class list : public object { public: - PYBIND11_OBJECT(list, object, PyList_Check) + PYBIND11_OBJECT_CVT(list, object, PyList_Check, PySequence_List) explicit list(size_t size = 0) : object(PyList_New((ssize_t) size), false) { if (!m_ptr) pybind11_fail("Could not allocate list object!"); } @@ -736,7 +754,7 @@ class kwargs : public dict { PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check) class set : public object { public: - PYBIND11_OBJECT(set, object, PySet_Check) + PYBIND11_OBJECT_CVT(set, object, PySet_Check, PySet_New) set() : object(PySet_New(nullptr), false) { if (!m_ptr) pybind11_fail("Could not allocate set object!"); } @@ -797,7 +815,7 @@ class memoryview : public object { pybind11_fail("Unable to create memoryview from buffer descriptor"); } - PYBIND11_OBJECT_DEFAULT(memoryview, object, PyMemoryView_Check) + PYBIND11_OBJECT_CVT(memoryview, object, PyMemoryView_Check, PyMemoryView_FromObject) }; inline size_t len(handle h) { @@ -807,6 +825,17 @@ inline size_t len(handle h) { return (size_t) result; } +inline str repr(handle h) { + PyObject *str_value = PyObject_Repr(h.ptr()); + if (!str_value) throw error_already_set(); +#if PY_MAJOR_VERSION < 3 + PyObject *unicode = PyUnicode_FromEncodedObject(str_value, "utf-8", nullptr); + Py_XDECREF(str_value); str_value = unicode; + if (!str_value) throw error_already_set(); +#endif + return {str_value, false}; +} + NAMESPACE_BEGIN(detail) template iterator object_api::begin() const { return {PyObject_GetIter(derived().ptr()), false}; } template iterator object_api::end() const { return {nullptr, false}; } @@ -820,24 +849,7 @@ template template bool object_api::contains(T &&key } template -pybind11::str object_api::str() const { - PyObject *str_value = PyObject_Str(derived().ptr()); -#if PY_MAJOR_VERSION < 3 - PyObject *unicode = PyUnicode_FromEncodedObject(str_value, "utf-8", nullptr); - Py_XDECREF(str_value); str_value = unicode; -#endif - return {str_value, false}; -} - -template -pybind11::str object_api::repr() const { - PyObject *str_value = PyObject_Repr(derived().ptr()); -#if PY_MAJOR_VERSION < 3 - PyObject *unicode = PyUnicode_FromEncodedObject(str_value, "utf-8", nullptr); - Py_XDECREF(str_value); str_value = unicode; -#endif - return {str_value, false}; -} +pybind11::str object_api::str() const { return pybind11::str(derived()); } template handle object_api::get_type() const { return (PyObject *) Py_TYPE(derived().ptr()); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ed8ab5cbb9..fc058ef25a 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -248,7 +248,7 @@ template<> struct type_caster NAMESPACE_END(detail) inline std::ostream &operator<<(std::ostream &os, const handle &obj) { - os << (std::string) obj.str(); + os << (std::string) str(obj); return os; } diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index 8f680c0716..20cb36269d 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -196,14 +196,14 @@ py::list print_format_descriptors() { py::list print_dtypes() { const auto dtypes = { - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str(), - py::dtype::of().str() + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()), + py::str(py::dtype::of()) }; auto l = py::list(); for (const auto &s : dtypes) { diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index 97cdc2e87c..05ba4af9e2 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -153,8 +153,8 @@ class ExamplePythonTypes { } void test_print(const py::object& obj) { - py::print(obj.str()); - py::print(obj.repr()); + py::print(py::str(obj)); + py::print(py::repr(obj)); } static int value; @@ -321,4 +321,46 @@ test_initializer python_types([](py::module &m) { m.attr("has_optional") = py::cast(has_optional); m.attr("has_exp_optional") = py::cast(has_exp_optional); + + m.def("test_default_constructors", []() { + return py::dict( + "str"_a=py::str(), + "bool"_a=py::bool_(), + "int"_a=py::int_(), + "float"_a=py::float_(), + "tuple"_a=py::tuple(), + "list"_a=py::list(), + "dict"_a=py::dict(), + "set"_a=py::set() + ); + }); + + m.def("test_converting_constructors", [](py::dict d) { + return py::dict( + "str"_a=py::str(d["str"]), + "bool"_a=py::bool_(d["bool"]), + "int"_a=py::int_(d["int"]), + "float"_a=py::float_(d["float"]), + "tuple"_a=py::tuple(d["tuple"]), + "list"_a=py::list(d["list"]), + "dict"_a=py::dict(d["dict"]), + "set"_a=py::set(d["set"]), + "memoryview"_a=py::memoryview(d["memoryview"]) + ); + }); + + m.def("test_cast_functions", [](py::dict d) { + // When converting between Python types, obj.cast() should be the same as T(obj) + return py::dict( + "str"_a=d["str"].cast(), + "bool"_a=d["bool"].cast(), + "int"_a=d["int"].cast(), + "float"_a=d["float"].cast(), + "tuple"_a=d["tuple"].cast(), + "list"_a=d["list"].cast(), + "dict"_a=d["dict"].cast(), + "set"_a=d["set"].cast(), + "memoryview"_a=d["memoryview"].cast() + ); + }); }); diff --git a/tests/test_python_types.py b/tests/test_python_types.py index b77a95b044..0b24c138f1 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -330,3 +330,29 @@ def test_exp_optional(): assert test_nullopt_exp(None) == 42 assert test_nullopt_exp(42) == 42 assert test_nullopt_exp(43) == 43 + + +def test_constructors(): + """C++ default and converting constructors are equivalent to type calls in Python""" + from pybind11_tests import (test_default_constructors, test_converting_constructors, + test_cast_functions) + + types = [str, bool, int, float, tuple, list, dict, set] + expected = {t.__name__: t() for t in types} + assert test_default_constructors() == expected + + data = { + str: 42, + bool: "Not empty", + int: "42", + float: "+1e3", + tuple: range(3), + list: range(3), + dict: [("two", 2), ("one", 1), ("three", 3)], + set: [4, 4, 5, 6, 6, 6], + memoryview: b'abc' + } + inputs = {k.__name__: v for k, v in data.items()} + expected = {k.__name__: k(v) for k, v in data.items()} + assert test_converting_constructors(inputs) == expected + assert test_cast_functions(inputs) == expected From c47339232463291fbb905d7f5b2efaa71753887a Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Fri, 28 Oct 2016 03:08:15 +0200 Subject: [PATCH 3/4] Add py::reinterpret_borrow()/steal() for low-level unchecked casts 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. `reinterpret_borrow()` and `reinterpret_steal()` serve as the low-level unsafe counterparts of `cast()`. This deprecates the `object(handle, bool)` constructor. Renamed `borrowed` parameter to `is_borrowed` to avoid shadowing warnings on MSVC. --- include/pybind11/cast.h | 61 ++++++++------- include/pybind11/eigen.h | 2 +- include/pybind11/eval.h | 22 +++--- include/pybind11/functional.h | 4 +- include/pybind11/numpy.h | 34 ++++----- include/pybind11/pybind11.h | 36 ++++----- include/pybind11/pytypes.h | 137 ++++++++++++++++++++++------------ include/pybind11/stl.h | 18 ++--- 8 files changed, 178 insertions(+), 136 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index fa9d21d7c0..d6210d3b1f 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -216,7 +216,7 @@ class type_caster_generic { auto const &type_dict = get_internals().registered_types_py; bool new_style_class = PyType_Check(tobj); if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) { - tuple parents(tobj->tp_bases, true); + auto parents = reinterpret_borrow(tobj->tp_bases); for (handle parent : parents) { bool result = load(src, convert, (PyTypeObject *) parent.ptr()); if (result) @@ -237,7 +237,7 @@ class type_caster_generic { /* Perform an implicit conversion */ if (convert) { for (auto &converter : typeinfo->implicit_conversions) { - temp = object(converter(src.ptr(), typeinfo->type), false); + temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); if (load(temp, false)) return true; } @@ -284,7 +284,7 @@ class type_caster_generic { return handle((PyObject *) it_i->second).inc_ref(); } - object inst(PyType_GenericAlloc(tinfo->type, 0), false); + auto inst = reinterpret_steal(PyType_GenericAlloc(tinfo->type, 0)); auto wrapper = (instance *) inst.ptr(); @@ -487,9 +487,9 @@ struct type_caster::value>> { #endif PyErr_Clear(); if (type_error && PyNumber_Check(src.ptr())) { - object tmp(std::is_floating_point::value - ? PyNumber_Float(src.ptr()) - : PyNumber_Long(src.ptr()), true); + auto tmp = reinterpret_borrow(std::is_floating_point::value + ? PyNumber_Float(src.ptr()) + : PyNumber_Long(src.ptr())); PyErr_Clear(); return load(tmp, false); } @@ -544,7 +544,7 @@ template <> class type_caster : public type_caster { /* Check if this is a capsule */ if (isinstance(h)) { - value = capsule(h, true); + value = reinterpret_borrow(h); return true; } @@ -596,7 +596,7 @@ template <> class type_caster { if (!src) { return false; } else if (PyUnicode_Check(load_src.ptr())) { - temp = object(PyUnicode_AsUTF8String(load_src.ptr()), false); + temp = reinterpret_steal(PyUnicode_AsUTF8String(load_src.ptr())); if (!temp) { PyErr_Clear(); return false; } // UnicodeEncodeError load_src = temp; } @@ -637,7 +637,7 @@ template <> class type_caster { if (!src) { return false; } else if (!PyUnicode_Check(load_src.ptr())) { - temp = object(PyUnicode_FromObject(load_src.ptr()), false); + temp = reinterpret_steal(PyUnicode_FromObject(load_src.ptr())); if (!temp) { PyErr_Clear(); return false; } load_src = temp; } @@ -646,10 +646,10 @@ template <> class type_caster { #if PY_MAJOR_VERSION >= 3 buffer = PyUnicode_AsWideCharString(load_src.ptr(), &length); #else - temp = object( + temp = reinterpret_steal( sizeof(wchar_t) == sizeof(short) ? PyUnicode_AsUTF16String(load_src.ptr()) - : PyUnicode_AsUTF32String(load_src.ptr()), false); + : PyUnicode_AsUTF32String(load_src.ptr())); if (temp) { int err = PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), (char **) &buffer, &length); if (err == -1) { buffer = nullptr; } // TypeError @@ -730,8 +730,8 @@ template class type_caster> { } static handle cast(const type &src, return_value_policy policy, handle parent) { - object o1 = object(make_caster::cast(src.first, policy, parent), false); - object o2 = object(make_caster::cast(src.second, policy, parent), false); + auto o1 = reinterpret_steal(make_caster::cast(src.first, policy, parent)); + auto o2 = reinterpret_steal(make_caster::cast(src.second, policy, parent)); if (!o1 || !o2) return handle(); tuple result(2); @@ -846,7 +846,7 @@ template class type_caster> { /* Implementation: Convert a C++ tuple into a Python tuple */ template static handle cast(const type &src, return_value_policy policy, handle parent, index_sequence) { std::array entries {{ - object(make_caster::cast(std::get(src), policy, parent), false)... + reinterpret_steal(make_caster::cast(std::get(src), policy, parent))... }}; for (const auto &entry: entries) if (!entry) @@ -905,7 +905,7 @@ template class type_caster_holder : public auto const &type_dict = get_internals().registered_types_py; bool new_style_class = PyType_Check(tobj); if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) { - tuple parents(tobj->tp_bases, true); + auto parents = reinterpret_borrow(tobj->tp_bases); for (handle parent : parents) { bool result = load(src, convert, (PyTypeObject *) parent.ptr()); if (result) @@ -919,7 +919,7 @@ template class type_caster_holder : public if (convert) { for (auto &converter : typeinfo->implicit_conversions) { - temp = object(converter(src.ptr(), typeinfo->type), false); + temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); if (load(temp, false)) return true; } @@ -1000,7 +1000,7 @@ struct pyobject_caster { bool load(handle src, bool /* convert */) { if (!isinstance(src)) return false; - value = type(src, true); + value = reinterpret_borrow(src); return true; } @@ -1070,6 +1070,7 @@ template make_caster load_type(const handle &handle) { NAMESPACE_END(detail) +// pytype -> C++ type template ::value, int> = 0> T cast(const handle &handle) { static_assert(!detail::cast_is_temporary_value_reference::value, @@ -1078,9 +1079,11 @@ T cast(const handle &handle) { return detail::load_type(handle).operator typename type_caster::template cast_op_type(); } +// pytype -> pytype (calls converting constructor) template ::value, int> = 0> -T cast(const handle &handle) { return {handle, true}; } +T cast(const handle &handle) { return T(reinterpret_borrow(handle)); } +// C++ type -> py::object template ::value, int> = 0> object cast(const T &value, return_value_policy policy = return_value_policy::automatic_reference, handle parent = handle()) { @@ -1088,7 +1091,7 @@ object cast(const T &value, return_value_policy policy = return_value_policy::au policy = std::is_pointer::value ? return_value_policy::take_ownership : return_value_policy::copy; else if (policy == return_value_policy::automatic_reference) policy = std::is_pointer::value ? return_value_policy::reference : return_value_policy::copy; - return object(detail::make_caster::cast(value, policy, parent), false); + return reinterpret_steal(detail::make_caster::cast(value, policy, parent)); } template T handle::cast() const { return pybind11::cast(*this); } @@ -1162,8 +1165,8 @@ template tuple make_tuple(Args&&... args_) { const size_t size = sizeof...(Args); std::array args { - { object(detail::make_caster::cast( - std::forward(args_), policy, nullptr), false)... } + { reinterpret_steal(detail::make_caster::cast( + std::forward(args_), policy, nullptr))... } }; for (auto &arg_value : args) { if (!arg_value) { @@ -1195,7 +1198,9 @@ struct arg_v : arg { template arg_v(const char *name, T &&x, const char *descr = nullptr) : arg(name), - value(detail::make_caster::cast(x, return_value_policy::automatic, handle()), false), + value(reinterpret_steal( + detail::make_caster::cast(x, return_value_policy::automatic, {}) + )), descr(descr) #if !defined(NDEBUG) , type(type_id()) @@ -1256,10 +1261,10 @@ class simple_collector { /// Call a Python function and pass the collected arguments object call(PyObject *ptr) const { - auto result = object(PyObject_CallObject(ptr, m_args.ptr()), false); + PyObject *result = PyObject_CallObject(ptr, m_args.ptr()); if (!result) throw error_already_set(); - return result; + return reinterpret_steal(result); } private: @@ -1289,16 +1294,16 @@ class unpacking_collector { /// Call a Python function and pass the collected arguments object call(PyObject *ptr) const { - auto result = object(PyObject_Call(ptr, m_args.ptr(), m_kwargs.ptr()), false); + PyObject *result = PyObject_Call(ptr, m_args.ptr(), m_kwargs.ptr()); if (!result) throw error_already_set(); - return result; + return reinterpret_steal(result); } private: template void process(list &args_list, T &&x) { - auto o = object(detail::make_caster::cast(std::forward(x), policy, nullptr), false); + auto o = reinterpret_steal(detail::make_caster::cast(std::forward(x), policy, {})); if (!o) { #if defined(NDEBUG) argument_cast_error(); @@ -1335,7 +1340,7 @@ class unpacking_collector { void process(list &/*args_list*/, detail::kwargs_proxy kp) { if (!kp) return; - for (const auto &k : dict(kp, true)) { + for (const auto &k : reinterpret_borrow(kp)) { if (m_kwargs.contains(k.first)) { #if defined(NDEBUG) multiple_values_error(); diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 3da857b712..729d0f655a 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -182,7 +182,7 @@ struct type_caster::value>> { if (!src) return false; - object obj(src, true); + auto obj = reinterpret_borrow(src); object sparse_module = module::import("scipy.sparse"); object matrix_type = sparse_module.attr( rowMajor ? "csr_matrix" : "csc_matrix"); diff --git a/include/pybind11/eval.h b/include/pybind11/eval.h index b7e7e95c90..204427d779 100644 --- a/include/pybind11/eval.h +++ b/include/pybind11/eval.h @@ -31,7 +31,7 @@ enum eval_mode { template object eval(str expr, object global = object(), object local = object()) { if (!global) { - global = object(PyEval_GetGlobals(), true); + global = reinterpret_borrow(PyEval_GetGlobals()); if (!global) global = dict(); } @@ -50,17 +50,16 @@ object eval(str expr, object global = object(), object local = object()) { default: pybind11_fail("invalid evaluation mode"); } - object result(PyRun_String(buffer.c_str(), start, global.ptr(), local.ptr()), false); - + PyObject *result = PyRun_String(buffer.c_str(), start, global.ptr(), local.ptr()); if (!result) throw error_already_set(); - return result; + return reinterpret_steal(result); } template object eval_file(str fname, object global = object(), object local = object()) { if (!global) { - global = object(PyEval_GetGlobals(), true); + global = reinterpret_borrow(PyEval_GetGlobals()); if (!global) global = dict(); } @@ -83,9 +82,9 @@ object eval_file(str fname, object global = object(), object local = object()) { FILE *f = _Py_fopen(fname.ptr(), "r"); #else /* No unicode support in open() :( */ - object fobj(PyFile_FromString( + auto fobj = reinterpret_steal(PyFile_FromString( const_cast(fname_str.c_str()), - const_cast("r")), false); + const_cast("r"))); FILE *f = nullptr; if (fobj) f = PyFile_AsFile(fobj.ptr()); @@ -96,14 +95,11 @@ object eval_file(str fname, object global = object(), object local = object()) { pybind11_fail("File \"" + fname_str + "\" could not be opened!"); } - object result(PyRun_FileEx(f, fname_str.c_str(), start, global.ptr(), - local.ptr(), closeFile), - false); - + PyObject *result = PyRun_FileEx(f, fname_str.c_str(), start, global.ptr(), + local.ptr(), closeFile); if (!result) throw error_already_set(); - - return result; + return reinterpret_steal(result); } NAMESPACE_END(pybind11) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 013b275927..dbeea5dc75 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -36,7 +36,7 @@ template struct type_caster(PyCFunction_GetSelf(src_.ptr())); auto rec = (function_record *) c; using FunctionType = Return (*) (Args...); @@ -47,7 +47,7 @@ template struct type_caster(src_); value = [src](Args... args) -> Return { gil_scoped_acquire acq; object retval(src(std::move(args)...)); diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 9fe4385029..3cbea01912 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -239,7 +239,7 @@ class dtype : public object { PyObject *ptr = nullptr; if (!detail::npy_api::get().PyArray_DescrConverter_(args.release().ptr(), &ptr) || !ptr) throw error_already_set(); - return object(ptr, false); + return reinterpret_steal(ptr); } /// Return dtype associated with a C++ type. @@ -266,7 +266,7 @@ class dtype : public object { static object _dtype_from_pep3118() { static PyObject *obj = module::import("numpy.core._internal") .attr("_dtype_from_pep3118").cast().release().ptr(); - return object(obj, true); + return reinterpret_borrow(obj); } dtype strip_padding() { @@ -279,7 +279,7 @@ class dtype : public object { std::vector field_descriptors; for (auto field : attr("fields").attr("items")()) { - auto spec = object(field, true).cast(); + auto spec = field.cast(); auto name = spec[0].cast(); auto format = spec[1].cast()[0].cast(); auto offset = spec[1].cast()[1].cast(); @@ -326,22 +326,22 @@ class array : public buffer { if (base && ptr) { if (isinstance(base)) /* Copy flags from base (except baseship bit) */ - flags = array(base, true).flags() & ~detail::npy_api::NPY_ARRAY_OWNDATA_; + flags = reinterpret_borrow(base).flags() & ~detail::npy_api::NPY_ARRAY_OWNDATA_; else /* Writable by default, easy to downgrade later on if needed */ flags = detail::npy_api::NPY_ARRAY_WRITEABLE_; } - object tmp(api.PyArray_NewFromDescr_( + auto tmp = reinterpret_steal(api.PyArray_NewFromDescr_( api.PyArray_Type_, descr.release().ptr(), (int) ndim, (Py_intptr_t *) shape.data(), - (Py_intptr_t *) strides.data(), const_cast(ptr), flags, nullptr), false); + (Py_intptr_t *) strides.data(), const_cast(ptr), flags, nullptr)); if (!tmp) pybind11_fail("NumPy: unable to create array!"); if (ptr) { if (base) { PyArray_GET_(tmp.ptr(), base) = base.inc_ref().ptr(); } else { - tmp = object(api.PyArray_NewCopy_(tmp.ptr(), -1 /* any order */), false); + tmp = reinterpret_steal(api.PyArray_NewCopy_(tmp.ptr(), -1 /* any order */)); } } m_ptr = tmp.release().ptr(); @@ -374,7 +374,7 @@ class array : public buffer { /// Array descriptor (dtype) pybind11::dtype dtype() const { - return object(PyArray_GET_(m_ptr, descr), true); + return reinterpret_borrow(PyArray_GET_(m_ptr, descr)); } /// Total number of elements @@ -399,7 +399,7 @@ class array : public buffer { /// Base object object base() const { - return object(PyArray_GET_(m_ptr, base), true); + return reinterpret_borrow(PyArray_GET_(m_ptr, base)); } /// Dimensions of the array @@ -474,14 +474,14 @@ class array : public buffer { /// Return a new view with all of the dimensions of length 1 removed array squeeze() { auto& api = detail::npy_api::get(); - return array(api.PyArray_Squeeze_(m_ptr), false); + return reinterpret_steal(api.PyArray_Squeeze_(m_ptr)); } /// Ensure that the argument is a NumPy array static array ensure(object input, int ExtraFlags = 0) { auto& api = detail::npy_api::get(); - return array(api.PyArray_FromAny_( - input.release().ptr(), nullptr, 0, 0, detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr), false); + return reinterpret_steal(api.PyArray_FromAny_( + input.release().ptr(), nullptr, 0, 0, detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr)); } protected: @@ -542,7 +542,7 @@ template class array_t : public public: array_t() : array() { } - array_t(handle h, bool borrowed) : array(h, borrowed) { m_ptr = ensure_(m_ptr); } + array_t(handle h, bool is_borrowed) : array(h, is_borrowed) { m_ptr = ensure_(m_ptr); } array_t(const object &o) : array(o) { m_ptr = ensure_(m_ptr); } @@ -668,7 +668,7 @@ template struct npy_format_descriptor::value ? 1 : 0)] }; static pybind11::dtype dtype() { if (auto ptr = npy_api::get().PyArray_DescrFromType_(value)) - return object(ptr, true); + return reinterpret_borrow(ptr); pybind11_fail("Unsupported buffer format!"); } template ::value, int> = 0> @@ -683,7 +683,7 @@ template constexpr const int npy_format_descriptor< enum { value = npy_api::NumPyName }; \ static pybind11::dtype dtype() { \ if (auto ptr = npy_api::get().PyArray_DescrFromType_(value)) \ - return object(ptr, true); \ + return reinterpret_borrow(ptr); \ pybind11_fail("Unsupported buffer format!"); \ } \ static PYBIND11_DESCR name() { return _(Name); } } @@ -778,7 +778,7 @@ struct npy_format_descriptor::value>> { static PYBIND11_DESCR name() { return _("struct"); } static pybind11::dtype dtype() { - return object(dtype_ptr(), true); + return reinterpret_borrow(dtype_ptr()); } static std::string format() { @@ -801,7 +801,7 @@ struct npy_format_descriptor::value>> { auto& api = npy_api::get(); if (!PyObject_TypeCheck(obj, api.PyVoidArrType_Type_)) return false; - if (auto descr = object(api.PyArray_DescrFromScalar_(obj), false)) { + if (auto descr = reinterpret_steal(api.PyArray_DescrFromScalar_(obj))) { if (api.PyArray_EquivTypes_(dtype_ptr(), descr.ptr())) { value = ((PyVoidScalarObject_Proxy *) obj)->obval; return true; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ca116eb4ce..6804fa91a3 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -256,7 +256,7 @@ class cpp_function : public function { detail::function_record *chain = nullptr, *chain_start = rec; if (rec->sibling) { if (PyCFunction_Check(rec->sibling.ptr())) { - capsule rec_capsule(PyCFunction_GetSelf(rec->sibling.ptr()), true); + auto rec_capsule = reinterpret_borrow(PyCFunction_GetSelf(rec->sibling.ptr())); chain = (detail::function_record *) rec_capsule; /* Never append a method to an overload chain of a parent class; instead, hide the parent's overloads in this case */ @@ -382,7 +382,7 @@ class cpp_function : public function { result = PYBIND11_TRY_NEXT_OVERLOAD; try { for (; it != nullptr; it = it->next) { - tuple args_(args, true); + auto args_ = reinterpret_borrow(args); size_t kwargs_consumed = 0; /* For each overload: @@ -502,7 +502,7 @@ class cpp_function : public function { msg += "\n"; } msg += "\nInvoked with: "; - tuple args_(args, true); + auto args_ = reinterpret_borrow(args); for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) { msg += static_cast(pybind11::str(args_[ti])); if ((ti + 1) != args_.size() ) @@ -565,7 +565,7 @@ class module : public object { module def_submodule(const char *name, const char *doc = nullptr) { std::string full_name = std::string(PyModule_GetName(m_ptr)) + std::string(".") + std::string(name); - module result(PyImport_AddModule(full_name.c_str()), true); + auto result = reinterpret_borrow(PyImport_AddModule(full_name.c_str())); if (doc && options::show_user_defined_docstrings()) result.attr("__doc__") = pybind11::str(doc); attr(name) = result; @@ -576,7 +576,7 @@ class module : public object { PyObject *obj = PyImport_ImportModule(name); if (!obj) throw import_error("Module \"" + std::string(name) + "\" not found!"); - return module(obj, false); + return reinterpret_steal(obj); } // Adds an object to the module using the given name. Throws if an object with the given name @@ -636,7 +636,7 @@ class generic_type : public object { pybind11_fail("generic_type: type \"" + std::string(rec->name) + "\" is already registered!"); - object name(PYBIND11_FROM_STRING(rec->name), false); + auto name = reinterpret_steal(PYBIND11_FROM_STRING(rec->name)); object scope_module; if (rec->scope) { if (hasattr(rec->scope, rec->name)) @@ -657,8 +657,8 @@ class generic_type : public object { scope_qualname = rec->scope.attr("__qualname__"); object ht_qualname; if (scope_qualname) { - ht_qualname = object(PyUnicode_FromFormat( - "%U.%U", scope_qualname.ptr(), name.ptr()), false); + ht_qualname = reinterpret_steal(PyUnicode_FromFormat( + "%U.%U", scope_qualname.ptr(), name.ptr())); } else { ht_qualname = name; } @@ -684,7 +684,7 @@ class generic_type : public object { garbage collector (the GC will call type_traverse(), which will in turn find the newly constructed type in an invalid state) */ - object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); + auto type_holder = reinterpret_steal(PyType_Type.tp_alloc(&PyType_Type, 0)); auto type = (PyHeapTypeObject*) type_holder.ptr(); if (!type_holder || !name) @@ -768,7 +768,7 @@ class generic_type : public object { /// Helper function which tags all parents of a type using mult. inheritance void mark_parents_nonsimple(PyTypeObject *value) { - tuple t(value->tp_bases, true); + auto t = reinterpret_borrow(value->tp_bases); for (handle h : t) { auto tinfo2 = get_type_info((PyTypeObject *) h.ptr()); if (tinfo2) @@ -785,10 +785,10 @@ class generic_type : public object { if (ob_type == &PyType_Type) { std::string name_ = std::string(ht_type.tp_name) + "__Meta"; #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 - object ht_qualname(PyUnicode_FromFormat("%U__Meta", attr("__qualname__").ptr()), false); + auto ht_qualname = reinterpret_steal(PyUnicode_FromFormat("%U__Meta", attr("__qualname__").ptr())); #endif - object name(PYBIND11_FROM_STRING(name_.c_str()), false); - object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); + auto name = reinterpret_steal(PYBIND11_FROM_STRING(name_.c_str())); + auto type_holder = reinterpret_steal(PyType_Type.tp_alloc(&PyType_Type, 0)); if (!type_holder || !name) pybind11_fail("generic_type::metaclass(): unable to create type object!"); @@ -936,7 +936,7 @@ class class_ : public detail::generic_type { static_assert(detail::all_of_t::value, "Unknown/invalid class_ template parameters provided"); - PYBIND11_OBJECT(class_, detail::generic_type, PyType_Check) + PYBIND11_OBJECT(class_, generic_type, PyType_Check) template class_(handle scope, const char *name, const Extra &... extra) { @@ -1117,9 +1117,9 @@ class class_ : public detail::generic_type { } } pybind11::str doc_obj = pybind11::str((rec_fget->doc && pybind11::options::show_user_defined_docstrings()) ? rec_fget->doc : ""); - object property( + const auto property = reinterpret_steal( PyObject_CallFunctionObjArgs((PyObject *) &PyProperty_Type, fget.ptr() ? fget.ptr() : Py_None, - fset.ptr() ? fset.ptr() : Py_None, Py_None, doc_obj.ptr(), nullptr), false); + fset.ptr() ? fset.ptr() : Py_None, Py_None, doc_obj.ptr(), nullptr)); if (rec_fget->class_) attr(name) = property; else @@ -1178,8 +1178,8 @@ class class_ : public detail::generic_type { static detail::function_record *get_function_record(handle h) { h = detail::get_function(h); - return h ? (detail::function_record *) capsule( - PyCFunction_GetSelf(h.ptr()), true) : nullptr; + return h ? (detail::function_record *) reinterpret_borrow(PyCFunction_GetSelf(h.ptr())) + : nullptr; } }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 9257643e8b..d078d58e0b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -102,9 +102,9 @@ class handle : public detail::object_api { class object : public handle { public: object() = default; + PYBIND11_DEPRECATED("Use reinterpret_borrow() or reinterpret_steal()") + object(handle h, bool is_borrowed) : handle(h) { if (is_borrowed) inc_ref(); } object(const object &o) : handle(o) { inc_ref(); } - object(const handle &h, bool borrowed) : handle(h) { if (borrowed) inc_ref(); } - object(PyObject *ptr, bool borrowed) : handle(ptr) { if (borrowed) inc_ref(); } object(object &&other) noexcept { m_ptr = other.m_ptr; other.m_ptr = nullptr; } ~object() { dec_ref(); } @@ -135,8 +135,26 @@ class object : public handle { template T cast() const &; // Calling on an object rvalue does a move, if needed and/or possible template T cast() &&; + +protected: + // Tags for choosing constructors from raw PyObject * + struct borrowed_t { }; static constexpr borrowed_t borrowed{}; + struct stolen_t { }; static constexpr stolen_t stolen{}; + + template friend T reinterpret_borrow(handle); + template friend T reinterpret_steal(handle); + +public: + // Only accessible from derived classes and the reinterpret_* functions + object(handle h, borrowed_t) : handle(h) { inc_ref(); } + object(handle h, stolen_t) : handle(h) { } }; +/** The following functions don't do any kind of conversion, they simply declare + that a PyObject is a certain type and borrow or steal the reference. */ +template T reinterpret_borrow(handle h) { return {h, object::borrowed}; } +template T reinterpret_steal(handle h) { return {h, object::stolen}; } + /// Check if `obj` is an instance of type `T` template ::value, int> = 0> bool isinstance(handle obj) { return T::_check(obj); } @@ -158,30 +176,30 @@ inline bool hasattr(handle obj, const char *name) { inline object getattr(handle obj, handle name) { PyObject *result = PyObject_GetAttr(obj.ptr(), name.ptr()); if (!result) { throw error_already_set(); } - return {result, false}; + return reinterpret_steal(result); } inline object getattr(handle obj, const char *name) { PyObject *result = PyObject_GetAttrString(obj.ptr(), name); if (!result) { throw error_already_set(); } - return {result, false}; + return reinterpret_steal(result); } inline object getattr(handle obj, handle name, handle default_) { if (PyObject *result = PyObject_GetAttr(obj.ptr(), name.ptr())) { - return {result, false}; + return reinterpret_steal(result); } else { PyErr_Clear(); - return {default_, true}; + return reinterpret_borrow(default_); } } inline object getattr(handle obj, const char *name, handle default_) { if (PyObject *result = PyObject_GetAttrString(obj.ptr(), name)) { - return {result, false}; + return reinterpret_steal(result); } else { PyErr_Clear(); - return {default_, true}; + return reinterpret_borrow(default_); } } @@ -218,7 +236,7 @@ class accessor : public object_api> { void operator=(const object &o) && { std::move(*this).operator=(handle(o)); } void operator=(const object &o) & { operator=(handle(o)); } void operator=(handle value) && { Policy::set(obj, key, value); } - void operator=(handle value) & { get_cache() = object(value, true); } + void operator=(handle value) & { get_cache() = reinterpret_borrow(value); } template PYBIND11_DEPRECATED("Use of obj.attr(...) as bool is deprecated in favor of pybind11::hasattr(obj, ...)") @@ -267,7 +285,7 @@ struct generic_item { static object get(handle obj, handle key) { PyObject *result = PyObject_GetItem(obj.ptr(), key.ptr()); if (!result) { throw error_already_set(); } - return {result, false}; + return reinterpret_steal(result); } static void set(handle obj, handle key, handle val) { @@ -281,7 +299,7 @@ struct sequence_item { static object get(handle obj, size_t index) { PyObject *result = PySequence_GetItem(obj.ptr(), static_cast(index)); if (!result) { throw error_already_set(); } - return {result, true}; + return reinterpret_borrow(result); } static void set(handle obj, size_t index, handle val) { @@ -298,7 +316,7 @@ struct list_item { static object get(handle obj, size_t index) { PyObject *result = PyList_GetItem(obj.ptr(), static_cast(index)); if (!result) { throw error_already_set(); } - return {result, true}; + return reinterpret_borrow(result); } static void set(handle obj, size_t index, handle val) { @@ -315,7 +333,7 @@ struct tuple_item { static object get(handle obj, size_t index) { PyObject *result = PyTuple_GetItem(obj.ptr(), static_cast(index)); if (!result) { throw error_already_set(); } - return {result, true}; + return reinterpret_borrow(result); } static void set(handle obj, size_t index, handle val) { @@ -390,23 +408,30 @@ class unpacking_collector; NAMESPACE_END(detail) +// TODO: After the deprecated constructors are removed, this macro can be simplified by +// inheriting ctors: `using Parent::Parent`. It's not an option right now because +// the `using` statement triggers the parent deprecation warning even if the ctor +// isn't even used. #define PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ public: \ + PYBIND11_DEPRECATED("Use reinterpret_borrow<"#Name">() or reinterpret_steal<"#Name">()") \ + Name(handle h, bool is_borrowed) : Parent(is_borrowed ? Parent(h, borrowed) : Parent(h, stolen)) { } \ + Name(handle h, borrowed_t) : Parent(h, borrowed) { } \ + Name(handle h, stolen_t) : Parent(h, stolen) { } \ PYBIND11_DEPRECATED("Use py::isinstance(obj) instead") \ bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \ static bool _check(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } #define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ - Name(handle h, bool borrowed) : Name(object(h, borrowed)) { } \ /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ - Name(const object &o) : Parent(ConvertFun(o.ptr()), false) { if (!m_ptr) throw error_already_set(); } + Name(const object &o) : Parent(ConvertFun(o.ptr()), stolen) { if (!m_ptr) throw error_already_set(); } #define PYBIND11_OBJECT(Name, Parent, CheckFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ - Name(handle h, bool borrowed) : Parent(h, borrowed) { } \ /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ - Name(const object &o) : Parent(o) { } + Name(const object &o) : Parent(o) { } \ + Name(object &&o) : Parent(std::move(o)) { } #define PYBIND11_OBJECT_DEFAULT(Name, Parent, CheckFun) \ PYBIND11_OBJECT(Name, Parent, CheckFun) \ @@ -448,7 +473,7 @@ class iterator : public object { } private: - void advance() { value = object(PyIter_Next(m_ptr), false); } + void advance() { value = reinterpret_steal(PyIter_Next(m_ptr)); } private: object value = {}; @@ -467,13 +492,13 @@ class str : public object { PYBIND11_OBJECT_CVT(str, object, detail::PyUnicode_Check_Permissive, raw_str) str(const char *c, size_t n) - : object(PyUnicode_FromStringAndSize(c, (ssize_t) n), false) { + : object(PyUnicode_FromStringAndSize(c, (ssize_t) n), stolen) { if (!m_ptr) pybind11_fail("Could not allocate string object!"); } // 'explicit' is explicitly omitted from the following constructors to allow implicit conversion to py::str from C++ string-like objects str(const char *c = "") - : object(PyUnicode_FromString(c), false) { + : object(PyUnicode_FromString(c), stolen) { if (!m_ptr) pybind11_fail("Could not allocate string object!"); } @@ -481,12 +506,12 @@ class str : public object { explicit str(const bytes &b); - explicit str(handle h) : object(raw_str(h.ptr()), false) { } + explicit str(handle h) : object(raw_str(h.ptr()), stolen) { } operator std::string() const { object temp = *this; if (PyUnicode_Check(m_ptr)) { - temp = object(PyUnicode_AsUTF8String(m_ptr), false); + temp = reinterpret_steal(PyUnicode_AsUTF8String(m_ptr)); if (!temp) pybind11_fail("Unable to extract string contents! (encoding issue)"); } @@ -526,12 +551,12 @@ class bytes : public object { // Allow implicit conversion: bytes(const char *c = "") - : object(PYBIND11_BYTES_FROM_STRING(c), false) { + : object(PYBIND11_BYTES_FROM_STRING(c), stolen) { if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); } bytes(const char *c, size_t n) - : object(PYBIND11_BYTES_FROM_STRING_AND_SIZE(c, (ssize_t) n), false) { + : object(PYBIND11_BYTES_FROM_STRING_AND_SIZE(c, (ssize_t) n), stolen) { if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); } @@ -552,7 +577,7 @@ class bytes : public object { inline bytes::bytes(const pybind11::str &s) { object temp = s; if (PyUnicode_Check(s.ptr())) { - temp = object(PyUnicode_AsUTF8String(s.ptr()), false); + temp = reinterpret_steal(PyUnicode_AsUTF8String(s.ptr())); if (!temp) pybind11_fail("Unable to extract string contents! (encoding issue)"); } @@ -560,7 +585,7 @@ inline bytes::bytes(const pybind11::str &s) { ssize_t length; if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length)) pybind11_fail("Unable to extract string contents! (invalid type)"); - auto obj = object(PYBIND11_BYTES_FROM_STRING_AND_SIZE(buffer, length), false); + auto obj = reinterpret_steal(PYBIND11_BYTES_FROM_STRING_AND_SIZE(buffer, length)); if (!obj) pybind11_fail("Could not allocate bytes object!"); m_ptr = obj.release().ptr(); @@ -571,7 +596,7 @@ inline str::str(const bytes& b) { ssize_t length; if (PYBIND11_BYTES_AS_STRING_AND_SIZE(b.ptr(), &buffer, &length)) pybind11_fail("Unable to extract bytes contents!"); - auto obj = object(PyUnicode_FromStringAndSize(buffer, (ssize_t) length), false); + auto obj = reinterpret_steal(PyUnicode_FromStringAndSize(buffer, (ssize_t) length)); if (!obj) pybind11_fail("Could not allocate string object!"); m_ptr = obj.release().ptr(); @@ -580,15 +605,15 @@ inline str::str(const bytes& b) { class none : public object { public: PYBIND11_OBJECT(none, object, detail::PyNone_Check) - none() : object(Py_None, true) { } + none() : object(Py_None, borrowed) { } }; class bool_ : public object { public: PYBIND11_OBJECT_CVT(bool_, object, PyBool_Check, raw_bool) - bool_() : object(Py_False, true) { } + bool_() : object(Py_False, borrowed) { } // Allow implicit conversion from and to `bool`: - bool_(bool value) : object(value ? Py_True : Py_False, true) { } + bool_(bool value) : object(value ? Py_True : Py_False, borrowed) { } operator bool() const { return m_ptr && PyLong_AsLong(m_ptr) != 0; } private: @@ -603,7 +628,7 @@ class bool_ : public object { class int_ : public object { public: PYBIND11_OBJECT_CVT(int_, object, PYBIND11_LONG_CHECK, PyNumber_Long) - int_() : object(PyLong_FromLong(0), false) { } + int_() : object(PyLong_FromLong(0), stolen) { } // Allow implicit conversion from C++ integral types: template ::value, int> = 0> @@ -643,10 +668,10 @@ class float_ : public object { public: PYBIND11_OBJECT_CVT(float_, object, PyFloat_Check, PyNumber_Float) // Allow implicit conversion from float/double: - float_(float value) : object(PyFloat_FromDouble((double) value), false) { + float_(float value) : object(PyFloat_FromDouble((double) value), stolen) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } - float_(double value = .0) : object(PyFloat_FromDouble((double) value), false) { + float_(double value = .0) : object(PyFloat_FromDouble((double) value), stolen) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } operator float() const { return (float) PyFloat_AsDouble(m_ptr); } @@ -656,7 +681,8 @@ class float_ : public object { class weakref : public object { public: PYBIND11_OBJECT_DEFAULT(weakref, object, PyWeakref_Check) - explicit weakref(handle obj, handle callback = handle()) : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), false) { + explicit weakref(handle obj, handle callback = {}) + : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), stolen) { if (!m_ptr) pybind11_fail("Could not allocate weak reference!"); } }; @@ -681,9 +707,10 @@ class slice : public object { class capsule : public object { public: PYBIND11_OBJECT_DEFAULT(capsule, object, PyCapsule_CheckExact) - capsule(PyObject *obj, bool borrowed) : object(obj, borrowed) { } + PYBIND11_DEPRECATED("Use reinterpret_borrow() or reinterpret_steal()") + capsule(PyObject *ptr, bool is_borrowed) : object(is_borrowed ? object(ptr, borrowed) : object(ptr, stolen)) { } explicit capsule(const void *value, void (*destruct)(PyObject *) = nullptr) - : object(PyCapsule_New(const_cast(value), nullptr, destruct), false) { + : object(PyCapsule_New(const_cast(value), nullptr, destruct), stolen) { if (!m_ptr) pybind11_fail("Could not allocate capsule object!"); } template operator T *() const { @@ -696,7 +723,7 @@ class capsule : public object { class tuple : public object { public: PYBIND11_OBJECT_CVT(tuple, object, PyTuple_Check, PySequence_Tuple) - explicit tuple(size_t size = 0) : object(PyTuple_New((ssize_t) size), false) { + explicit tuple(size_t size = 0) : object(PyTuple_New((ssize_t) size), stolen) { if (!m_ptr) pybind11_fail("Could not allocate tuple object!"); } size_t size() const { return (size_t) PyTuple_Size(m_ptr); } @@ -706,7 +733,7 @@ class tuple : public object { class dict : public object { public: PYBIND11_OBJECT_CVT(dict, object, PyDict_Check, raw_dict) - dict() : object(PyDict_New(), false) { + dict() : object(PyDict_New(), stolen) { if (!m_ptr) pybind11_fail("Could not allocate dict object!"); } template (str_value); } NAMESPACE_BEGIN(detail) -template iterator object_api::begin() const { return {PyObject_GetIter(derived().ptr()), false}; } -template iterator object_api::end() const { return {nullptr, false}; } -template item_accessor object_api::operator[](handle key) const { return {derived(), object(key, true)}; } -template item_accessor object_api::operator[](const char *key) const { return {derived(), pybind11::str(key)}; } -template obj_attr_accessor object_api::attr(handle key) const { return {derived(), object(key, true)}; } -template str_attr_accessor object_api::attr(const char *key) const { return {derived(), key}; } -template args_proxy object_api::operator*() const { return args_proxy(derived().ptr()); } +template iterator object_api::begin() const { + return reinterpret_steal(PyObject_GetIter(derived().ptr())); +} +template iterator object_api::end() const { + return {}; +} +template item_accessor object_api::operator[](handle key) const { + return {derived(), reinterpret_borrow(key)}; +} +template item_accessor object_api::operator[](const char *key) const { + return {derived(), pybind11::str(key)}; +} +template obj_attr_accessor object_api::attr(handle key) const { + return {derived(), reinterpret_borrow(key)}; +} +template str_attr_accessor object_api::attr(const char *key) const { + return {derived(), key}; +} +template args_proxy object_api::operator*() const { + return args_proxy(derived().ptr()); +} template template bool object_api::contains(T &&key) const { return attr("__contains__")(std::forward(key)).template cast(); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index fc058ef25a..c1ed0f9796 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -47,7 +47,7 @@ template struct set_caster { bool load(handle src, bool convert) { if (!isinstance(src)) return false; - pybind11::set s(src, true); + auto s = reinterpret_borrow(src); value.clear(); key_conv conv; for (auto entry : s) { @@ -61,7 +61,7 @@ template struct set_caster { static handle cast(const type &src, return_value_policy policy, handle parent) { pybind11::set s; for (auto const &value: src) { - object value_ = object(key_conv::cast(value, policy, parent), false); + auto value_ = reinterpret_steal(key_conv::cast(value, policy, parent)); if (!value_ || !s.add(value_)) return handle(); } @@ -79,7 +79,7 @@ template struct map_caster { bool load(handle src, bool convert) { if (!isinstance(src)) return false; - dict d(src, true); + auto d = reinterpret_borrow(src); key_conv kconv; value_conv vconv; value.clear(); @@ -95,8 +95,8 @@ template struct map_caster { static handle cast(const type &src, return_value_policy policy, handle parent) { dict d; for (auto const &kv: src) { - object key = object(key_conv::cast(kv.first, policy, parent), false); - object value = object(value_conv::cast(kv.second, policy, parent), false); + auto key = reinterpret_steal(key_conv::cast(kv.first, policy, parent)); + auto value = reinterpret_steal(value_conv::cast(kv.second, policy, parent)); if (!key || !value) return handle(); d[key] = value; @@ -114,7 +114,7 @@ template struct list_caster { bool load(handle src, bool convert) { if (!isinstance(src)) return false; - sequence s(src, true); + auto s = reinterpret_borrow(src); value_conv conv; value.clear(); reserve_maybe(s, &value); @@ -135,7 +135,7 @@ template struct list_caster { list l(src.size()); size_t index = 0; for (auto const &value: src) { - object value_ = object(value_conv::cast(value, policy, parent), false); + auto value_ = reinterpret_steal(value_conv::cast(value, policy, parent)); if (!value_) return handle(); PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference @@ -159,7 +159,7 @@ template struct type_caster> bool load(handle src, bool convert) { if (!isinstance(src)) return false; - list l(src, true); + auto l = reinterpret_borrow(src); if (l.size() != Size) return false; value_conv conv; @@ -176,7 +176,7 @@ template struct type_caster> list l(Size); size_t index = 0; for (auto const &value: src) { - object value_ = object(value_conv::cast(value, policy, parent), false); + auto value_ = reinterpret_steal(value_conv::cast(value, policy, parent)); if (!value_) return handle(); PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference From ee946b8b34d0ae20b586bc4d4e017bef342e3289 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Wed, 16 Nov 2016 01:35:22 +0100 Subject: [PATCH 4/4] Improve consistency of array and array_t with regard to other pytypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * `array_t(const object &)` now throws on error * `array_t::ensure()` is intended for casters —- old constructor is deprecated * `array` and `array_t` get default constructors (empty array) * `array` gets a converting constructor * `py::isinstance>()` checks the type (but not flags) There is only one special thing which must remain: `array_t` gets its own `type_caster` specialization which uses `ensure` instead of a simple check. --- include/pybind11/eigen.h | 2 +- include/pybind11/numpy.h | 67 ++++++++++++++++++++++++++++---------- tests/test_numpy_array.cpp | 24 ++++++++++++++ tests/test_numpy_array.py | 27 +++++++++++++++ 4 files changed, 101 insertions(+), 19 deletions(-) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 729d0f655a..0a1208e16d 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -54,7 +54,7 @@ struct type_caster::value && !is_eigen_re static constexpr bool isVector = Type::IsVectorAtCompileTime; bool load(handle src, bool) { - array_t buf(src, true); + auto buf = array_t::ensure(src); if (!buf) return false; diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 3cbea01912..77006c8877 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -305,7 +305,7 @@ class dtype : public object { class array : public buffer { public: - PYBIND11_OBJECT_DEFAULT(array, buffer, detail::npy_api::get().PyArray_Check_) + PYBIND11_OBJECT_CVT(array, buffer, detail::npy_api::get().PyArray_Check_, raw_array) enum { c_style = detail::npy_api::NPY_C_CONTIGUOUS_, @@ -313,6 +313,8 @@ class array : public buffer { forcecast = detail::npy_api::NPY_ARRAY_FORCECAST_ }; + array() : array(0, static_cast(nullptr)) {} + array(const pybind11::dtype &dt, const std::vector &shape, const std::vector &strides, const void *ptr = nullptr, handle base = handle()) { @@ -478,10 +480,12 @@ class array : public buffer { } /// Ensure that the argument is a NumPy array - static array ensure(object input, int ExtraFlags = 0) { - auto& api = detail::npy_api::get(); - return reinterpret_steal(api.PyArray_FromAny_( - input.release().ptr(), nullptr, 0, 0, detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr)); + /// In case of an error, nullptr is returned and the Python error is cleared. + static array ensure(handle h, int ExtraFlags = 0) { + auto result = reinterpret_steal(raw_array(h.ptr(), ExtraFlags)); + if (!result) + PyErr_Clear(); + return result; } protected: @@ -520,8 +524,6 @@ class array : public buffer { return strides; } -protected: - template void check_dimensions(Ix... index) const { check_dimensions_impl(size_t(0), shape(), size_t(index)...); } @@ -536,15 +538,31 @@ class array : public buffer { } check_dimensions_impl(axis + 1, shape + 1, index...); } + + /// Create array from any object -- always returns a new reference + static PyObject *raw_array(PyObject *ptr, int ExtraFlags = 0) { + if (ptr == nullptr) + return nullptr; + return detail::npy_api::get().PyArray_FromAny_( + ptr, nullptr, 0, 0, detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr); + } }; template class array_t : public array { public: - array_t() : array() { } + array_t() : array(0, static_cast(nullptr)) {} + array_t(handle h, borrowed_t) : array(h, borrowed) { } + array_t(handle h, stolen_t) : array(h, stolen) { } - array_t(handle h, bool is_borrowed) : array(h, is_borrowed) { m_ptr = ensure_(m_ptr); } + PYBIND11_DEPRECATED("Use array_t::ensure() instead") + array_t(handle h, bool is_borrowed) : array(raw_array_t(h.ptr()), stolen) { + if (!m_ptr) PyErr_Clear(); + if (!is_borrowed) Py_XDECREF(h.ptr()); + } - array_t(const object &o) : array(o) { m_ptr = ensure_(m_ptr); } + array_t(const object &o) : array(raw_array_t(o.ptr()), stolen) { + if (!m_ptr) throw error_already_set(); + } explicit array_t(const buffer_info& info) : array(info) { } @@ -590,17 +608,30 @@ template class array_t : public return *(static_cast(array::mutable_data()) + byte_offset(size_t(index)...) / itemsize()); } - static PyObject *ensure_(PyObject *ptr) { - if (ptr == nullptr) - return nullptr; - auto& api = detail::npy_api::get(); - PyObject *result = api.PyArray_FromAny_(ptr, pybind11::dtype::of().release().ptr(), 0, 0, - detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr); + /// Ensure that the argument is a NumPy array of the correct dtype. + /// In case of an error, nullptr is returned and the Python error is cleared. + static array_t ensure(handle h) { + auto result = reinterpret_steal(raw_array_t(h.ptr())); if (!result) PyErr_Clear(); - Py_DECREF(ptr); return result; } + + static bool _check(handle h) { + const auto &api = detail::npy_api::get(); + return api.PyArray_Check_(h.ptr()) + && api.PyArray_EquivTypes_(PyArray_GET_(h.ptr(), descr), dtype::of().ptr()); + } + +protected: + /// Create array from any object -- always returns a new reference + static PyObject *raw_array_t(PyObject *ptr) { + if (ptr == nullptr) + return nullptr; + return detail::npy_api::get().PyArray_FromAny_( + ptr, dtype::of().release().ptr(), 0, 0, + detail::npy_api::NPY_ENSURE_ARRAY_ | ExtraFlags, nullptr); + } }; template @@ -631,7 +662,7 @@ struct pyobject_caster> { using type = array_t; bool load(handle src, bool /* convert */) { - value = type(src, true); + value = type::ensure(src); return static_cast(value); } diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index df6377eb73..14c4c29995 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -126,4 +126,28 @@ test_initializer numpy_array([](py::module &m) { ); sm.def("function_taking_uint64", [](uint64_t) { }); + + sm.def("isinstance_untyped", [](py::object yes, py::object no) { + return py::isinstance(yes) && !py::isinstance(no); + }); + + sm.def("isinstance_typed", [](py::object o) { + return py::isinstance>(o) && !py::isinstance>(o); + }); + + sm.def("default_constructors", []() { + return py::dict( + "array"_a=py::array(), + "array_t"_a=py::array_t(), + "array_t"_a=py::array_t() + ); + }); + + sm.def("converting_constructors", [](py::object o) { + return py::dict( + "array"_a=py::array(o), + "array_t"_a=py::array_t(o), + "array_t"_a=py::array_t(o) + ); + }); }); diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 40682efc23..cec0054193 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -245,3 +245,30 @@ def test_cast_numpy_int64_to_uint64(): from pybind11_tests.array import function_taking_uint64 function_taking_uint64(123) function_taking_uint64(np.uint64(123)) + + +@pytest.requires_numpy +def test_isinstance(): + from pybind11_tests.array import isinstance_untyped, isinstance_typed + + assert isinstance_untyped(np.array([1, 2, 3]), "not an array") + assert isinstance_typed(np.array([1.0, 2.0, 3.0])) + + +@pytest.requires_numpy +def test_constructors(): + from pybind11_tests.array import default_constructors, converting_constructors + + defaults = default_constructors() + for a in defaults.values(): + assert a.size == 0 + assert defaults["array"].dtype == np.array([]).dtype + assert defaults["array_t"].dtype == np.int32 + assert defaults["array_t"].dtype == np.float64 + + results = converting_constructors([1, 2, 3]) + for a in results.values(): + np.testing.assert_array_equal(a, [1, 2, 3]) + assert results["array"].dtype == np.int_ + assert results["array_t"].dtype == np.int32 + assert results["array_t"].dtype == np.float64