fix: Make SumType::Unit(N) equal to SumType::General([(); N]) (#2250)

aborgna-q · web-flow · commit 0c68de869cb4 · 2025-05-23T12:26:28.000Z
`SumType` can represent the same sum of empty tuples in two different ways, but its equality implementation didn't consider them equal. This PR fixes this by manually implementing `PartialEq`, as well as `Hash` (to preserve correctness). We already do that on the python side https://github.com/CQCL/hugr/blob/940895cf3eae57bb6cae6d13f96bf4f065cb4d2b/hugr-py/src/hugr/tys.py#L311-L312 Ideally we'd make the general-sum-of-empty-tuples unrepresentable, but since `SumType` is an enum (with public member), we are not able to avoid arbitrary mutation.
diff --git a/hugr-core/src/types.rs b/hugr-core/src/types.rs
@@ -171,7 +171,7 @@ pub(crate) fn least_upper_bound(mut tags: impl Iterator<Item = TypeBound>) -> Ty
     .into_inner()
 }
 
-#[derive(Clone, PartialEq, Debug, Eq, Hash, Serialize, Deserialize)]
+#[derive(Clone, Debug, Eq, Serialize, Deserialize)]
 #[serde(tag = "s")]
 #[non_exhaustive]
 /// Representation of a Sum type.
@@ -186,6 +186,18 @@ pub enum SumType {
     General { rows: Vec<TypeRowRV> },
 }
 
+impl std::hash::Hash for SumType {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.variants().for_each(|v| v.hash(state));
+    }
+}
+
+impl PartialEq for SumType {
+    fn eq(&self, other: &Self) -> bool {
+        self.num_variants() == other.num_variants() && self.variants().eq(other.variants())
+    }
+}
+
 impl std::fmt::Display for SumType {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if self.num_variants() == 0 {
@@ -839,6 +851,7 @@ pub(crate) fn check_typevar_decl(
 
 #[cfg(test)]
 pub(crate) mod test {
+    use std::hash::{Hash, Hasher};
     use std::sync::Weak;
 
     use super::*;
@@ -917,20 +930,26 @@ pub(crate) mod test {
 
     #[test]
     fn sum_variants() {
-        {
-            let variants: Vec<TypeRowRV> = vec![
-                TypeRV::UNIT.into(),
-                vec![TypeRV::new_row_var_use(0, TypeBound::Any)].into(),
-            ];
-            let t = SumType::new(variants.clone());
-            assert_eq!(variants, t.variants().cloned().collect_vec());
-        }
-        {
-            assert_eq!(
-                vec![&TypeRV::EMPTY_TYPEROW; 3],
-                SumType::new_unary(3).variants().collect_vec()
-            );
-        }
+        let variants: Vec<TypeRowRV> = vec![
+            TypeRV::UNIT.into(),
+            vec![TypeRV::new_row_var_use(0, TypeBound::Any)].into(),
+        ];
+        let t = SumType::new(variants.clone());
+        assert_eq!(variants, t.variants().cloned().collect_vec());
+
+        let empty_rows = vec![TypeRV::EMPTY_TYPEROW; 3];
+        let sum_unary = SumType::new_unary(3);
+        let sum_general = SumType::General {
+            rows: empty_rows.clone(),
+        };
+        assert_eq!(&empty_rows, &sum_unary.variants().cloned().collect_vec());
+        assert_eq!(sum_general, sum_unary);
+
+        let mut hasher_general = std::hash::DefaultHasher::new();
+        sum_general.hash(&mut hasher_general);
+        let mut hasher_unary = std::hash::DefaultHasher::new();
+        sum_unary.hash(&mut hasher_unary);
+        assert_eq!(hasher_general.finish(), hasher_unary.finish());
     }
 
     pub(super) struct FnTransformer<T>(pub(super) T);
diff --git a/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@llvm14_2.snap b/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@llvm14_2.snap
@@ -5,12 +5,12 @@ expression: mod_str
 ; ModuleID = 'test_context'
 source_filename = "test_context"
 
-@sa.c.911aa16d.0 = constant { i64, [10 x i1] } { i64 10, [10 x i1] [i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false] }
+@sa.c.d2dddd66.0 = constant { i64, [10 x i1] } { i64 10, [10 x i1] [i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false] }
 
 define { i64, [0 x i1] }* @_hl.main.1() {
 alloca_block:
   br label %entry_block
 
 entry_block:                                      ; preds = %alloca_block
-  ret { i64, [0 x i1] }* bitcast ({ i64, [10 x i1] }* @sa.c.911aa16d.0 to { i64, [0 x i1] }*)
+  ret { i64, [0 x i1] }* bitcast ({ i64, [10 x i1] }* @sa.c.d2dddd66.0 to { i64, [0 x i1] }*)
 }
diff --git a/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@llvm14_3.snap b/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@llvm14_3.snap
@@ -5,12 +5,12 @@ expression: mod_str
 ; ModuleID = 'test_context'
 source_filename = "test_context"
 
-@sa.d.4c6da27.0 = constant { i64, [10 x { i1, i64 }] } { i64 10, [10 x { i1, i64 }] [{ i1, i64 } { i1 true, i64 0 }, { i1, i64 } { i1 true, i64 1 }, { i1, i64 } { i1 true, i64 2 }, { i1, i64 } { i1 true, i64 3 }, { i1, i64 } { i1 true, i64 4 }, { i1, i64 } { i1 true, i64 5 }, { i1, i64 } { i1 true, i64 6 }, { i1, i64 } { i1 true, i64 7 }, { i1, i64 } { i1 true, i64 8 }, { i1, i64 } { i1 true, i64 9 }] }
+@sa.d.eee08a59.0 = constant { i64, [10 x { i1, i64 }] } { i64 10, [10 x { i1, i64 }] [{ i1, i64 } { i1 true, i64 0 }, { i1, i64 } { i1 true, i64 1 }, { i1, i64 } { i1 true, i64 2 }, { i1, i64 } { i1 true, i64 3 }, { i1, i64 } { i1 true, i64 4 }, { i1, i64 } { i1 true, i64 5 }, { i1, i64 } { i1 true, i64 6 }, { i1, i64 } { i1 true, i64 7 }, { i1, i64 } { i1 true, i64 8 }, { i1, i64 } { i1 true, i64 9 }] }
 
 define { i64, [0 x { i1, i64 }] }* @_hl.main.1() {
 alloca_block:
   br label %entry_block
 
 entry_block:                                      ; preds = %alloca_block
-  ret { i64, [0 x { i1, i64 }] }* bitcast ({ i64, [10 x { i1, i64 }] }* @sa.d.4c6da27.0 to { i64, [0 x { i1, i64 }] }*)
+  ret { i64, [0 x { i1, i64 }] }* bitcast ({ i64, [10 x { i1, i64 }] }* @sa.d.eee08a59.0 to { i64, [0 x { i1, i64 }] }*)
 }
diff --git a/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@pre-mem2reg@llvm14_2.snap b/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@pre-mem2reg@llvm14_2.snap
@@ -5,7 +5,7 @@ expression: mod_str
 ; ModuleID = 'test_context'
 source_filename = "test_context"
 
-@sa.c.911aa16d.0 = constant { i64, [10 x i1] } { i64 10, [10 x i1] [i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false] }
+@sa.c.d2dddd66.0 = constant { i64, [10 x i1] } { i64 10, [10 x i1] [i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false, i1 true, i1 false] }
 
 define { i64, [0 x i1] }* @_hl.main.1() {
 alloca_block:
@@ -14,7 +14,7 @@ alloca_block:
   br label %entry_block
 
 entry_block:                                      ; preds = %alloca_block
-  store { i64, [0 x i1] }* bitcast ({ i64, [10 x i1] }* @sa.c.911aa16d.0 to { i64, [0 x i1] }*), { i64, [0 x i1] }** %"5_0", align 8
+  store { i64, [0 x i1] }* bitcast ({ i64, [10 x i1] }* @sa.c.d2dddd66.0 to { i64, [0 x i1] }*), { i64, [0 x i1] }** %"5_0", align 8
   %"5_01" = load { i64, [0 x i1] }*, { i64, [0 x i1] }** %"5_0", align 8
   store { i64, [0 x i1] }* %"5_01", { i64, [0 x i1] }** %"0", align 8
   %"02" = load { i64, [0 x i1] }*, { i64, [0 x i1] }** %"0", align 8
diff --git a/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@pre-mem2reg@llvm14_3.snap b/hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__static_array__test__static_array_const_codegen@pre-mem2reg@llvm14_3.snap
@@ -5,7 +5,7 @@ expression: mod_str
 ; ModuleID = 'test_context'
 source_filename = "test_context"
 
-@sa.d.4c6da27.0 = constant { i64, [10 x { i1, i64 }] } { i64 10, [10 x { i1, i64 }] [{ i1, i64 } { i1 true, i64 0 }, { i1, i64 } { i1 true, i64 1 }, { i1, i64 } { i1 true, i64 2 }, { i1, i64 } { i1 true, i64 3 }, { i1, i64 } { i1 true, i64 4 }, { i1, i64 } { i1 true, i64 5 }, { i1, i64 } { i1 true, i64 6 }, { i1, i64 } { i1 true, i64 7 }, { i1, i64 } { i1 true, i64 8 }, { i1, i64 } { i1 true, i64 9 }] }
+@sa.d.eee08a59.0 = constant { i64, [10 x { i1, i64 }] } { i64 10, [10 x { i1, i64 }] [{ i1, i64 } { i1 true, i64 0 }, { i1, i64 } { i1 true, i64 1 }, { i1, i64 } { i1 true, i64 2 }, { i1, i64 } { i1 true, i64 3 }, { i1, i64 } { i1 true, i64 4 }, { i1, i64 } { i1 true, i64 5 }, { i1, i64 } { i1 true, i64 6 }, { i1, i64 } { i1 true, i64 7 }, { i1, i64 } { i1 true, i64 8 }, { i1, i64 } { i1 true, i64 9 }] }
 
 define { i64, [0 x { i1, i64 }] }* @_hl.main.1() {
 alloca_block:
@@ -14,7 +14,7 @@ alloca_block:
   br label %entry_block
 
 entry_block:                                      ; preds = %alloca_block
-  store { i64, [0 x { i1, i64 }] }* bitcast ({ i64, [10 x { i1, i64 }] }* @sa.d.4c6da27.0 to { i64, [0 x { i1, i64 }] }*), { i64, [0 x { i1, i64 }] }** %"5_0", align 8
+  store { i64, [0 x { i1, i64 }] }* bitcast ({ i64, [10 x { i1, i64 }] }* @sa.d.eee08a59.0 to { i64, [0 x { i1, i64 }] }*), { i64, [0 x { i1, i64 }] }** %"5_0", align 8
   %"5_01" = load { i64, [0 x { i1, i64 }] }*, { i64, [0 x { i1, i64 }] }** %"5_0", align 8
   store { i64, [0 x { i1, i64 }] }* %"5_01", { i64, [0 x { i1, i64 }] }** %"0", align 8
   %"02" = load { i64, [0 x { i1, i64 }] }*, { i64, [0 x { i1, i64 }] }** %"0", align 8
