Skip to content
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

fix: enable py::implicitly_convertible<py::none, ...> for py::class_-wrapped types #3059

Merged
merged 2 commits into from
Jun 26, 2021
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
26 changes: 13 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ jobs:
run: >
cmake -S . -B .
-DPYBIND11_WERROR=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_CATCH=OFF
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=11
${{ matrix.args }}
Expand All @@ -111,10 +111,10 @@ jobs:
- name: Python tests C++11
run: cmake --build . --target pytest -j 2

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

- name: Interface test C++11
run: cmake --build . --target test_cmake_build
Expand All @@ -127,7 +127,7 @@ jobs:
run: >
cmake -S . -B build2
-DPYBIND11_WERROR=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_CATCH=OFF
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=17
${{ matrix.args }}
Expand All @@ -139,10 +139,10 @@ jobs:
- name: Python tests
run: cmake --build build2 --target pytest

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

- name: Interface test
run: cmake --build build2 --target test_cmake_build
Expand Down Expand Up @@ -754,7 +754,7 @@ jobs:
cmake -S . -B build
-G "Visual Studio 16 2019" -A Win32
-DPYBIND11_WERROR=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_CATCH=OFF
-DDOWNLOAD_EIGEN=ON
${{ matrix.args }}
- name: Build C++11
Expand Down Expand Up @@ -800,7 +800,7 @@ jobs:
cmake -S . -B build
-G "Visual Studio 14 2015" -A x64
-DPYBIND11_WERROR=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_CATCH=OFF
-DDOWNLOAD_EIGEN=ON

- name: Build C++14
Expand Down Expand Up @@ -849,7 +849,7 @@ jobs:
cmake -S . -B build
-G "Visual Studio 15 2017" -A x64
-DPYBIND11_WERROR=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_CATCH=OFF
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=${{ matrix.std }}
${{ matrix.args }}
Expand Down
20 changes: 13 additions & 7 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -657,12 +657,6 @@ class type_caster_generic {
PYBIND11_NOINLINE bool load_impl(handle src, bool convert) {
if (!src) return false;
if (!typeinfo) return try_load_foreign_module_local(src);
if (src.is_none()) {
// Defer accepting None to other overloads (if we aren't in convert mode):
if (!convert) return false;
value = nullptr;
return true;
}

auto &this_ = static_cast<ThisT &>(*this);
this_.check_holder_compat();
Expand Down Expand Up @@ -731,7 +725,19 @@ class type_caster_generic {
}

// Global typeinfo has precedence over foreign module_local
return try_load_foreign_module_local(src);
if (try_load_foreign_module_local(src)) {
return true;
}

// Custom converters didn't take None, now we convert None to nullptr.
if (src.is_none()) {
// Defer accepting None to other overloads (if we aren't in convert mode):
if (!convert) return false;
value = nullptr;
return true;
}

return false;
}


Expand Down
18 changes: 18 additions & 0 deletions tests/test_methods_and_attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ int none3(std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; }
int none4(std::shared_ptr<NoneTester> *obj) { return obj && *obj ? (*obj)->answer : -1; }
int none5(const std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; }

// Issue #2778: implicit casting from None to object (not pointer)
class NoneCastTester {
public:
int answer = -1;
NoneCastTester() = default;
NoneCastTester(int v) : answer(v) {};
};

struct StrIssue {
int val = -1;

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

// test_casts_none
// Issue #2778: implicit casting from None to object (not pointer)
py::class_<NoneCastTester>(m, "NoneCastTester")
.def(py::init<>())
.def(py::init<int>())
.def(py::init([](py::none const&) { return NoneCastTester{}; }));
py::implicitly_convertible<py::none, NoneCastTester>();
m.def("ok_obj_or_none", [](NoneCastTester const& foo) { return foo.answer; });


// test_str_issue
// Issue #283: __str__ called on uninitialized instance when constructor arguments invalid
py::class_<StrIssue>(m, "StrIssue")
Expand Down
11 changes: 11 additions & 0 deletions tests/test_methods_and_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,17 @@ def test_accepts_none(msg):
assert "incompatible function arguments" in str(excinfo.value)


def test_casts_none(msg):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-merge comment: msg is unused and can be removed. Looks like a copy-paste accident.
(I'll create a PR to fix.)

"""#2778: implicit casting from None to object (not pointer)"""
a = m.NoneCastTester()
assert m.ok_obj_or_none(a) == -1
a = m.NoneCastTester(4)
assert m.ok_obj_or_none(a) == 4
a = m.NoneCastTester(None)
assert m.ok_obj_or_none(a) == -1
assert m.ok_obj_or_none(None) == -1


def test_str_issue(msg):
"""#283: __str__ called on uninitialized instance when constructor arguments invalid"""

Expand Down