Repository: qir-alliance/pyqir (language: python)

Issue:
Make enums hashable
For data types that are plain enums, like Opcode, IntPredicate, FloatPredicate, and Linkage, it would be convenient if they were hashable so that they could be used as keys in a dict. For example:
```
from pyqir import Opcode, IntPredicate  
dict1 = {
    Opcode.ADD: "+",  
    Opcode.SUB: "-",  
  }  
dict2 = {
    IntPredicate.EQ: "==",  
    IntPredicate.NE: "!=",  
  }  
```


Discussion / hints:

Fix patch:
diff --git a/pyqir/pyqir/_native.pyi b/pyqir/pyqir/_native.pyi
index 7cf86415..42be7609 100644
--- a/pyqir/pyqir/_native.pyi
+++ b/pyqir/pyqir/_native.pyi
@@ -300,6 +300,18 @@ class FloatConstant(Constant):
 class FloatPredicate(Enum):
     """A floating-point comparison predicate."""
 
+    def __richcmp__(self, other: Value, op: int) -> bool:
+        """
+        Compares this value to another value.
+        Only == and != are supported.
+
+        :param other: The other value.
+        :param op: The comparison operator.
+        :returns: The result of the comparison.
+        """
+        ...
+    def __hash__(self) -> int: ...
+
     FALSE: FloatPredicate
     OEQ: FloatPredicate
     OGT: FloatPredicate
@@ -414,6 +426,18 @@ class IntConstant(Constant):
 class IntPredicate(Enum):
     """An integer comparison predicate."""
 
+    def __richcmp__(self, other: Value, op: int) -> bool:
+        """
+        Compares this value to another value.
+        Only == and != are supported.
+
+        :param other: The other value.
+        :param op: The comparison operator.
+        :returns: The result of the comparison.
+        """
+        ...
+    def __hash__(self) -> int: ...
+
     EQ: IntPredicate
     NE: IntPredicate
     UGT: IntPredicate
@@ -444,6 +468,18 @@ class IntType(Type):
 class Linkage(Enum):
     """The linkage kind for a global value in a module."""
 
+    def __richcmp__(self, other: Value, op: int) -> bool:
+        """
+        Compares this value to another value.
+        Only == and != are supported.
+
+        :param other: The other value.
+        :param op: The comparison operator.
+        :returns: The result of the comparison.
+        """
+        ...
+    def __hash__(self) -> int: ...
+
     APPENDING: Linkage
     AVAILABLE_EXTERNALLY: Linkage
     COMMON: Linkage
@@ -553,6 +589,18 @@ class ModuleFlagBehavior(Enum):
 class Opcode(Enum):
     """An instruction opcode."""
 
+    def __richcmp__(self, other: Value, op: int) -> bool:
+        """
+        Compares this value to another value.
+        Only == and != are supported.
+
+        :param other: The other value.
+        :param op: The comparison operator.
+        :returns: The result of the comparison.
+        """
+        ...
+    def __hash__(self) -> int: ...
+
     ADD: Opcode
     ADDR_SPACE_CAST: Opcode
     ALLOCA: Opcode
diff --git a/pyqir/src/instructions.rs b/pyqir/src/instructions.rs
index fefecb56..919f530d 100644
--- a/pyqir/src/instructions.rs
+++ b/pyqir/src/instructions.rs
@@ -6,8 +6,13 @@
 use crate::values::{BasicBlock, Owner, Value};
 #[allow(clippy::wildcard_imports)]
 use llvm_sys::{core::*, prelude::*, LLVMIntPredicate, LLVMOpcode, LLVMRealPredicate};
-use pyo3::{conversion::ToPyObject, prelude::*};
-use std::{convert::Into, ptr::NonNull};
+use pyo3::{conversion::ToPyObject, prelude::*, pyclass::CompareOp, PyRef};
+use std::{
+    collections::hash_map::DefaultHasher,
+    convert::Into,
+    hash::{Hash, Hasher},
+    ptr::NonNull,
+};
 
 /// An instruction.
 #[pyclass(extends = Value, subclass)]
@@ -96,6 +101,7 @@ impl Instruction {
 
 /// An instruction opcode.
 #[pyclass]
+#[derive(PartialEq, Hash)]
 pub(crate) enum Opcode {
     #[pyo3(name = "ADD")]
     Add,
@@ -233,6 +239,25 @@ pub(crate) enum Opcode {
     ZExt,
 }
 
+#[pymethods]
+impl Opcode {
+    // In order to implement the comparison operators, we have to do
+    // it all in one impl of __richcmp__ for pyo3 to work.
+    fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyObject {
+        match op {
+            CompareOp::Eq => self.eq(other).into_py(py),
+            CompareOp::Ne => (!self.eq(other)).into_py(py),
+            _ => py.NotImplemented(),
+        }
+    }
+
+    fn __hash__(&self) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        self.hash(&mut hasher);
+        hasher.finish()
+    }
+}
+
 impl From<LLVMOpcode> for Opcode {
     fn from(opcode: LLVMOpcode) -> Self {
         match opcode {
@@ -378,7 +403,7 @@ impl ICmp {
 
 /// An integer comparison predicate.
 #[pyclass]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, PartialEq, Hash)]
 pub(crate) enum IntPredicate {
     #[pyo3(name = "EQ")]
     Eq,
@@ -402,6 +427,26 @@ pub(crate) enum IntPredicate {
     Sle,
 }
 
+#[allow(clippy::trivially_copy_pass_by_ref)]
+#[pymethods]
+impl IntPredicate {
+    // In order to implement the comparison operators, we have to do
+    // it all in one impl of __richcmp__ for pyo3 to work.
+    fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyObject {
+        match op {
+            CompareOp::Eq => self.eq(other).into_py(py),
+            CompareOp::Ne => (!self.eq(other)).into_py(py),
+            _ => py.NotImplemented(),
+        }
+    }
+
+    fn __hash__(&self) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        self.hash(&mut hasher);
+        hasher.finish()
+    }
+}
+
 impl From<LLVMIntPredicate> for IntPredicate {
     fn from(pred: LLVMIntPredicate) -> Self {
         match pred {
@@ -453,7 +498,7 @@ impl FCmp {
 
 /// A floating-point comparison predicate.
 #[pyclass]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, PartialEq, Hash)]
 pub(crate) enum FloatPredicate {
     #[pyo3(name = "FALSE")]
     False,
@@ -489,6 +534,26 @@ pub(crate) enum FloatPredicate {
     True,
 }
 
+#[pymethods]
+#[allow(clippy::trivially_copy_pass_by_ref)]
+impl FloatPredicate {
+    // In order to implement the comparison operators, we have to do
+    // it all in one impl of __richcmp__ for pyo3 to work.
+    fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyObject {
+        match op {
+            CompareOp::Eq => self.eq(other).into_py(py),
+            CompareOp::Ne => (!self.eq(other)).into_py(py),
+            _ => py.NotImplemented(),
+        }
+    }
+
+    fn __hash__(&self) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        self.hash(&mut hasher);
+        hasher.finish()
+    }
+}
+
 impl From<LLVMRealPredicate> for FloatPredicate {
     fn from(pred: LLVMRealPredicate) -> Self {
         match pred {
diff --git a/pyqir/src/module.rs b/pyqir/src/module.rs
index 8a00c0a8..6ce298a5 100644
--- a/pyqir/src/module.rs
+++ b/pyqir/src/module.rs
@@ -19,10 +19,12 @@ use llvm_sys::{
     ir_reader::LLVMParseIRInContext,
     LLVMLinkage, LLVMModule,
 };
-use pyo3::{exceptions::PyValueError, prelude::*, types::PyBytes};
+use pyo3::{exceptions::PyValueError, prelude::*, pyclass::CompareOp, types::PyBytes};
 use qirlib::module::FlagBehavior;
 use std::{
+    collections::hash_map::DefaultHasher,
     ffi::CString,
+    hash::{Hash, Hasher},
     ops::Deref,
     ptr::{self, NonNull},
     str,
@@ -289,7 +291,7 @@ impl PartialEq for Module {
 
 /// The linkage kind for a global value in a module.
 #[pyclass]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, PartialEq, Hash)]
 pub(crate) enum Linkage {
     #[pyo3(name = "APPENDING")]
     Appending,
@@ -315,6 +317,26 @@ pub(crate) enum Linkage {
     WeakOdr,
 }
 
+#[pymethods]
+#[allow(clippy::trivially_copy_pass_by_ref)]
+impl Linkage {
+    // In order to implement the comparison operators, we have to do
+    // it all in one impl of __richcmp__ for pyo3 to work.
+    fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyObject {
+        match op {
+            CompareOp::Eq => self.eq(other).into_py(py),
+            CompareOp::Ne => (!self.eq(other)).into_py(py),
+            _ => py.NotImplemented(),
+        }
+    }
+
+    fn __hash__(&self) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        self.hash(&mut hasher);
+        hasher.finish()
+    }
+}
+
 impl From<Linkage> for LLVMLinkage {
     fn from(linkage: Linkage) -> Self {
         match linkage {


Test patch:
diff --git a/pyqir/tests/test_value_operators.py b/pyqir/tests/test_value_operators.py
index d0b374c8..c7bbd87e 100644
--- a/pyqir/tests/test_value_operators.py
+++ b/pyqir/tests/test_value_operators.py
@@ -8,6 +8,10 @@
     Instruction,
     Module,
     Value,
+    Opcode,
+    Linkage,
+    FloatPredicate,
+    IntPredicate,
 )
 import pytest
 
@@ -26,6 +30,26 @@ def get_module() -> Module:
     return mod
 
 
+def get_opcodes(module: Optional[Module] = None) -> List[Opcode]:
+    mod: Module = module if module else get_module()
+    return list(map(lambda x: x.opcode, mod.functions[0].basic_blocks[0].instructions))
+
+
+def get_linkages() -> List[Linkage]:
+    # provide some linkages to use in testing
+    return [Linkage.INTERNAL, Linkage.AVAILABLE_EXTERNALLY]
+
+
+def get_float_predicates() -> List[FloatPredicate]:
+    # provide some float predicates to use in testing
+    return [FloatPredicate.FALSE, FloatPredicate.OEQ]
+
+
+def get_int_predicates() -> List[IntPredicate]:
+    # provide some int predicates to use in testing
+    return [IntPredicate.EQ, IntPredicate.NE]
+
+
 def get_instructions(module: Optional[Module] = None) -> List[Instruction]:
     mod: Module = module if module else get_module()
     return mod.functions[0].basic_blocks[0].instructions
@@ -168,3 +192,143 @@ def test_instruction_hash_is_same_when_from_same_module() -> None:
     first = get_instructions(mod)[1]
     second = get_instructions(mod)[1]
     assert hash(first) == hash(second)
+
+
+def test_opcode_equals_opcode() -> None:
+    op = get_opcodes()[0]
+    assert op == op
+
+
+def test_opcode__eq__opcode() -> None:
+    op = get_opcodes()[0]
+    assert op.__eq__(op)
+
+
+def test_opcode_is_self_opcode() -> None:
+    op = get_opcodes()[0]
+    assert op is op
+
+
+def test_opcode_not_equals_opcode() -> None:
+    ops = get_opcodes()
+    assert ops[0] != ops[1]
+
+
+def test_opcode__ne__opcode() -> None:
+    ops = get_opcodes()
+    assert ops[0].__ne__(ops[1])
+
+
+def test_opcode_is_not_other_opcode() -> None:
+    ops = get_opcodes()
+    assert ops[0] is not ops[1]
+
+
+def test_opcode_hash_equals_self_hash() -> None:
+    op = get_opcodes()[0]
+    assert hash(op) == hash(op)
+
+
+def test_linkage_equals_linkage() -> None:
+    linkage = get_linkages()[0]
+    assert linkage == linkage
+
+
+def test_linkage__eq__linkage() -> None:
+    linkage = get_linkages()[0]
+    assert linkage.__eq__(linkage)
+
+
+def test_linkage_is_self_linkage() -> None:
+    linkage = get_linkages()[0]
+    assert linkage is linkage
+
+
+def test_linkage_not_equals_linkage() -> None:
+    linkages = get_linkages()
+    assert linkages[0] != linkages[1]
+
+
+def test_linkage__ne__linkage() -> None:
+    linkages = get_linkages()
+    assert linkages[0].__ne__(linkages[1])
+
+
+def test_linkage_is_not_other_linkage() -> None:
+    linkages = get_linkages()
+    assert linkages[0] is not linkages[1]
+
+
+def test_linkage_hash_equals_self_hash() -> None:
+    linkage = get_linkages()[0]
+    assert hash(linkage) == hash(linkage)
+
+
+def test_float_predicate_equals_float_predicate() -> None:
+    pred = get_float_predicates()[0]
+    assert pred == pred
+
+
+def test_float_predicate__eq__float_predicate() -> None:
+    pred = get_float_predicates()[0]
+    assert pred.__eq__(pred)
+
+
+def test_float_predicate_is_self_float_predicate() -> None:
+    pred = get_float_predicates()[0]
+    assert pred is pred
+
+
+def test_float_predicate_not_equals_float_predicate() -> None:
+    preds = get_float_predicates()
+    assert preds[0] != preds[1]
+
+
+def test_float_predicate__ne__float_predicate() -> None:
+    preds = get_float_predicates()
+    assert preds[0].__ne__(preds[1])
+
+
+def test_float_predicate_is_not_other_float_predicate() -> None:
+    preds = get_float_predicates()
+    assert preds[0] is not preds[1]
+
+
+def test_float_predicate_hash_equals_self_hash() -> None:
+    pred = get_float_predicates()[0]
+    assert hash(pred) == hash(pred)
+
+
+def test_int_predicate_equals_int_predicate() -> None:
+    pred = get_int_predicates()[0]
+    assert pred == pred
+
+
+def test_int_predicate__eq__int_predicate() -> None:
+    pred = get_int_predicates()[0]
+    assert pred.__eq__(pred)
+
+
+def test_int_predicate_is_self_int_predicate() -> None:
+    pred = get_int_predicates()[0]
+    assert pred is pred
+
+
+def test_int_predicate_not_equals_int_predicate() -> None:
+    preds = get_int_predicates()
+    assert preds[0] != preds[1]
+
+
+def test_int_predicate__ne__int_predicate() -> None:
+    preds = get_int_predicates()
+    assert preds[0].__ne__(preds[1])
+
+
+def test_int_predicate_is_not_other_int_predicate() -> None:
+    preds = get_int_predicates()
+    assert preds[0] is not preds[1]
+
+
+def test_int_predicate_hash_equals_self_hash() -> None:
+    pred = get_int_predicates()[0]
+    assert hash(pred) == hash(pred)