Skip to content

Priority overload #1131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions docs/advanced/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand All @@ -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.
6 changes: 6 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ See :ref:`upgrade-guide-2.6` for help upgrading to the new version.
``py::type::of(h)``.
`#2364 <https://github.com/pybind/pybind11/pull/2364>`_

* Added ``py::prepend()``, allowing a function to be placed at the beginning of
the overload chain.
`#1131 <https://github.com/pybind/pybind11/pull/1131>`_

* Perfect forwarding support for methods.
`#2048 <https://github.com/pybind/pybind11/pull/2048>`_
Expand Down Expand Up @@ -136,6 +139,9 @@ Smaller or developer focused features:
* ``py::vectorize`` is now supported on functions that return void.
`#1969 <https://github.com/pybind/pybind11/pull/1969>`_

* ``py::capsule`` supports ``get_pointer`` and ``set_pointer``.
`#1131 <https://github.com/pybind/pybind11/pull/1131>`_

* Bugfixes related to more extensive testing.
`#2321 <https://github.com/pybind/pybind11/pull/2321>`_

Expand Down
4 changes: 4 additions & 0 deletions docs/upgrade.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 14 additions & 2 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.. */
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -477,6 +483,12 @@ struct process_attribute<module_local> : process_attribute_default<module_local>
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<prepend> : process_attribute_default<prepend> {
static void init(const prepend &, function_record *r) { r->prepend = true; }
};

/// Process an 'arithmetic' attribute for enums (does nothing here)
template <>
struct process_attribute<arithmetic> : process_attribute_default<arithmetic> {};
Expand Down
22 changes: 17 additions & 5 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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->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<capsule>(((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;
Expand Down
14 changes: 13 additions & 1 deletion include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1229,12 +1229,24 @@ class capsule : public object {
}

template <typename T> operator T *() const {
return get_pointer<T>();
}

/// Get the pointer the capsule holds.
template<typename T = void>
T* get_pointer() const {
auto name = this->name();
T * result = static_cast<T *>(PyCapsule_GetPointer(m_ptr, name));
T *result = static_cast<T *>(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<void *>(value)) != 0)
pybind11_fail("Could not set capsule pointer");
}

const char *name() const { return PyCapsule_GetName(m_ptr); }
};

Expand Down
6 changes: 6 additions & 0 deletions tests/test_methods_and_attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ TEST_SUBMODULE(methods_and_attributes, m) {
py::class_<MetaclassOverride>(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 {
Expand Down
22 changes: 22 additions & 0 deletions tests/test_methods_and_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
17 changes: 14 additions & 3 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,27 @@ 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(
(size_t) PyCapsule_GetPointer(ptr, name), name
));
}
});
void *contents = capsule;
py::print("created capsule ({}, '{}')"_s.format((size_t) contents, capsule.name()));

capsule.set_pointer((void *) 1234);

// Using get_pointer<T>()
void* contents1 = static_cast<void*>(capsule);
void* contents2 = capsule.get_pointer();
void* contents3 = capsule.get_pointer<void>();

auto result1 = reinterpret_cast<size_t>(contents1);
auto result2 = reinterpret_cast<size_t>(contents2);
auto result3 = reinterpret_cast<size_t>(contents3);

py::print("created capsule ({}, '{}')"_s.format(result1 & result2 & result3, capsule.name()));
return capsule;
});

Expand Down