bugfix: py contains raises errors when appropiate (#4209)

* bugfix: contains now throws an exception if the key is not hashable

* Fix tests and improve robustness

* Remove todo

* Workaround PyPy corner case

* PyPy xfail

* Fix typo

* fix xfail

* Make clang-tidy happy

* Remove redundant exc checking

include/pybind11/pytypes.h

@@ -1967,7 +1967,11 @@ public:
     void clear() /* py-non-const */ { PyDict_Clear(ptr()); }
     void clear() /* py-non-const */ { PyDict_Clear(ptr()); }
     template <typename T>
     template <typename T>
     bool contains(T &&key) const {
     bool contains(T &&key) const {
-        return PyDict_Contains(m_ptr, detail::object_or_cast(std::forward<T>(key)).ptr()) == 1;
+        auto result = PyDict_Contains(m_ptr, detail::object_or_cast(std::forward<T>(key)).ptr());
+        if (result == -1) {
+            throw error_already_set();
+        }
+        return result == 1;
     }
     }
 
 
 private:
 private:
@@ -2053,7 +2057,11 @@ public:
     bool empty() const { return size() == 0; }
     bool empty() const { return size() == 0; }
     template <typename T>
     template <typename T>
     bool contains(T &&val) const {
     bool contains(T &&val) const {
-        return PySet_Contains(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr()) == 1;
+        auto result = PySet_Contains(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr());
+        if (result == -1) {
+            throw error_already_set();
+        }
+        return result == 1;
     }
     }
 };
 };
 
 

tests/test_pytypes.cpp

@@ -183,7 +183,7 @@ TEST_SUBMODULE(pytypes, m) {
         return d2;
         return d2;
     });
     });
     m.def("dict_contains",
     m.def("dict_contains",
-          [](const py::dict &dict, py::object val) { return dict.contains(val); });
+          [](const py::dict &dict, const py::object &val) { return dict.contains(val); });
     m.def("dict_contains",
     m.def("dict_contains",
           [](const py::dict &dict, const char *val) { return dict.contains(val); });
           [](const py::dict &dict, const char *val) { return dict.contains(val); });
 
 
@@ -538,6 +538,9 @@ TEST_SUBMODULE(pytypes, m) {
 
 
     m.def("hash_function", [](py::object obj) { return py::hash(std::move(obj)); });
     m.def("hash_function", [](py::object obj) { return py::hash(std::move(obj)); });
 
 
+    m.def("obj_contains",
+          [](py::object &obj, const py::object &key) { return obj.contains(key); });
+
     m.def("test_number_protocol", [](const py::object &a, const py::object &b) {
     m.def("test_number_protocol", [](const py::object &a, const py::object &b) {
         py::list l;
         py::list l;
         l.append(a.equal(b));
         l.append(a.equal(b));

tests/test_pytypes.py

@@ -168,6 +168,31 @@ def test_dict(capture, doc):
     assert m.dict_keyword_constructor() == {"x": 1, "y": 2, "z": 3}
     assert m.dict_keyword_constructor() == {"x": 1, "y": 2, "z": 3}
 
 
 
 
+class CustomContains:
+    d = {"key": None}
+
+    def __contains__(self, m):
+        return m in self.d
+
+
+@pytest.mark.parametrize(
+    "arg,func",
+    [
+        (set(), m.anyset_contains),
+        (dict(), m.dict_contains),
+        (CustomContains(), m.obj_contains),
+    ],
+)
+@pytest.mark.xfail("env.PYPY and sys.pypy_version_info < (7, 3, 10)", strict=False)
+def test_unhashable_exceptions(arg, func):
+    class Unhashable:
+        __hash__ = None
+
+    with pytest.raises(TypeError) as exc_info:
+        func(arg, Unhashable())
+    assert "unhashable type:" in str(exc_info.value)
+
+
 def test_tuple():
 def test_tuple():
     assert m.tuple_no_args() == ()
     assert m.tuple_no_args() == ()
     assert m.tuple_ssize_t() == ()
     assert m.tuple_ssize_t() == ()