From f6ed6adbd1798036c09460fbdcab3b430a80dd28 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 9 Jan 2022 13:48:48 -0500 Subject: [PATCH 1/3] Add additional info to TypeInfo when C++->Python casting fails --- include/pybind11/pybind11.h | 7 +++++++ tests/test_operator_overloading.cpp | 24 +++++++++++++++++++++--- tests/test_operator_overloading.py | 8 ++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 8c7545ba3a..ec60429b1d 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -996,6 +996,13 @@ class cpp_function : public function { "Python type! The signature was\n\t"; msg += it->signature; append_note_if_missing_header_is_suspected(msg); +#if PY_VERSION_HEX >= 0x03030000 + // Attach additional error info to the exception if supported + if (PyErr_Occurred()) { + raise_from(PyExc_TypeError, msg.c_str()); + return nullptr; + } +#endif PyErr_SetString(PyExc_TypeError, msg.c_str()); return nullptr; } diff --git a/tests/test_operator_overloading.cpp b/tests/test_operator_overloading.cpp index 0b6c496cf2..2756c0bfd3 100644 --- a/tests/test_operator_overloading.cpp +++ b/tests/test_operator_overloading.cpp @@ -7,10 +7,11 @@ BSD-style license that can be found in the LICENSE file. */ -#include "pybind11_tests.h" #include "constructor_stats.h" -#include +#include "pybind11_tests.h" #include +#include +#include class Vector2 { public: @@ -71,6 +72,12 @@ int operator+(const C2 &, const C2 &) { return 22; } int operator+(const C2 &, const C1 &) { return 21; } int operator+(const C1 &, const C2 &) { return 12; } +struct HashMe { + std::string member; +}; + +bool operator==(const HashMe &lhs, const HashMe &rhs) { return lhs.member == rhs.member; } + // Note: Specializing explicit within `namespace std { ... }` is done due to a // bug in GCC<7. If you are supporting compilers later than this, consider // specializing `using template<> struct std::hash<...>` in the global @@ -82,6 +89,13 @@ namespace std { // Not a good hash function, but easy to test size_t operator()(const Vector2 &) { return 4; } }; + + template <> + struct hash { + std::size_t operator()(const HashMe &selector) const { + return std::hash()(selector.member); + } + }; } // namespace std // Not a good abs function, but easy to test. @@ -228,8 +242,12 @@ TEST_SUBMODULE(operators, m) { .def("__hash__", &Hashable::hash) .def(py::init()) .def(py::self == py::self); -} + // define __eq__ but not __hash__ + py::class_(m, "Hashable3").def(py::self == py::self); + + m.def("get_unhashable_HashMe_set", []() { return std::unordered_set{{"one"}}; }); +} #if !defined(_MSC_VER) && !defined(__INTEL_COMPILER) #pragma GCC diagnostic pop #endif diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index b7137d1592..44c6ecb31d 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import pytest +import env from pybind11_tests import ConstructorStats from pybind11_tests import operators as m @@ -144,3 +145,10 @@ def test_overriding_eq_reset_hash(): assert hash(hashable(15)) == 15 assert hash(hashable(15)) == hash(hashable(15)) + + +def test_return_set_of_unhashable(): + with pytest.raises(TypeError) as excinfo: + m.get_unhashable_HashMe_set() + if not env.PY2: + assert str(excinfo.value.__cause__).startswith("unhashable type:") From 92009c9f4ebe7d1a88ddf01e9606933fa8950894 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 9 Jan 2022 16:08:10 -0500 Subject: [PATCH 2/3] Fix typo --- tests/test_operator_overloading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_operator_overloading.cpp b/tests/test_operator_overloading.cpp index 2756c0bfd3..990810741d 100644 --- a/tests/test_operator_overloading.cpp +++ b/tests/test_operator_overloading.cpp @@ -244,7 +244,7 @@ TEST_SUBMODULE(operators, m) { .def(py::self == py::self); // define __eq__ but not __hash__ - py::class_(m, "Hashable3").def(py::self == py::self); + py::class_(m, "HashMe").def(py::self == py::self); m.def("get_unhashable_HashMe_set", []() { return std::unordered_set{{"one"}}; }); } From 9af5d81c4817dbdea3b4a0e6b39f0a782a4150ef Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 10 Jan 2022 10:16:46 -0500 Subject: [PATCH 3/3] Address reviewer comments --- tests/test_operator_overloading.cpp | 1 + tests/test_operator_overloading.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_operator_overloading.cpp b/tests/test_operator_overloading.cpp index 2756c0bfd3..97d44b3e4d 100644 --- a/tests/test_operator_overloading.cpp +++ b/tests/test_operator_overloading.cpp @@ -90,6 +90,7 @@ namespace std { size_t operator()(const Vector2 &) { return 4; } }; + // HashMe has a hash function in C++ but no `__hash__` for Python. template <> struct hash { std::size_t operator()(const HashMe &selector) const { diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index 44c6ecb31d..8cf375b6da 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -136,8 +136,9 @@ def test_overriding_eq_reset_hash(): assert m.Comparable(15) is not m.Comparable(15) assert m.Comparable(15) == m.Comparable(15) - with pytest.raises(TypeError): - hash(m.Comparable(15)) # TypeError: unhashable type: 'm.Comparable' + with pytest.raises(TypeError) as excinfo: + hash(m.Comparable(15)) + assert str(excinfo.value).startswith("unhashable type:") for hashable in (m.Hashable, m.Hashable2): assert hashable(15) is not hashable(15)