From 37e22e436e012c18acedcce7229e9515cef0db45 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 8 Sep 2016 16:36:01 +0200 Subject: [PATCH 1/5] Move common object functions into object_api mixin --- include/pybind11/attr.h | 2 +- include/pybind11/cast.h | 16 +++-- include/pybind11/pytypes.h | 129 +++++++++++++++++++++++-------------- 3 files changed, 89 insertions(+), 58 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index ba297355d..f37e8627e 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -276,7 +276,7 @@ template <> struct process_attribute : process_attribute_default { /// Process a parent class attribute template -struct process_attribute::value>> : process_attribute_default { +struct process_attribute::value>> : process_attribute_default { static void init(const handle &h, type_record *r) { r->bases.append(h); } }; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 780a02efc..1d9740d38 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -908,7 +908,7 @@ 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>> { +struct type_caster::value>> { public: template ::value, int> = 0> bool load(handle src, bool /* convert */) { value = type(src); return value.check(); } @@ -1296,18 +1296,20 @@ unpacking_collector collect_arguments(Args &&...args) { return { std::forward(args)... }; } -NAMESPACE_END(detail) - +template template -object handle::operator()(Args &&...args) const { - return detail::collect_arguments(std::forward(args)...).call(m_ptr); +object object_api::operator()(Args &&...args) const { + return detail::collect_arguments(std::forward(args)...).call(derived().ptr()); } -template object handle::call(Args &&... args) const { +template +template +object object_api::call(Args &&...args) const { return operator()(std::forward(args)...); } +NAMESPACE_END(detail) + #define PYBIND11_MAKE_OPAQUE(Type) \ namespace pybind11 { namespace detail { \ template<> class type_caster : public type_caster_base { }; \ diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 8681bbb12..872c90cc9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -16,50 +16,72 @@ NAMESPACE_BEGIN(pybind11) /* A few forward declarations */ -class object; class str; class iterator; +class handle; class object; +class str; class iterator; struct arg; struct arg_v; -namespace detail { class accessor; class args_proxy; class kwargs_proxy; } + +NAMESPACE_BEGIN(detail) +class accessor; class args_proxy; + +/// Tag and check to identify a class which implements the Python object API +class pyobject_tag { }; +template using is_pyobject = std::is_base_of; + +/// Mixin which adds common functions to handle, object and various accessors. +/// The only requirement for `Derived` is to implement `PyObject *Derived::ptr() const`. +template +class object_api : public pyobject_tag { + const Derived &derived() const { return static_cast(*this); } + +public: + iterator begin() const; + iterator end() const; + accessor operator[](handle key) const; + accessor operator[](const char *key) const; + accessor attr(handle key) const; + accessor attr(const char *key) const; + args_proxy operator*() const; + + template + object operator()(Args &&...args) const; + template + PYBIND11_DEPRECATED("call(...) was deprecated in favor of operator()(...)") + object call(Args&&... args) const; + + bool is_none() const { return derived().ptr() == Py_None; } + pybind11::str str() const; + pybind11::str repr() const; + + int ref_count() const { return static_cast(Py_REFCNT(derived().ptr())); } + handle get_type() const; +}; + +NAMESPACE_END(detail) /// Holds a reference to a Python object (no reference counting) -class handle { +class handle : public detail::object_api { public: - handle() : m_ptr(nullptr) { } - handle(const handle &other) : m_ptr(other.m_ptr) { } + handle() = default; handle(PyObject *ptr) : m_ptr(ptr) { } + PyObject *ptr() const { return m_ptr; } PyObject *&ptr() { return m_ptr; } const handle& inc_ref() const { Py_XINCREF(m_ptr); return *this; } const handle& dec_ref() const { Py_XDECREF(m_ptr); return *this; } - int ref_count() const { return (int) Py_REFCNT(m_ptr); } - handle get_type() const { return handle((PyObject *) Py_TYPE(m_ptr)); } - inline iterator begin() const; - inline iterator end() const; - inline detail::accessor operator[](handle key) const; - inline detail::accessor operator[](const char *key) const; - inline detail::accessor attr(handle key) const; - inline detail::accessor attr(const char *key) const; - inline pybind11::str str() const; - inline pybind11::str repr() const; - bool is_none() const { return m_ptr == Py_None; } + template T cast() const; - template - PYBIND11_DEPRECATED("call(...) was deprecated in favor of operator()(...)") - object call(Args&&... args) const; - template - object operator()(Args&&... args) const; 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; } bool check() const { return m_ptr != nullptr; } - inline detail::args_proxy operator*() const; protected: - PyObject *m_ptr; + PyObject *m_ptr = nullptr; }; /// Holds a reference to a Python object (with reference counting) class object : public handle { public: - object() { } + object() = default; 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(); } @@ -347,14 +369,6 @@ public: PYBIND11_OBJECT_DEFAULT(iterable, object, detail::PyIterable_Check) }; -inline detail::accessor handle::operator[](handle key) const { return detail::accessor(*this, key, false); } -inline detail::accessor handle::operator[](const char *key) const { return detail::accessor(*this, key, false); } -inline detail::accessor handle::attr(handle key) const { return detail::accessor(*this, key, true); } -inline detail::accessor handle::attr(const char *key) const { return detail::accessor(*this, key, true); } -inline iterator handle::begin() const { return iterator(PyObject_GetIter(ptr()), false); } -inline iterator handle::end() const { return iterator(nullptr, false); } -inline detail::args_proxy handle::operator*() const { return detail::args_proxy(*this); } - class bytes; class str : public object { @@ -400,24 +414,6 @@ inline namespace literals { inline str operator"" _s(const char *s, size_t size) { return {s, size}; } } -inline pybind11::str handle::str() const { - PyObject *strValue = PyObject_Str(m_ptr); -#if PY_MAJOR_VERSION < 3 - PyObject *unicode = PyUnicode_FromEncodedObject(strValue, "utf-8", nullptr); - Py_XDECREF(strValue); strValue = unicode; -#endif - return pybind11::str(strValue, false); -} - -inline pybind11::str handle::repr() const { - PyObject *strValue = PyObject_Repr(m_ptr); -#if PY_MAJOR_VERSION < 3 - PyObject *unicode = PyUnicode_FromEncodedObject(strValue, "utf-8", nullptr); - Py_XDECREF(strValue); strValue = unicode; -#endif - return pybind11::str(strValue, false); -} - class bytes : public object { public: PYBIND11_OBJECT_DEFAULT(bytes, object, PYBIND11_BYTES_CHECK) @@ -691,4 +687,37 @@ inline size_t len(handle h) { return (size_t) result; } +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 accessor object_api::operator[](handle key) const { return {derived(), key, false}; } +template accessor object_api::operator[](const char *key) const { return {derived(), key, false}; } +template accessor object_api::attr(handle key) const { return {derived(), key, true}; } +template accessor object_api::attr(const char *key) const { return {derived(), key, true}; } +template args_proxy object_api::operator*() const { return {derived().ptr()}; } + +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}; +} + +template +handle object_api::get_type() const { return (PyObject *) Py_TYPE(derived().ptr()); } + +NAMESPACE_END(detail) NAMESPACE_END(pybind11) From 865e43034b98a317dc523a23075750d564cb477c Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Wed, 21 Sep 2016 01:06:32 +0200 Subject: [PATCH 2/5] Make attr and item accessors throw on error instead of returning nullptr This also adds the `hasattr` and `getattr` functions which are needed with the new attribute behavior. The new functions behave exactly like their Python counterparts. Similarly `object` gets a `contains` method which calls `__contains__`, i.e. it's the same as the `in` keyword in Python. --- include/pybind11/cast.h | 9 +++-- include/pybind11/numpy.h | 6 ++-- include/pybind11/pybind11.h | 45 +++++++++++++----------- include/pybind11/pytypes.h | 70 +++++++++++++++++++++++++++++-------- tests/pybind11_tests.cpp | 2 +- 5 files changed, 90 insertions(+), 42 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1d9740d38..7fa0348eb 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -39,7 +39,10 @@ PYBIND11_NOINLINE inline internals &get_internals() { return *internals_ptr; handle builtins(PyEval_GetBuiltins()); const char *id = PYBIND11_INTERNALS_ID; - capsule caps(builtins[id]); + capsule caps; + if (builtins.contains(id)) { + caps = builtins[id]; + } if (caps.check()) { internals_ptr = caps; } else { @@ -1221,7 +1224,7 @@ private: } void process(list &/*args_list*/, arg_v a) { - if (m_kwargs[a.name]) { + if (m_kwargs.contains(a.name)) { #if defined(NDEBUG) multiple_values_error(); #else @@ -1240,7 +1243,7 @@ private: void process(list &/*args_list*/, detail::kwargs_proxy kp) { for (const auto &k : dict(kp, true)) { - if (m_kwargs[k.first]) { + if (m_kwargs.contains(k.first)) { #if defined(NDEBUG) multiple_values_error(); #else diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index a3e66728a..fb8d3c6d0 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -125,11 +125,11 @@ private: static npy_api lookup() { module m = module::import("numpy.core.multiarray"); - object c = (object) m.attr("_ARRAY_API"); + auto c = m.attr("_ARRAY_API").cast(); #if PY_MAJOR_VERSION >= 3 - void **api_ptr = (void **) (c ? PyCapsule_GetPointer(c.ptr(), NULL) : nullptr); + void **api_ptr = (void **) PyCapsule_GetPointer(c.ptr(), NULL); #else - void **api_ptr = (void **) (c ? PyCObject_AsVoidPtr(c.ptr()) : nullptr); + void **api_ptr = (void **) PyCObject_AsVoidPtr(c.ptr()); #endif npy_api api; #define DECL_NPY_API(Func) api.Func##_ = (decltype(api.Func##_)) api_ptr[API_##Func]; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 4a6fa8dcf..a0e2e725c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -278,9 +278,11 @@ protected: object scope_module; if (rec->scope) { - scope_module = (object) rec->scope.attr("__module__"); - if (!scope_module) - scope_module = (object) rec->scope.attr("__name__"); + if (hasattr(rec->scope, "__module__")) { + scope_module = rec->scope.attr("__module__"); + } else if (hasattr(rec->scope, "__name__")) { + scope_module = rec->scope.attr("__name__"); + } } m_ptr = PyCFunction_NewEx(rec->def, rec_capsule.ptr(), scope_module.ptr()); @@ -544,8 +546,8 @@ public: template module &def(const char *name_, Func &&f, const Extra& ... extra) { - cpp_function func(std::forward(f), name(name_), - sibling((handle) attr(name_)), scope(*this), extra...); + cpp_function func(std::forward(f), name(name_), scope(*this), + sibling(getattr(*this, name_, none())), extra...); /* PyModule_AddObject steals a reference to 'func' */ PyModule_AddObject(ptr(), name_, func.inc_ref().ptr()); return *this; @@ -588,16 +590,18 @@ protected: object name(PYBIND11_FROM_STRING(rec->name), false); object scope_module; if (rec->scope) { - scope_module = (object) rec->scope.attr("__module__"); - if (!scope_module) - scope_module = (object) rec->scope.attr("__name__"); + if (hasattr(rec->scope, "__module__")) { + scope_module = rec->scope.attr("__module__"); + } else if (hasattr(rec->scope, "__name__")) { + scope_module = rec->scope.attr("__name__"); + } } #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 /* Qualified names for Python >= 3.3 */ object scope_qualname; - if (rec->scope) - scope_qualname = (object) rec->scope.attr("__qualname__"); + if (rec->scope && hasattr(rec->scope, "__qualname__")) + scope_qualname = rec->scope.attr("__qualname__"); object ht_qualname; if (scope_qualname) { ht_qualname = object(PyUnicode_FromFormat( @@ -894,17 +898,16 @@ public: template class_ &def(const char *name_, Func&& f, const Extra&... extra) { - cpp_function cf(std::forward(f), name(name_), - sibling(attr(name_)), is_method(*this), - extra...); + cpp_function cf(std::forward(f), name(name_), is_method(*this), + sibling(getattr(*this, name_, none())), extra...); attr(cf.name()) = cf; return *this; } template class_ & def_static(const char *name_, Func f, const Extra&... extra) { - cpp_function cf(std::forward(f), name(name_), - sibling(attr(name_)), scope(*this), extra...); + cpp_function cf(std::forward(f), name(name_), scope(*this), + sibling(getattr(*this, name_, none())), extra...); attr(cf.name()) = cf; return *this; } @@ -1338,16 +1341,16 @@ PYBIND11_NOINLINE inline void print(tuple args, dict kwargs) { for (size_t i = 0; i < args.size(); ++i) { strings[i] = args[i].cast().str(); } - auto sep = kwargs["sep"] ? kwargs["sep"] : cast(" "); + auto sep = kwargs.contains("sep") ? kwargs["sep"] : cast(" "); auto line = sep.attr("join").cast()(strings); - auto file = kwargs["file"] ? kwargs["file"].cast() - : module::import("sys").attr("stdout"); + auto file = kwargs.contains("file") ? kwargs["file"].cast() + : module::import("sys").attr("stdout"); auto write = file.attr("write").cast(); write(line); - write(kwargs["end"] ? kwargs["end"] : cast("\n")); + write(kwargs.contains("end") ? kwargs["end"] : cast("\n")); - if (kwargs["flush"] && kwargs["flush"].cast()) { + if (kwargs.contains("flush") && kwargs["flush"].cast()) { file.attr("flush").cast()(); } } @@ -1500,7 +1503,7 @@ inline function get_type_overload(const void *this_ptr, const detail::type_info if (cache.find(key) != cache.end()) return function(); - function overload = (function) py_object.attr(name); + function overload = getattr(py_object, name, function()); if (overload.is_cpp_function()) { cache.insert(key); return function(); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 872c90cc9..180bcdd7c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -41,6 +41,7 @@ public: accessor attr(handle key) const; accessor attr(const char *key) const; args_proxy operator*() const; + template bool contains(T &&key) const; template object operator()(Args &&...args) const; @@ -117,6 +118,52 @@ public: template T cast() &&; }; +inline bool hasattr(handle obj, handle name) { + return PyObject_HasAttr(obj.ptr(), name.ptr()) == 1; +} + +inline bool hasattr(handle obj, const char *name) { + return PyObject_HasAttrString(obj.ptr(), name) == 1; +} + +inline object getattr(handle obj, handle name) { + PyObject *result = PyObject_GetAttr(obj.ptr(), name.ptr()); + if (!result) { throw error_already_set(); } + return {result, false}; +} + +inline object getattr(handle obj, const char *name) { + PyObject *result = PyObject_GetAttrString(obj.ptr(), name); + if (!result) { throw error_already_set(); } + return {result, false}; +} + +inline object getattr(handle obj, handle name, handle default_) { + if (PyObject *result = PyObject_GetAttr(obj.ptr(), name.ptr())) { + return {result, false}; + } else { + PyErr_Clear(); + return {default_, true}; + } +} + +inline object getattr(handle obj, const char *name, handle default_) { + if (PyObject *result = PyObject_GetAttrString(obj.ptr(), name)) { + return {result, false}; + } else { + PyErr_Clear(); + return {default_, true}; + } +} + +inline void setattr(handle obj, handle name, handle value) { + if (PyObject_SetAttr(obj.ptr(), name.ptr(), value.ptr()) != 0) { throw error_already_set(); } +} + +inline void setattr(handle obj, const char *name, handle value) { + if (PyObject_SetAttrString(obj.ptr(), name, value.ptr()) != 0) { throw error_already_set(); } +} + NAMESPACE_BEGIN(detail) inline handle get_function(handle value) { if (value) { @@ -151,24 +198,14 @@ public: } operator object() const { - object result(attr ? PyObject_GetAttr(obj.ptr(), key.ptr()) - : PyObject_GetItem(obj.ptr(), key.ptr()), false); - if (!result) {PyErr_Clear(); } - return result; + PyObject *result = attr ? PyObject_GetAttr(obj.ptr(), key.ptr()) + : PyObject_GetItem(obj.ptr(), key.ptr()); + if (!result) { throw error_already_set(); } + return {result, false}; } template T cast() const { return operator object().cast(); } - operator bool() const { - if (attr) { - return (bool) PyObject_HasAttr(obj.ptr(), key.ptr()); - } else { - object result(PyObject_GetItem(obj.ptr(), key.ptr()), false); - if (!result) PyErr_Clear(); - return (bool) result; - } - }; - private: handle obj; object key; @@ -598,6 +635,8 @@ public: detail::dict_iterator begin() const { return (++detail::dict_iterator(*this, 0)); } detail::dict_iterator end() const { return detail::dict_iterator(); } 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; } }; class list : public object { @@ -695,6 +734,9 @@ template accessor object_api::operator[](const char *key) const template accessor object_api::attr(handle key) const { return {derived(), key, true}; } template accessor object_api::attr(const char *key) const { return {derived(), key, true}; } template args_proxy object_api::operator*() const { return {derived().ptr()}; } +template template bool object_api::contains(T &&key) const { + return attr("__contains__").template cast()(std::forward(key)).template cast(); +} template pybind11::str object_api::str() const { diff --git a/tests/pybind11_tests.cpp b/tests/pybind11_tests.cpp index f3f557a23..35981a0a6 100644 --- a/tests/pybind11_tests.cpp +++ b/tests/pybind11_tests.cpp @@ -39,7 +39,7 @@ PYBIND11_PLUGIN(pybind11_tests) { for (const auto &initializer : initializers()) initializer(m); - if (!m.attr("have_eigen")) m.attr("have_eigen") = py::cast(false); + if (!py::hasattr(m, "have_eigen")) m.attr("have_eigen") = py::cast(false); return m.ptr(); } From 242b146a5165015045d6547d4840fa6afc6161f4 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 8 Sep 2016 17:02:04 +0200 Subject: [PATCH 3/5] Extend attribute and item accessor interface using object_api --- docs/changelog.rst | 2 + include/pybind11/cast.h | 2 +- include/pybind11/numpy.h | 6 +-- include/pybind11/pybind11.h | 11 ++-- include/pybind11/pytypes.h | 101 +++++++++++++++++++++++------------- tests/constructor_stats.h | 2 +- tests/test_python_types.cpp | 37 ++++++++++++- tests/test_python_types.py | 30 +++++++++++ 8 files changed, 143 insertions(+), 48 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a9886e039..647622448 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -53,6 +53,8 @@ Breaking changes queued for v2.0.0 (Not yet released) * Added ``py::dict`` keyword constructor:``auto d = dict("number"_a=42, "name"_a="World");`` * Added ``py::str::format()`` method and ``_s`` literal: ``py::str s = "1 + 2 = {}"_s.format(3);`` +* Attribute and item accessors now have a more complete interface which makes it possible + to chain attributes ``obj.attr("a")[key].attr("b").attr("method")(1, 2, 3)```. * Various minor improvements of library internals (no user-visible changes) 1.8.1 (July 12, 2016) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 7fa0348eb..683eb2ccd 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1219,7 +1219,7 @@ private: void process(list &args_list, detail::args_proxy ap) { for (const auto &a : ap) { - args_list.append(a.cast()); + args_list.append(a); } } diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index fb8d3c6d0..996bb7c6d 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -125,7 +125,7 @@ private: static npy_api lookup() { module m = module::import("numpy.core.multiarray"); - auto c = m.attr("_ARRAY_API").cast(); + auto c = m.attr("_ARRAY_API"); #if PY_MAJOR_VERSION >= 3 void **api_ptr = (void **) PyCapsule_GetPointer(c.ptr(), NULL); #else @@ -220,9 +220,7 @@ private: struct field_descr { PYBIND11_STR_TYPE name; object format; pybind11::int_ offset; }; std::vector field_descriptors; - auto fields = attr("fields").cast(); - auto items = fields.attr("items").cast(); - for (auto field : items()) { + for (auto field : attr("fields").attr("items")()) { auto spec = object(field, true).cast(); auto name = spec[0].cast(); auto format = spec[1].cast()[0].cast(); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index a0e2e725c..9c6bc32b8 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -176,7 +176,7 @@ protected: if (a.descr) a.descr = strdup(a.descr); else if (a.value) - a.descr = strdup(((std::string) ((object) handle(a.value).attr("__repr__"))().str()).c_str()); + a.descr = strdup(a.value.attr("__repr__")().cast().c_str()); } auto const ®istered_types = detail::get_internals().registered_types_cpp; @@ -723,8 +723,7 @@ protected: 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", ((object) attr("__qualname__")).ptr()), false); + object ht_qualname(PyUnicode_FromFormat("%U__Meta", attr("__qualname__").ptr()), false); #endif object name(PYBIND11_FROM_STRING(name_.c_str()), false); object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); @@ -1342,16 +1341,16 @@ PYBIND11_NOINLINE inline void print(tuple args, dict kwargs) { strings[i] = args[i].cast().str(); } auto sep = kwargs.contains("sep") ? kwargs["sep"] : cast(" "); - auto line = sep.attr("join").cast()(strings); + auto line = sep.attr("join")(strings); auto file = kwargs.contains("file") ? kwargs["file"].cast() : module::import("sys").attr("stdout"); - auto write = file.attr("write").cast(); + auto write = file.attr("write"); write(line); write(kwargs.contains("end") ? kwargs["end"] : cast("\n")); if (kwargs.contains("flush") && kwargs["flush"].cast()) { - file.attr("flush").cast()(); + file.attr("flush")(); } } NAMESPACE_END(detail) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 180bcdd7c..a5e26345c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -21,7 +21,18 @@ class str; class iterator; struct arg; struct arg_v; NAMESPACE_BEGIN(detail) -class accessor; class args_proxy; +class args_proxy; + +// Accessor forward declarations +template class accessor; +namespace accessor_policies { + struct obj_attr; + struct str_attr; + struct generic_item; +} +using obj_attr_accessor = accessor; +using str_attr_accessor = accessor; +using item_accessor = accessor; /// Tag and check to identify a class which implements the Python object API class pyobject_tag { }; @@ -36,10 +47,10 @@ class object_api : public pyobject_tag { public: iterator begin() const; iterator end() const; - accessor operator[](handle key) const; - accessor operator[](const char *key) const; - accessor attr(handle key) const; - accessor attr(const char *key) const; + item_accessor operator[](handle key) const; + item_accessor operator[](const char *key) const; + obj_attr_accessor attr(handle key) const; + str_attr_accessor attr(const char *key) const; args_proxy operator*() const; template bool contains(T &&key) const; @@ -177,40 +188,60 @@ inline handle get_function(handle value) { return value; } -class accessor { +template +class accessor : public object_api> { + using key_type = typename Policy::key_type; + public: - accessor(handle obj, handle key, bool attr) - : obj(obj), key(key, true), attr(attr) { } - accessor(handle obj, const char *key, bool attr) - : obj(obj), key(PyUnicode_FromString(key), false), attr(attr) { } - accessor(const accessor &a) : obj(a.obj), key(a.key), attr(a.attr) { } + accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { } - void operator=(accessor o) { operator=(object(o)); } + void operator=(const accessor &a) { operator=(handle(a)); } + void operator=(const object &o) { operator=(handle(o)); } + void operator=(handle value) { Policy::set(obj, key, value); } - void operator=(const handle &value) { - if (attr) { - if (PyObject_SetAttr(obj.ptr(), key.ptr(), value.ptr()) == -1) - throw error_already_set(); - } else { - if (PyObject_SetItem(obj.ptr(), key.ptr(), value.ptr()) == -1) - throw error_already_set(); - } + operator object() const { return get_cache(); } + PyObject *ptr() const { return get_cache().ptr(); } + template T cast() const { return get_cache().template cast(); } + +private: + const object &get_cache() const { + if (!cache) { cache = Policy::get(obj, key); } + return cache; } - operator object() const { - PyObject *result = attr ? PyObject_GetAttr(obj.ptr(), key.ptr()) - : PyObject_GetItem(obj.ptr(), key.ptr()); +private: + handle obj; + key_type key; + mutable object cache; +}; + +NAMESPACE_BEGIN(accessor_policies) +struct obj_attr { + using key_type = object; + static object get(handle obj, handle key) { return getattr(obj, key); } + static void set(handle obj, handle key, handle val) { setattr(obj, key, val); } +}; + +struct str_attr { + using key_type = const char *; + static object get(handle obj, const char *key) { return getattr(obj, key); } + static void set(handle obj, const char *key, handle val) { setattr(obj, key, val); } +}; + +struct generic_item { + using key_type = object; + + static object get(handle obj, handle key) { + PyObject *result = PyObject_GetItem(obj.ptr(), key.ptr()); if (!result) { throw error_already_set(); } return {result, false}; } - template T cast() const { return operator object().cast(); } - -private: - handle obj; - object key; - bool attr; + static void set(handle obj, handle key, handle val) { + if (PyObject_SetItem(obj.ptr(), key.ptr(), val.ptr()) != 0) { throw error_already_set(); } + } }; +NAMESPACE_END(accessor_policies) struct list_accessor { public: @@ -442,7 +473,7 @@ public: template str format(Args &&...args) const { - return attr("format").cast()(std::forward(args)...); + return attr("format")(std::forward(args)...); } }; @@ -729,13 +760,13 @@ inline size_t len(handle h) { 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 accessor object_api::operator[](handle key) const { return {derived(), key, false}; } -template accessor object_api::operator[](const char *key) const { return {derived(), key, false}; } -template accessor object_api::attr(handle key) const { return {derived(), key, true}; } -template accessor object_api::attr(const char *key) const { return {derived(), key, true}; } +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 {derived().ptr()}; } template template bool object_api::contains(T &&key) const { - return attr("__contains__").template cast()(std::forward(key)).template cast(); + return attr("__contains__")(std::forward(key)).template cast(); } template diff --git a/tests/constructor_stats.h b/tests/constructor_stats.h index 69e385ec6..5dd215f19 100644 --- a/tests/constructor_stats.h +++ b/tests/constructor_stats.h @@ -103,7 +103,7 @@ public: int alive() { // Force garbage collection to ensure any pending destructors are invoked: - py::module::import("gc").attr("collect").operator py::object()(); + py::module::import("gc").attr("collect")(); int total = 0; for (const auto &p : _instances) if (p.second > 0) total += p.second; return total; diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index 9dafe777c..4ab90e63a 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -203,7 +203,7 @@ test_initializer python_types([](py::module &m) { py::print("no new line here", "end"_a=" -- "); py::print("next print"); - auto py_stderr = py::module::import("sys").attr("stderr").cast(); + auto py_stderr = py::module::import("sys").attr("stderr"); py::print("this goes to stderr", "file"_a=py_stderr); py::print("flush", "flush"_a=true); @@ -222,4 +222,39 @@ test_initializer python_types([](py::module &m) { auto d2 = py::dict("z"_a=3, **d1); return d2; }); + + m.def("test_accessor_api", [](py::object o) { + auto d = py::dict(); + + d["basic_attr"] = o.attr("basic_attr"); + + auto l = py::list(); + for (const auto &item : o.attr("begin_end")) { + l.append(item); + } + d["begin_end"] = l; + + d["operator[object]"] = o.attr("d")["operator[object]"_s]; + d["operator[char *]"] = o.attr("d")["operator[char *]"]; + + d["attr(object)"] = o.attr("sub").attr("attr_obj"); + d["attr(char *)"] = o.attr("sub").attr("attr_char"); + try { + o.attr("sub").attr("missing").ptr(); + } catch (const py::error_already_set &) { + d["missing_attr_ptr"] = "raised"_s; + } + try { + o.attr("missing").attr("doesn't matter"); + } catch (const py::error_already_set &) { + d["missing_attr_chain"] = "raised"_s; + } + + d["is_none"] = py::cast(o.attr("basic_attr").is_none()); + + d["operator()"] = o.attr("func")(1); + d["operator*"] = o.attr("func")(*o.attr("begin_end")); + + return d; + }); }); diff --git a/tests/test_python_types.py b/tests/test_python_types.py index fe58f9321..4f2cdb2c3 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -248,3 +248,33 @@ def test_dict_api(): from pybind11_tests import test_dict_keyword_constructor assert test_dict_keyword_constructor() == {"x": 1, "y": 2, "z": 3} + + +def test_accessors(): + from pybind11_tests import test_accessor_api + + class SubTestObject: + attr_obj = 1 + attr_char = 2 + + class TestObject: + basic_attr = 1 + begin_end = [1, 2, 3] + d = {"operator[object]": 1, "operator[char *]": 2} + sub = SubTestObject() + + def func(self, x, *args): + return self.basic_attr + x + sum(args) + + d = test_accessor_api(TestObject()) + assert d["basic_attr"] == 1 + assert d["begin_end"] == [1, 2, 3] + assert d["operator[object]"] == 1 + assert d["operator[char *]"] == 2 + assert d["attr(object)"] == 1 + assert d["attr(char *)"] == 2 + assert d["missing_attr_ptr"] == "raised" + assert d["missing_attr_chain"] == "raised" + assert d["is_none"] is False + assert d["operator()"] == 2 + assert d["operator*"] == 7 From ea763a57a8b3d79e9fdeada091a87e4fec51af7f Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 22 Sep 2016 23:39:06 +0200 Subject: [PATCH 4/5] Extend tuple and list accessor interface --- include/pybind11/pybind11.h | 2 +- include/pybind11/pytypes.h | 92 ++++++++++++++++--------------------- tests/test_python_types.cpp | 17 ++++++- tests/test_python_types.py | 4 +- 4 files changed, 60 insertions(+), 55 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9c6bc32b8..3e81d4bb5 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1338,7 +1338,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].cast().str(); + strings[i] = args[i].str(); } auto sep = kwargs.contains("sep") ? kwargs["sep"] : cast(" "); auto line = sep.attr("join")(strings); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a5e26345c..8fa01ff86 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -29,10 +29,14 @@ namespace accessor_policies { struct obj_attr; struct str_attr; struct generic_item; + struct list_item; + struct tuple_item; } using obj_attr_accessor = accessor; using str_attr_accessor = accessor; using item_accessor = accessor; +using list_accessor = accessor; +using tuple_accessor = accessor; /// Tag and check to identify a class which implements the Python object API class pyobject_tag { }; @@ -241,58 +245,42 @@ struct generic_item { if (PyObject_SetItem(obj.ptr(), key.ptr(), val.ptr()) != 0) { throw error_already_set(); } } }; + +struct list_item { + using key_type = size_t; + + 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}; + } + + static void set(handle obj, size_t index, handle val) { + // PyList_SetItem steals a reference to 'val' + if (PyList_SetItem(obj.ptr(), static_cast(index), val.inc_ref().ptr()) != 0) { + throw error_already_set(); + } + } +}; + +struct tuple_item { + using key_type = size_t; + + 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}; + } + + static void set(handle obj, size_t index, handle val) { + // PyTuple_SetItem steals a reference to 'val' + if (PyTuple_SetItem(obj.ptr(), static_cast(index), val.inc_ref().ptr()) != 0) { + throw error_already_set(); + } + } +}; NAMESPACE_END(accessor_policies) -struct list_accessor { -public: - list_accessor(handle list, size_t index) : list(list), index(index) { } - - void operator=(list_accessor o) { return operator=(object(o)); } - - void operator=(const handle &o) { - // PyList_SetItem steals a reference to 'o' - if (PyList_SetItem(list.ptr(), (ssize_t) index, o.inc_ref().ptr()) < 0) - pybind11_fail("Unable to assign value in Python list!"); - } - - operator object() const { - PyObject *result = PyList_GetItem(list.ptr(), (ssize_t) index); - if (!result) - pybind11_fail("Unable to retrieve value from Python list!"); - return object(result, true); - } - - template T cast() const { return operator object().cast(); } -private: - handle list; - size_t index; -}; - -struct tuple_accessor { -public: - tuple_accessor(handle tuple, size_t index) : tuple(tuple), index(index) { } - - void operator=(tuple_accessor o) { return operator=(object(o)); } - - void operator=(const handle &o) { - // PyTuple_SetItem steals a referenceto 'o' - if (PyTuple_SetItem(tuple.ptr(), (ssize_t) index, o.inc_ref().ptr()) < 0) - pybind11_fail("Unable to assign value in Python tuple!"); - } - - operator object() const { - PyObject *result = PyTuple_GetItem(tuple.ptr(), (ssize_t) index); - if (!result) - pybind11_fail("Unable to retrieve value from Python tuple!"); - return object(result, true); - } - - template T cast() const { return operator object().cast(); } -private: - handle tuple; - size_t index; -}; - struct dict_iterator { public: dict_iterator(handle dict = handle(), ssize_t pos = -1) : dict(dict), pos(pos) { } @@ -647,7 +635,7 @@ public: if (!m_ptr) pybind11_fail("Could not allocate tuple object!"); } size_t size() const { return (size_t) PyTuple_Size(m_ptr); } - detail::tuple_accessor operator[](size_t index) const { return detail::tuple_accessor(*this, index); } + detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } }; class dict : public object { @@ -677,7 +665,7 @@ public: if (!m_ptr) pybind11_fail("Could not allocate list object!"); } size_t size() const { return (size_t) PyList_Size(m_ptr); } - detail::list_accessor operator[](size_t index) const { return detail::list_accessor(*this, index); } + detail::list_accessor operator[](size_t index) const { return {*this, index}; } void append(handle h) const { PyList_Append(m_ptr, h.ptr()); } }; diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index 4ab90e63a..fabc78e8e 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -60,7 +60,7 @@ public: py::list get_list() { py::list list; list.append(py::str("value")); - py::print("Entry at position 0:", py::object(list[0])); + py::print("Entry at position 0:", list[0]); list[0] = py::str("overwritten"); return list; } @@ -257,4 +257,19 @@ test_initializer python_types([](py::module &m) { return d; }); + + m.def("test_tuple_accessor", [](py::tuple existing_t) { + try { + existing_t[0] = py::cast(1); + } catch (const py::error_already_set &) { + // --> Python system error + // Only new tuples (refcount == 1) are mutable + auto new_t = py::tuple(3); + for (size_t i = 0; i < new_t.size(); ++i) { + new_t[i] = py::cast(i); + } + return new_t; + } + return py::tuple(); + }); }); diff --git a/tests/test_python_types.py b/tests/test_python_types.py index 4f2cdb2c3..a6cf1fe03 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -251,7 +251,7 @@ def test_dict_api(): def test_accessors(): - from pybind11_tests import test_accessor_api + from pybind11_tests import test_accessor_api, test_tuple_accessor class SubTestObject: attr_obj = 1 @@ -278,3 +278,5 @@ def test_accessors(): assert d["is_none"] is False assert d["operator()"] == 2 assert d["operator*"] == 7 + + assert test_tuple_accessor(tuple()) == (0, 1, 2) From 2bab5793f75ddbd38b5639ec8c1fb822b7ad505d Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Fri, 23 Sep 2016 00:27:59 +0200 Subject: [PATCH 5/5] Later assignments to accessors should not overwrite the original field `auto var = l[0]` has a strange quirk: `var` is actually an accessor and not an object, so any later assignment of `var = ...` would modify l[0] instead of `var`. This is surprising compared to the non-auto assignment `py::object var = l[0]; var = ...`. By overloading `operator=` on lvalue/rvalue, the expected behavior is restored even for `auto` variables. --- include/pybind11/pybind11.h | 1 + include/pybind11/pytypes.h | 11 +++++++---- tests/test_python_types.cpp | 17 +++++++++++++++++ tests/test_python_types.py | 9 ++++++++- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3e81d4bb5..686fe5361 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -18,6 +18,7 @@ # pragma warning(disable: 4800) // warning C4800: 'int': forcing value to bool 'true' or 'false' (performance warning) # pragma warning(disable: 4996) // warning C4996: The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name # pragma warning(disable: 4702) // warning C4702: unreachable code +# pragma warning(disable: 4522) // warning C4522: multiple assignment operators specified #elif defined(__INTEL_COMPILER) # pragma warning(push) # pragma warning(disable: 186) // pointless comparison of unsigned integer with zero diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 8fa01ff86..d4c6f39ae 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -199,16 +199,19 @@ class accessor : public object_api> { public: accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { } - void operator=(const accessor &a) { operator=(handle(a)); } - void operator=(const object &o) { operator=(handle(o)); } - void operator=(handle value) { Policy::set(obj, key, value); } + void operator=(const accessor &a) && { std::move(*this).operator=(handle(a)); } + void operator=(const accessor &a) & { operator=(handle(a)); } + 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); } operator object() const { return get_cache(); } PyObject *ptr() const { return get_cache().ptr(); } template T cast() const { return get_cache().template cast(); } private: - const object &get_cache() const { + object &get_cache() const { if (!cache) { cache = Policy::get(obj, key); } return cache; } diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index fabc78e8e..678a56d15 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -272,4 +272,21 @@ test_initializer python_types([](py::module &m) { } return py::tuple(); }); + + m.def("test_accessor_assignment", []() { + auto l = py::list(1); + l[0] = py::cast(0); + + auto d = py::dict(); + d["get"] = l[0]; + auto var = l[0]; + d["deferred_get"] = var; + l[0] = py::cast(1); + d["set"] = l[0]; + var = py::cast(99); // this assignment should not overwrite l[0] + d["deferred_set"] = l[0]; + d["var"] = var; + + return d; + }); }); diff --git a/tests/test_python_types.py b/tests/test_python_types.py index a6cf1fe03..628e46124 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -251,7 +251,7 @@ def test_dict_api(): def test_accessors(): - from pybind11_tests import test_accessor_api, test_tuple_accessor + from pybind11_tests import test_accessor_api, test_tuple_accessor, test_accessor_assignment class SubTestObject: attr_obj = 1 @@ -280,3 +280,10 @@ def test_accessors(): assert d["operator*"] == 7 assert test_tuple_accessor(tuple()) == (0, 1, 2) + + d = test_accessor_assignment() + assert d["get"] == 0 + assert d["deferred_get"] == 0 + assert d["set"] == 1 + assert d["deferred_set"] == 1 + assert d["var"] == 99