From a64b2b7b021107763b560a77f358764d1a516d38 Mon Sep 17 00:00:00 2001 From: Henry Fredrick Schreiner Date: Sun, 8 Oct 2017 23:27:40 -0400 Subject: [PATCH 1/6] feat: add a priority overload with py::prepend --- docs/changelog.rst | 3 +++ docs/classes.rst | 4 +++- include/pybind11/attr.h | 15 +++++++++++++-- include/pybind11/pybind11.h | 22 +++++++++++++++++----- include/pybind11/pytypes.h | 6 ++++++ tests/test_methods_and_attributes.cpp | 6 ++++++ tests/test_methods_and_attributes.py | 22 ++++++++++++++++++++++ 7 files changed, 70 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 568611bcbd..0347da945a 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 `_ diff --git a/docs/classes.rst b/docs/classes.rst index f3610ef367..945ce9f327 100644 --- a/docs/classes.rst +++ b/docs/classes.rst @@ -367,7 +367,9 @@ Attempting to bind ``Pet::set`` will cause an error since the compiler does not know which method the user intended to select. We can disambiguate by casting them to function pointers. Binding multiple functions to the same Python name automatically creates a chain of function overloads that will be tried in -sequence. +the order they were defined. If the ``py::prepend()`` tag is added to the +definition, the function will be placed at the begining of the overload sequence +instead. .. code-block:: cpp diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index d0a8b34b8f..1ee7f86c24 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), is_prepended(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 is_prepended : 1; + /// Number of arguments (including py::args and/or py::kwargs, if present) std::uint16_t nargs; @@ -477,6 +483,11 @@ struct process_attribute : process_attribute_default static void init(const module_local &l, type_record *r) { r->module_local = l.value; } }; +/// Process an 'prepend' attribute, putting this at the top of the overload chain +template <> struct process_attribute : process_attribute_default { + static void init(const prepend &, function_record *r) { r->is_prepended = 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 db0efd445e..adde5233b3 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -372,10 +372,9 @@ class cpp_function : public function { 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 @@ class cpp_function : public function { 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->is_prepended) { + // 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 48b5240836..f2d2061fcf 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1235,6 +1235,12 @@ class capsule : public object { 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 b6df41d4d8..8febefcdbb 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 c296b6868d..d5793296b7 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) From 10a3260d22533c52d46e253522a3897d42e717dd Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sun, 4 Oct 2020 09:22:39 -0400 Subject: [PATCH 2/6] doc: fix wording as suggested by rwgk --- include/pybind11/attr.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 1ee7f86c24..46099dbe10 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -483,8 +483,9 @@ struct process_attribute : process_attribute_default static void init(const module_local &l, type_record *r) { r->module_local = l.value; } }; -/// Process an 'prepend' attribute, putting this at the top of the overload chain -template <> struct process_attribute : process_attribute_default { +/// 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->is_prepended = true; } }; From 44fa0a73e062a553f46d216551bef39ad0a727a0 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2020 16:06:42 -0400 Subject: [PATCH 3/6] feat: add get_pointer --- docs/changelog.rst | 3 +++ include/pybind11/pytypes.h | 10 ++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0347da945a..3a4fb6dc45 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -139,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/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f2d2061fcf..b042188f49 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1229,13 +1229,19 @@ class capsule : public object { } 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. + /// 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"); From 3e8ecc7853000a96d727fe44b86b4138a6455e7b Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2020 16:38:09 -0400 Subject: [PATCH 4/6] refactor: is_prepended -> prepend (internal) --- include/pybind11/attr.h | 6 +++--- include/pybind11/pybind11.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 46099dbe10..79d3e58804 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -142,7 +142,7 @@ 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_prepended(false) { } + has_kwargs(false), has_kw_only_args(false), prepend(false) { } /// Function name char *name = nullptr; /* why no C++ strings? They generate heavier code.. */ @@ -193,7 +193,7 @@ struct function_record { bool has_kw_only_args : 1; /// True if this function is to be inserted at the beginning of the overload resolution chain - bool is_prepended : 1; + bool prepend : 1; /// Number of arguments (including py::args and/or py::kwargs, if present) std::uint16_t nargs; @@ -486,7 +486,7 @@ struct process_attribute : process_attribute_default /// 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->is_prepended = true; } + static void init(const prepend &, function_record *r) { r->prepend = true; } }; /// Process an 'arithmetic' attribute for enums (does nothing here) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index adde5233b3..1f1e310d50 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -385,7 +385,7 @@ class cpp_function : public function { #endif ); - if (rec->is_prepended) { + 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. From dd862c2de6ed39e25d19a2c1390ee333cd591ee6 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2020 16:41:23 -0400 Subject: [PATCH 5/6] docs: suggestion from @wjakob --- docs/advanced/functions.rst | 12 +++++++++--- docs/classes.rst | 4 +--- docs/upgrade.rst | 4 ++++ include/pybind11/attr.h | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst index 81b4c0282e..ebdff9c93d 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/classes.rst b/docs/classes.rst index 945ce9f327..f3610ef367 100644 --- a/docs/classes.rst +++ b/docs/classes.rst @@ -367,9 +367,7 @@ Attempting to bind ``Pet::set`` will cause an error since the compiler does not know which method the user intended to select. We can disambiguate by casting them to function pointers. Binding multiple functions to the same Python name automatically creates a chain of function overloads that will be tried in -the order they were defined. If the ``py::prepend()`` tag is added to the -definition, the function will be placed at the begining of the overload sequence -instead. +sequence. .. code-block:: cpp diff --git a/docs/upgrade.rst b/docs/upgrade.rst index 62e2312e94..02d9049bb4 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 79d3e58804..0c41670926 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -141,7 +141,7 @@ 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), + is_operator(false), is_method(false), has_args(false), has_kwargs(false), has_kw_only_args(false), prepend(false) { } /// Function name From 3cd25d2aca08e7a87813133ece1f9877e1c48607 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2020 21:22:07 -0400 Subject: [PATCH 6/6] tests: add test covering get_pointer/set_pointer --- tests/test_pytypes.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index d38ea197d4..f478c21ecc 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; });