From 9a0c96dd4c7438955c891b05e07fbbd91c45d7a0 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2020 22:36:33 -0400 Subject: [PATCH] feat: py::prepend tag (#1131) * feat: add a priority overload with py::prepend * doc: fix wording as suggested by rwgk * feat: add get_pointer * refactor: is_prepended -> prepend (internal) * docs: suggestion from @wjakob * tests: add test covering get_pointer/set_pointer --- docs/advanced/functions.rst | 12 +++++++++--- docs/changelog.rst | 6 ++++++ docs/upgrade.rst | 4 ++++ include/pybind11/attr.h | 16 ++++++++++++++-- include/pybind11/pybind11.h | 22 +++++++++++++++++----- include/pybind11/pytypes.h | 14 +++++++++++++- tests/test_methods_and_attributes.cpp | 6 ++++++ tests/test_methods_and_attributes.py | 22 ++++++++++++++++++++++ tests/test_pytypes.cpp | 17 ++++++++++++++--- 9 files changed, 105 insertions(+), 14 deletions(-) diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst index 81b4c0282..ebdff9c93 100644 --- a/docs/advanced/functions.rst +++ b/docs/advanced/functions.rst @@ -540,11 +540,13 @@ an explicit ``py::arg().noconvert()`` attribute in the function definition). If the second pass also fails a ``TypeError`` is raised. Within each pass, overloads are tried in the order they were registered with -pybind11. +pybind11. If the ``py::prepend()`` tag is added to the definition, a function +can be placed at the beginning of the overload sequence instead, allowing user +overloads to proceed built in functions. What this means in practice is that pybind11 will prefer any overload that does -not require conversion of arguments to an overload that does, but otherwise prefers -earlier-defined overloads to later-defined ones. +not require conversion of arguments to an overload that does, but otherwise +prefers earlier-defined overloads to later-defined ones. .. note:: @@ -553,3 +555,7 @@ earlier-defined overloads to later-defined ones. requiring one conversion over one requiring three, but only prioritizes overloads requiring no conversion at all to overloads that require conversion of at least one argument. + +.. versionadded:: 2.6 + + The ``py::prepend()`` tag. diff --git a/docs/changelog.rst b/docs/changelog.rst index 568611bcb..3a4fb6dc4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,9 @@ See :ref:`upgrade-guide-2.6` for help upgrading to the new version. ``py::type::of(h)``. `#2364 `_ +* Added ``py::prepend()``, allowing a function to be placed at the beginning of + the overload chain. + `#1131 `_ * Perfect forwarding support for methods. `#2048 `_ @@ -136,6 +139,9 @@ Smaller or developer focused features: * ``py::vectorize`` is now supported on functions that return void. `#1969 `_ +* ``py::capsule`` supports ``get_pointer`` and ``set_pointer``. + `#1131 `_ + * Bugfixes related to more extensive testing. `#2321 `_ diff --git a/docs/upgrade.rst b/docs/upgrade.rst index 62e2312e9..02d9049bb 100644 --- a/docs/upgrade.rst +++ b/docs/upgrade.rst @@ -26,6 +26,10 @@ missing. The undocumented ``h.get_type()`` method has been deprecated and replaced by ``py::type::of(h)``. +Enums now have a ``__str__`` method pre-defined; if you want to override it, +the simplest fix is to add the new ``py::prepend()`` tag when defining +``"__str__"``. + If ``__eq__`` defined but not ``__hash__``, ``__hash__`` is now set to ``None``, as in normal CPython. You should add ``__hash__`` if you intended the class to be hashable, possibly using the new ``py::hash`` shortcut. diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index d0a8b34b8..0c4167092 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -74,6 +74,9 @@ struct module_local { const bool value; constexpr module_local(bool v = true) : /// Annotation to mark enums as an arithmetic type struct arithmetic { }; +/// Mark a function for addition at the beginning of the existing overload chain instead of the end +struct prepend { }; + /** \rst A call policy which places one or more guard variables (``Ts...``) around the function call. @@ -138,8 +141,8 @@ struct argument_record { struct function_record { function_record() : is_constructor(false), is_new_style_constructor(false), is_stateless(false), - is_operator(false), is_method(false), - has_args(false), has_kwargs(false), has_kw_only_args(false) { } + is_operator(false), is_method(false), has_args(false), + has_kwargs(false), has_kw_only_args(false), prepend(false) { } /// Function name char *name = nullptr; /* why no C++ strings? They generate heavier code.. */ @@ -189,6 +192,9 @@ struct function_record { /// True once a 'py::kw_only' is encountered (any following args are keyword-only) bool has_kw_only_args : 1; + /// True if this function is to be inserted at the beginning of the overload resolution chain + bool prepend : 1; + /// Number of arguments (including py::args and/or py::kwargs, if present) std::uint16_t nargs; @@ -477,6 +483,12 @@ struct process_attribute : process_attribute_default static void init(const module_local &l, type_record *r) { r->module_local = l.value; } }; +/// Process a 'prepend' attribute, putting this at the beginning of the overload chain +template <> +struct process_attribute : process_attribute_default { + static void init(const prepend &, function_record *r) { r->prepend = true; } +}; + /// Process an 'arithmetic' attribute for enums (does nothing here) template <> struct process_attribute : process_attribute_default {}; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index db0efd445..1f1e310d5 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -372,10 +372,9 @@ protected: if (!m_ptr) pybind11_fail("cpp_function::cpp_function(): Could not allocate function object"); } else { - /* Append at the end of the overload chain */ + /* Append at the beginning or end of the overload chain */ m_ptr = rec->sibling.ptr(); inc_ref(); - chain_start = chain; if (chain->is_method != rec->is_method) pybind11_fail("overloading a method with both static and instance methods is not supported; " #if defined(NDEBUG) @@ -385,9 +384,22 @@ protected: std::string(pybind11::str(rec->scope.attr("__name__"))) + "." + std::string(rec->name) + signature #endif ); - while (chain->next) - chain = chain->next; - chain->next = rec; + + if (rec->prepend) { + // Beginning of chain; we need to replace the capsule's current head-of-the-chain + // pointer with this one, then make this one point to the previous head of the + // chain. + chain_start = rec; + rec->next = chain; + auto rec_capsule = reinterpret_borrow(((PyCFunctionObject *) m_ptr)->m_self); + rec_capsule.set_pointer(rec); + } else { + // Or end of chain (normal behavior) + chain_start = chain; + while (chain->next) + chain = chain->next; + chain->next = rec; + } } std::string signatures; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e292b74fe..103badcc2 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1236,12 +1236,24 @@ public: } template operator T *() const { + return get_pointer(); + } + + /// Get the pointer the capsule holds. + template + T* get_pointer() const { auto name = this->name(); - T * result = static_cast(PyCapsule_GetPointer(m_ptr, name)); + T *result = static_cast(PyCapsule_GetPointer(m_ptr, name)); if (!result) pybind11_fail("Unable to extract capsule contents!"); return result; } + /// Replaces a capsule's pointer *without* calling the destructor on the existing one. + void set_pointer(const void *value) { + if (PyCapsule_SetPointer(m_ptr, const_cast(value)) != 0) + pybind11_fail("Could not set capsule pointer"); + } + const char *name() const { return PyCapsule_GetName(m_ptr); } }; diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index b6df41d4d..8febefcdb 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -284,6 +284,12 @@ TEST_SUBMODULE(methods_and_attributes, m) { py::class_(m, "MetaclassOverride", py::metaclass((PyObject *) &PyType_Type)) .def_property_readonly_static("readonly", [](py::object) { return 1; }); + // test_overload_ordering + m.def("overload_order", [](std::string) { return 1; }); + m.def("overload_order", [](std::string) { return 2; }); + m.def("overload_order", [](int) { return 3; }); + m.def("overload_order", [](int) { return 4; }, py::prepend{}); + #if !defined(PYPY_VERSION) // test_dynamic_attributes class DynamicClass { diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index c296b6868..d5793296b 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -438,3 +438,25 @@ def test_ref_qualified(): r.refQualified(17) assert r.value == 17 assert r.constRefQualified(23) == 40 + + +def test_overload_ordering(): + 'Check to see if the normal overload order (first defined) and prepend overload order works' + assert m.overload_order("string") == 1 + assert m.overload_order(0) == 4 + + # Different for Python 2 vs. 3 + uni_name = type(u"").__name__ + + assert "1. overload_order(arg0: int) -> int" in m.overload_order.__doc__ + assert "2. overload_order(arg0: {}) -> int".format(uni_name) in m.overload_order.__doc__ + assert "3. overload_order(arg0: {}) -> int".format(uni_name) in m.overload_order.__doc__ + assert "4. overload_order(arg0: int) -> int" in m.overload_order.__doc__ + + with pytest.raises(TypeError) as err: + m.overload_order(1.1) + + assert "1. (arg0: int) -> int" in str(err.value) + assert "2. (arg0: {}) -> int".format(uni_name) in str(err.value) + assert "3. (arg0: {}) -> int".format(uni_name) in str(err.value) + assert "4. (arg0: int) -> int" in str(err.value) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 8717f601e..45fd87185 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -108,7 +108,7 @@ TEST_SUBMODULE(pytypes, m) { }); m.def("return_capsule_with_name_and_destructor", []() { - auto capsule = py::capsule((void *) 1234, "pointer type description", [](PyObject *ptr) { + auto capsule = py::capsule((void *) 12345, "pointer type description", [](PyObject *ptr) { if (ptr) { auto name = PyCapsule_GetName(ptr); py::print("destructing capsule ({}, '{}')"_s.format( @@ -116,8 +116,19 @@ TEST_SUBMODULE(pytypes, m) { )); } }); - void *contents = capsule; - py::print("created capsule ({}, '{}')"_s.format((size_t) contents, capsule.name())); + + capsule.set_pointer((void *) 1234); + + // Using get_pointer() + void* contents1 = static_cast(capsule); + void* contents2 = capsule.get_pointer(); + void* contents3 = capsule.get_pointer(); + + auto result1 = reinterpret_cast(contents1); + auto result2 = reinterpret_cast(contents2); + auto result3 = reinterpret_cast(contents3); + + py::print("created capsule ({}, '{}')"_s.format(result1 & result2 & result3, capsule.name())); return capsule; });