Update to pass test for unhashable collections (#4640)

Lib/test/test_collections.py

@@ -892,8 +892,6 @@ class TestOneTrickPonyABCs(ABCTestCase):
         self.assertFalse(isinstance(CoroLike(), Coroutine))
         self.assertFalse(issubclass(CoroLike, Coroutine))
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_Hashable(self):
         # Check some non-hashables
         non_samples = [bytearray(), list(), set(), dict()]

Lib/test/test_typing.py

@@ -3516,8 +3516,6 @@ class GetUtilitiesTestCase(TestCase):
 
 class CollectionsAbcTests(BaseTestCase):
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_hashable(self):
         self.assertIsInstance(42, typing.Hashable)
         self.assertNotIsInstance([], typing.Hashable)

derive-impl/src/pyclass.rs

@@ -218,6 +218,7 @@ fn generate_class_def(
     module_name: Option<&str>,
     base: Option<String>,
     metaclass: Option<String>,
+    unhashable: bool,
     attrs: &[Attribute],
 ) -> Result<TokenStream> {
     let doc = attrs.doc().or_else(|| {
@@ -242,6 +243,11 @@ fn generate_class_def(
         Some(v) => quote!(Some(#v) ),
         None => quote!(None),
     };
+    let unhashable = if unhashable {
+        quote!(true)
+    } else {
+        quote!(false)
+    };
     let basicsize = quote!(std::mem::size_of::<#ident>());
     let is_pystruct = attrs.iter().any(|attr| {
         attr.path.is_ident("derive")
@@ -290,6 +296,7 @@ fn generate_class_def(
             const TP_NAME: &'static str = #module_class_name;
             const DOC: Option<&'static str> = #doc;
             const BASICSIZE: usize = #basicsize;
+            const UNHASHABLE: bool = #unhashable;
         }
 
         impl ::rustpython_vm::class::StaticType for #ident {
@@ -319,12 +326,15 @@ pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStrea
     let module_name = class_meta.module()?;
     let base = class_meta.base()?;
     let metaclass = class_meta.metaclass()?;
+    let unhashable = class_meta.unhashable()?;
+
     let class_def = generate_class_def(
         ident,
         &class_name,
         module_name.as_deref(),
         base,
         metaclass,
+        unhashable,
         attrs,
     )?;
 

derive-impl/src/util.rs

@@ -263,7 +263,8 @@ impl ItemMeta for AttrItemMeta {
 pub(crate) struct ClassItemMeta(ItemMetaInner);
 
 impl ItemMeta for ClassItemMeta {
-    const ALLOWED_NAMES: &'static [&'static str] = &["module", "name", "base", "metaclass"];
+    const ALLOWED_NAMES: &'static [&'static str] =
+        &["module", "name", "base", "metaclass", "unhashable"];
 
     fn from_inner(inner: ItemMetaInner) -> Self {
         Self(inner)
@@ -299,6 +300,10 @@ impl ClassItemMeta {
         self.inner()._optional_str("base")
     }
 
+    pub fn unhashable(&self) -> Result<bool> {
+        self.inner()._bool("unhashable")
+    }
+
     pub fn metaclass(&self) -> Result<Option<String>> {
         self.inner()._optional_str("metaclass")
     }

vm/src/builtins/bytearray.rs

@@ -30,9 +30,8 @@ use crate::{
     },
     sliceable::{SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp},
     types::{
-        AsBuffer, AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, Hashable,
-        Initializer, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible,
-        Unhashable,
+        AsBuffer, AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, Initializer,
+        IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible,
     },
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
     VirtualMachine,
@@ -41,7 +40,7 @@ use bstr::ByteSlice;
 use once_cell::sync::Lazy;
 use std::mem::size_of;
 
-#[pyclass(module = false, name = "bytearray")]
+#[pyclass(module = false, name = "bytearray", unhashable = true)]
 #[derive(Debug, Default)]
 pub struct PyByteArray {
     inner: PyRwLock<PyBytesInner>,
@@ -100,7 +99,6 @@ pub(crate) fn init(context: &Context) {
     with(
         Constructor,
         Initializer,
-        Hashable,
         Comparable,
         AsBuffer,
         AsMapping,
@@ -873,8 +871,6 @@ impl AsNumber for PyByteArray {
     }
 }
 
-impl Unhashable for PyByteArray {}
-
 impl Iterable for PyByteArray {
     fn iter(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
         Ok(PyByteArrayIterator {

vm/src/builtins/dict.rs

@@ -19,8 +19,8 @@ use crate::{
     protocol::{PyIterIter, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods},
     recursion::ReprGuard,
     types::{
-        AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, Hashable, Initializer,
-        IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable,
+        AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, Initializer, IterNext,
+        IterNextIterable, Iterable, PyComparisonOp, Unconstructible,
     },
     vm::VirtualMachine,
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
@@ -32,7 +32,7 @@ use std::fmt;
 
 pub type DictContentType = dictdatatype::Dict;
 
-#[pyclass(module = false, name = "dict")]
+#[pyclass(module = false, name = "dict", unhashable = true)]
 #[derive(Default)]
 pub struct PyDict {
     entries: DictContentType,
@@ -206,7 +206,6 @@ impl PyDict {
         Constructor,
         Initializer,
         AsMapping,
-        Hashable,
         Comparable,
         Iterable,
         AsSequence,
@@ -512,8 +511,6 @@ impl Comparable for PyDict {
     }
 }
 
-impl Unhashable for PyDict {}
-
 impl Iterable for PyDict {
     fn iter(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
         Ok(PyDictKeyIterator::new(zelf).into_pyobject(vm))

vm/src/builtins/list.rs

@@ -13,8 +13,8 @@ use crate::{
     sequence::{MutObjectSequenceOp, OptionalRangeArgs, SequenceExt, SequenceMutExt},
     sliceable::{SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp},
     types::{
-        AsMapping, AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext,
-        IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable,
+        AsMapping, AsSequence, Comparable, Constructor, Initializer, IterNext, IterNextIterable,
+        Iterable, PyComparisonOp, Unconstructible,
     },
     utils::collection_repr,
     vm::VirtualMachine,
@@ -22,7 +22,7 @@ use crate::{
 };
 use std::{fmt, ops::DerefMut};
 
-#[pyclass(module = false, name = "list")]
+#[pyclass(module = false, name = "list", unhashable = true)]
 #[derive(Default)]
 pub struct PyList {
     elements: PyRwLock<Vec<PyObjectRef>>,
@@ -86,15 +86,7 @@ pub(crate) struct SortOptions {
 pub type PyListRef = PyRef<PyList>;
 
 #[pyclass(
-    with(
-        Constructor,
-        Initializer,
-        AsMapping,
-        Iterable,
-        Hashable,
-        Comparable,
-        AsSequence
-    ),
+    with(Constructor, Initializer, AsMapping, Iterable, Comparable, AsSequence),
     flags(BASETYPE)
 )]
 impl PyList {
@@ -492,8 +484,6 @@ impl Comparable for PyList {
     }
 }
 
-impl Unhashable for PyList {}
-
 fn do_sort(
     vm: &VirtualMachine,
     values: &mut Vec<PyObjectRef>,

vm/src/builtins/set.rs

@@ -17,7 +17,7 @@ use crate::{
     types::AsNumber,
     types::{
         AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable,
-        Iterable, PyComparisonOp, Unconstructible, Unhashable,
+        Iterable, PyComparisonOp, Unconstructible,
     },
     utils::collection_repr,
     vm::VirtualMachine,
@@ -28,7 +28,7 @@ use std::{fmt, ops::Deref};
 
 pub type SetContentType = dictdatatype::Dict<()>;
 
-#[pyclass(module = false, name = "set")]
+#[pyclass(module = false, name = "set", unhashable = true)]
 #[derive(Default)]
 pub struct PySet {
     pub(super) inner: PySetInner,
@@ -70,7 +70,7 @@ impl PySet {
     }
 }
 
-#[pyclass(module = false, name = "frozenset")]
+#[pyclass(module = false, name = "frozenset", unhashable = true)]
 #[derive(Default)]
 pub struct PyFrozenSet {
     inner: PySetInner,
@@ -489,15 +489,7 @@ fn reduce_set(
 }
 
 #[pyclass(
-    with(
-        Constructor,
-        Initializer,
-        AsSequence,
-        Hashable,
-        Comparable,
-        Iterable,
-        AsNumber
-    ),
+    with(Constructor, Initializer, AsSequence, Comparable, Iterable, AsNumber),
     flags(BASETYPE)
 )]
 impl PySet {
@@ -805,8 +797,6 @@ impl Comparable for PySet {
     }
 }
 
-impl Unhashable for PySet {}
-
 impl Iterable for PySet {
     fn iter(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
         Ok(zelf.inner.iter().into_pyobject(vm))

vm/src/builtins/slice.rs

@@ -5,13 +5,13 @@ use crate::{
     convert::ToPyObject,
     function::{FuncArgs, OptionalArg, PyComparisonValue},
     sliceable::SaturatedSlice,
-    types::{Comparable, Constructor, Hashable, PyComparisonOp, Unhashable},
+    types::{Comparable, Constructor, PyComparisonOp},
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
 };
 use num_bigint::{BigInt, ToBigInt};
 use num_traits::{One, Signed, Zero};
 
-#[pyclass(module = false, name = "slice")]
+#[pyclass(module = false, name = "slice", unhashable = true)]
 #[derive(Debug)]
 pub struct PySlice {
     pub start: Option<PyObjectRef>,
@@ -25,7 +25,7 @@ impl PyPayload for PySlice {
     }
 }
 
-#[pyclass(with(Hashable, Comparable))]
+#[pyclass(with(Comparable))]
 impl PySlice {
     #[pygetset]
     fn start(&self, vm: &VirtualMachine) -> PyObjectRef {
@@ -258,8 +258,6 @@ impl Comparable for PySlice {
     }
 }
 
-impl Unhashable for PySlice {}
-
 #[pyclass(module = false, name = "EllipsisType")]
 #[derive(Debug)]
 pub struct PyEllipsis;

vm/src/builtins/type.rs

@@ -1019,11 +1019,10 @@ impl SetAttr for PyType {
         }
         let assign = value.is_assign();
 
-        let mut attributes = zelf.attributes.write();
         if let PySetterValue::Assign(value) = value {
-            attributes.insert(attr_name, value);
+            zelf.attributes.write().insert(attr_name, value);
         } else {
-            let prev_value = attributes.remove(attr_name);
+            let prev_value = zelf.attributes.write().remove(attr_name);
             if prev_value.is_none() {
                 return Err(vm.new_exception(
                     vm.ctx.exceptions.attribute_error.to_owned(),

vm/src/class.rs

@@ -4,7 +4,7 @@ use crate::{
     builtins::{PyBaseObject, PyBoundMethod, PyType, PyTypeRef},
     identifier,
     object::{Py, PyObjectPayload, PyObjectRef, PyRef},
-    types::{PyTypeFlags, PyTypeSlots},
+    types::{hash_not_implemented, PyTypeFlags, PyTypeSlots},
     vm::Context,
 };
 use rustpython_common::{lock::PyRwLock, static_cell};
@@ -60,6 +60,7 @@ pub trait PyClassDef {
     const TP_NAME: &'static str;
     const DOC: Option<&'static str> = None;
     const BASICSIZE: usize;
+    const UNHASHABLE: bool = false;
 }
 
 impl<T> PyClassDef for PyRef<T>
@@ -71,6 +72,7 @@ where
     const TP_NAME: &'static str = T::TP_NAME;
     const DOC: Option<&'static str> = T::DOC;
     const BASICSIZE: usize = T::BASICSIZE;
+    const UNHASHABLE: bool = T::UNHASHABLE;
 }
 
 pub trait PyClassImpl: PyClassDef {
@@ -112,6 +114,10 @@ pub trait PyClassImpl: PyClassDef {
                     .into();
             class.set_attr(identifier!(ctx, __new__), bound);
         }
+
+        if class.slots.hash.load().map_or(0, |h| h as usize) == hash_not_implemented as usize {
+            class.set_attr(ctx.names.__hash__, ctx.none.clone().into());
+        }
     }
 
     fn make_class(ctx: &Context) -> PyTypeRef
@@ -140,6 +146,11 @@ pub trait PyClassImpl: PyClassDef {
             doc: Self::DOC,
             ..Default::default()
         };
+
+        if Self::UNHASHABLE {
+            slots.hash.store(Some(hash_not_implemented));
+        }
+
         Self::extend_slots(&mut slots);
         slots
     }

vm/src/stdlib/collections.rs

@@ -16,8 +16,8 @@ mod _collections {
         sequence::{MutObjectSequenceOp, OptionalRangeArgs},
         sliceable::SequenceIndexOp,
         types::{
-            AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable,
-            Iterable, PyComparisonOp, Unhashable,
+            AsSequence, Comparable, Constructor, Initializer, IterNext, IterNextIterable, Iterable,
+            PyComparisonOp,
         },
         utils::collection_repr,
         AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
@@ -27,7 +27,7 @@ mod _collections {
     use std::collections::VecDeque;
 
     #[pyattr]
-    #[pyclass(name = "deque")]
+    #[pyclass(name = "deque", unhashable = true)]
     #[derive(Debug, Default, PyPayload)]
     struct PyDeque {
         deque: PyRwLock<VecDeque<PyObjectRef>>,
@@ -57,7 +57,7 @@ mod _collections {
 
     #[pyclass(
         flags(BASETYPE),
-        with(Constructor, Initializer, AsSequence, Comparable, Hashable, Iterable)
+        with(Constructor, Initializer, AsSequence, Comparable, Iterable)
     )]
     impl PyDeque {
         #[pymethod]
@@ -574,8 +574,6 @@ mod _collections {
         }
     }
 
-    impl Unhashable for PyDeque {}
-
     impl Iterable for PyDeque {
         fn iter(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
             Ok(PyDequeIterator::new(zelf).into_pyobject(vm))

vm/src/types/slot.rs

@@ -243,6 +243,11 @@ fn hash_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyHash> {
     }
 }
 
+/// Marks a type as unhashable. Similar to PyObject_HashNotImplemented in CPython
+pub fn hash_not_implemented(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyHash> {
+    Err(vm.new_type_error(format!("unhashable type: {}", zelf.class().name())))
+}
+
 fn call_wrapper(zelf: &PyObject, args: FuncArgs, vm: &VirtualMachine) -> PyResult {
     vm.call_special_method(zelf.to_owned(), identifier!(vm, __call__), args)
 }
@@ -414,7 +419,17 @@ impl PyType {
                 update_pointer_slot!(as_mapping, mapping_methods);
             }
             _ if name == identifier!(ctx, __hash__) => {
-                toggle_slot!(hash, hash_wrapper);
+                let is_unhashable = self
+                    .attributes
+                    .read()
+                    .get(identifier!(ctx, __hash__))
+                    .map_or(false, |a| a.is(&ctx.none));
+                let wrapper = if is_unhashable {
+                    hash_not_implemented
+                } else {
+                    hash_wrapper
+                };
+                toggle_slot!(hash, wrapper);
             }
             _ if name == identifier!(ctx, __call__) => {
                 toggle_slot!(call, call_wrapper);
@@ -663,22 +678,6 @@ pub trait Hashable: PyPayload {
     fn hash(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyHash>;
 }
 
-pub trait Unhashable: PyPayload {}
-
-impl<T> Hashable for T
-where
-    T: Unhashable,
-{
-    fn slot_hash(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyHash> {
-        Err(vm.new_type_error(format!("unhashable type: '{}'", zelf.class().name())))
-    }
-
-    #[cold]
-    fn hash(_zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<PyHash> {
-        unreachable!("slot_hash is implemented for unhashable types");
-    }
-}
-
 #[pyclass]
 pub trait Comparable: PyPayload {
     #[inline]