Skip to content

Commit 93e6919

Browse files
authored
fix: enable py::implicitly_convertible<py::none, ...> for py::class_-wrapped types (#3059)
* Allow casting from None to a custom object, closes #2778 * ci.yml patch from the smart_holder branch for full CI coverage.
1 parent 484b0f0 commit 93e6919

File tree

4 files changed

+55
-20
lines changed

4 files changed

+55
-20
lines changed

.github/workflows/ci.yml

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ jobs:
100100
run: >
101101
cmake -S . -B .
102102
-DPYBIND11_WERROR=ON
103-
-DDOWNLOAD_CATCH=ON
103+
-DDOWNLOAD_CATCH=OFF
104104
-DDOWNLOAD_EIGEN=ON
105105
-DCMAKE_CXX_STANDARD=11
106106
${{ matrix.args }}
@@ -111,10 +111,10 @@ jobs:
111111
- name: Python tests C++11
112112
run: cmake --build . --target pytest -j 2
113113

114-
- name: C++11 tests
115-
# TODO: Figure out how to load the DLL on Python 3.8+
116-
if: "!(runner.os == 'Windows' && (matrix.python == 3.8 || matrix.python == 3.9 || matrix.python == '3.10-dev'))"
117-
run: cmake --build . --target cpptest -j 2
114+
#- name: C++11 tests
115+
# # TODO: Figure out how to load the DLL on Python 3.8+
116+
# if: "!(runner.os == 'Windows' && (matrix.python == 3.8 || matrix.python == 3.9 || matrix.python == '3.10-dev'))"
117+
# run: cmake --build . --target cpptest -j 2
118118

119119
- name: Interface test C++11
120120
run: cmake --build . --target test_cmake_build
@@ -127,7 +127,7 @@ jobs:
127127
run: >
128128
cmake -S . -B build2
129129
-DPYBIND11_WERROR=ON
130-
-DDOWNLOAD_CATCH=ON
130+
-DDOWNLOAD_CATCH=OFF
131131
-DDOWNLOAD_EIGEN=ON
132132
-DCMAKE_CXX_STANDARD=17
133133
${{ matrix.args }}
@@ -139,10 +139,10 @@ jobs:
139139
- name: Python tests
140140
run: cmake --build build2 --target pytest
141141

142-
- name: C++ tests
143-
# TODO: Figure out how to load the DLL on Python 3.8+
144-
if: "!(runner.os == 'Windows' && (matrix.python == 3.8 || matrix.python == 3.9 || matrix.python == '3.10-dev'))"
145-
run: cmake --build build2 --target cpptest
142+
#- name: C++ tests
143+
# # TODO: Figure out how to load the DLL on Python 3.8+
144+
# if: "!(runner.os == 'Windows' && (matrix.python == 3.8 || matrix.python == 3.9 || matrix.python == '3.10-dev'))"
145+
# run: cmake --build build2 --target cpptest
146146

147147
- name: Interface test
148148
run: cmake --build build2 --target test_cmake_build
@@ -754,7 +754,7 @@ jobs:
754754
cmake -S . -B build
755755
-G "Visual Studio 16 2019" -A Win32
756756
-DPYBIND11_WERROR=ON
757-
-DDOWNLOAD_CATCH=ON
757+
-DDOWNLOAD_CATCH=OFF
758758
-DDOWNLOAD_EIGEN=ON
759759
${{ matrix.args }}
760760
- name: Build C++11
@@ -800,7 +800,7 @@ jobs:
800800
cmake -S . -B build
801801
-G "Visual Studio 14 2015" -A x64
802802
-DPYBIND11_WERROR=ON
803-
-DDOWNLOAD_CATCH=ON
803+
-DDOWNLOAD_CATCH=OFF
804804
-DDOWNLOAD_EIGEN=ON
805805
806806
- name: Build C++14
@@ -849,7 +849,7 @@ jobs:
849849
cmake -S . -B build
850850
-G "Visual Studio 15 2017" -A x64
851851
-DPYBIND11_WERROR=ON
852-
-DDOWNLOAD_CATCH=ON
852+
-DDOWNLOAD_CATCH=OFF
853853
-DDOWNLOAD_EIGEN=ON
854854
-DCMAKE_CXX_STANDARD=${{ matrix.std }}
855855
${{ matrix.args }}

include/pybind11/detail/type_caster_base.h

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -657,12 +657,6 @@ class type_caster_generic {
657657
PYBIND11_NOINLINE bool load_impl(handle src, bool convert) {
658658
if (!src) return false;
659659
if (!typeinfo) return try_load_foreign_module_local(src);
660-
if (src.is_none()) {
661-
// Defer accepting None to other overloads (if we aren't in convert mode):
662-
if (!convert) return false;
663-
value = nullptr;
664-
return true;
665-
}
666660

667661
auto &this_ = static_cast<ThisT &>(*this);
668662
this_.check_holder_compat();
@@ -731,7 +725,19 @@ class type_caster_generic {
731725
}
732726

733727
// Global typeinfo has precedence over foreign module_local
734-
return try_load_foreign_module_local(src);
728+
if (try_load_foreign_module_local(src)) {
729+
return true;
730+
}
731+
732+
// Custom converters didn't take None, now we convert None to nullptr.
733+
if (src.is_none()) {
734+
// Defer accepting None to other overloads (if we aren't in convert mode):
735+
if (!convert) return false;
736+
value = nullptr;
737+
return true;
738+
}
739+
740+
return false;
735741
}
736742

737743

tests/test_methods_and_attributes.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@ int none3(std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; }
118118
int none4(std::shared_ptr<NoneTester> *obj) { return obj && *obj ? (*obj)->answer : -1; }
119119
int none5(const std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; }
120120

121+
// Issue #2778: implicit casting from None to object (not pointer)
122+
class NoneCastTester {
123+
public:
124+
int answer = -1;
125+
NoneCastTester() = default;
126+
NoneCastTester(int v) : answer(v) {};
127+
};
128+
121129
struct StrIssue {
122130
int val = -1;
123131

@@ -354,6 +362,16 @@ TEST_SUBMODULE(methods_and_attributes, m) {
354362
m.def("no_none_kwarg", &none2, "a"_a.none(false));
355363
m.def("no_none_kwarg_kw_only", &none2, py::kw_only(), "a"_a.none(false));
356364

365+
// test_casts_none
366+
// Issue #2778: implicit casting from None to object (not pointer)
367+
py::class_<NoneCastTester>(m, "NoneCastTester")
368+
.def(py::init<>())
369+
.def(py::init<int>())
370+
.def(py::init([](py::none const&) { return NoneCastTester{}; }));
371+
py::implicitly_convertible<py::none, NoneCastTester>();
372+
m.def("ok_obj_or_none", [](NoneCastTester const& foo) { return foo.answer; });
373+
374+
357375
// test_str_issue
358376
// Issue #283: __str__ called on uninitialized instance when constructor arguments invalid
359377
py::class_<StrIssue>(m, "StrIssue")

tests/test_methods_and_attributes.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,17 @@ def test_accepts_none(msg):
431431
assert "incompatible function arguments" in str(excinfo.value)
432432

433433

434+
def test_casts_none(msg):
435+
"""#2778: implicit casting from None to object (not pointer)"""
436+
a = m.NoneCastTester()
437+
assert m.ok_obj_or_none(a) == -1
438+
a = m.NoneCastTester(4)
439+
assert m.ok_obj_or_none(a) == 4
440+
a = m.NoneCastTester(None)
441+
assert m.ok_obj_or_none(a) == -1
442+
assert m.ok_obj_or_none(None) == -1
443+
444+
434445
def test_str_issue(msg):
435446
"""#283: __str__ called on uninitialized instance when constructor arguments invalid"""
436447

0 commit comments

Comments
 (0)