Skip to content

Commit a288ae4

Browse files
committed
Defer None loading to second pass
Many of our `is_none()` checks in type caster loading return true, but this should really be considered a deferral so that, for example, an overload with a `py::none` argument would win over one that takes `py::none` as a null option. This keeps None-accepting for the `!convert` pass only for std::optional and void casters. (The `char` caster already deferred None; this just extends that behaviour to other casters).
1 parent d15b217 commit a288ae4

File tree

4 files changed

+35
-2
lines changed

4 files changed

+35
-2
lines changed

include/pybind11/cast.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ class type_caster_generic {
218218
if (!src || !typeinfo)
219219
return false;
220220
if (src.is_none()) {
221+
// Defer accepting None to other overloads (if we aren't in convert mode):
222+
if (!convert) return false;
221223
value = nullptr;
222224
return true;
223225
}
@@ -978,6 +980,8 @@ struct copyable_holder_caster : public type_caster_base<type> {
978980
if (!src || !typeinfo)
979981
return false;
980982
if (src.is_none()) {
983+
// Defer accepting None to other overloads (if we aren't in convert mode):
984+
if (!convert) return false;
981985
value = nullptr;
982986
return true;
983987
}

include/pybind11/functional.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ struct type_caster<std::function<Return(Args...)>> {
2222
using function_type = Return (*) (Args...);
2323

2424
public:
25-
bool load(handle src, bool) {
26-
if (src.is_none())
25+
bool load(handle src, bool convert) {
26+
if (src.is_none()) {
27+
// Defer accepting None to other overloads (if we aren't in convert mode):
28+
if (!convert) return false;
2729
return true;
30+
}
2831

2932
if (!isinstance<function>(src))
3033
return false;

tests/test_python_types.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,18 @@ test_initializer python_types([](py::module &m) {
499499
m.def("return_none_int", []() -> int * { return nullptr; });
500500
m.def("return_none_float", []() -> float * { return nullptr; });
501501

502+
m.def("defer_none_cstring", [](char *) { return false; });
503+
m.def("defer_none_cstring", [](py::none) { return true; });
504+
m.def("defer_none_custom", [](ExamplePythonTypes *) { return false; });
505+
m.def("defer_none_custom", [](py::none) { return true; });
506+
// void and optional, however, don't defer:
507+
m.def("nodefer_none_void", [](void *) { return true; });
508+
m.def("nodefer_none_void", [](py::none) { return false; });
509+
#ifdef PYBIND11_HAS_OPTIONAL
510+
m.def("nodefer_none_optional", [](std::optional<int>) { return true; });
511+
m.def("nodefer_none_optional", [](py::none) { return false; });
512+
#endif
513+
502514
m.def("return_capsule_with_destructor",
503515
[]() {
504516
py::print("creating capsule");

tests/test_python_types.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,22 @@ def test_builtins_cast_return_none():
549549
assert m.return_none_float() is None
550550

551551

552+
def test_none_deferred():
553+
"""None passed as various argument types should defer to other overloads"""
554+
import pybind11_tests as m
555+
556+
assert not m.defer_none_cstring("abc")
557+
assert m.defer_none_cstring(None)
558+
assert not m.defer_none_custom(m.ExamplePythonTypes.new_instance())
559+
assert m.defer_none_custom(None)
560+
assert m.nodefer_none_void(None)
561+
if has_optional:
562+
assert m.nodefer_none_optional(None)
563+
564+
552565
def test_capsule_with_destructor(capture):
553566
import pybind11_tests as m
567+
pytest.gc_collect()
554568
with capture:
555569
a = m.return_capsule_with_destructor()
556570
del a

0 commit comments

Comments
 (0)