Introduce HashableLiteral

The only reason `Literal` had to implement `Eq` and `Hash` was due to the SPV backend having to cache those in a `HashMap`. The `PartialEq` impl was error prone due to the match not being exhaustive.
gfx-rs · Apr 18, 2024 · ca729cc · ca729cc
1 parent d08a069
commit ca729cc
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 52 deletions.
diff --git a/naga/src/back/spv/mod.rs b/naga/src/back/spv/mod.rs
@@ -457,7 +457,7 @@ impl recyclable::Recyclable for CachedExpressions {
 
 #[derive(Eq, Hash, PartialEq)]
 enum CachedConstant {
-    Literal(crate::Literal),
+    Literal(crate::proc::HashableLiteral),
     Composite {
         ty: LookupType,
         constituent_ids: Vec<Word>,

diff --git a/naga/src/back/spv/writer.rs b/naga/src/back/spv/writer.rs
@@ -1150,7 +1150,7 @@ impl Writer {
     }
 
     pub(super) fn get_constant_scalar(&mut self, value: crate::Literal) -> Word {
-        let scalar = CachedConstant::Literal(value);
+        let scalar = CachedConstant::Literal(value.into());
         if let Some(&id) = self.cached_constants.get(&scalar) {
             return id;
         }

diff --git a/naga/src/lib.rs b/naga/src/lib.rs
@@ -871,7 +871,7 @@ pub enum TypeInner {
     BindingArray { base: Handle<Type>, size: ArraySize },
 }
 
-#[derive(Debug, Clone, Copy, PartialOrd)]
+#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)]
 #[cfg_attr(feature = "serialize", derive(Serialize))]
 #[cfg_attr(feature = "deserialize", derive(Deserialize))]
 #[cfg_attr(feature = "arbitrary", derive(Arbitrary))]

diff --git a/naga/src/proc/mod.rs b/naga/src/proc/mod.rs
@@ -153,56 +153,31 @@ impl super::Scalar {
     }
 }
 
-impl PartialEq for crate::Literal {
-    fn eq(&self, other: &Self) -> bool {
-        match (*self, *other) {
-            (Self::F64(a), Self::F64(b)) => a.to_bits() == b.to_bits(),
-            (Self::F32(a), Self::F32(b)) => a.to_bits() == b.to_bits(),
-            (Self::U32(a), Self::U32(b)) => a == b,
-            (Self::I32(a), Self::I32(b)) => a == b,
-            (Self::U64(a), Self::U64(b)) => a == b,
-            (Self::I64(a), Self::I64(b)) => a == b,
-            (Self::Bool(a), Self::Bool(b)) => a == b,
-            _ => false,
-        }
-    }
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum HashableLiteral {
+    F64(u64),
+    F32(u32),
+    U32(u32),
+    I32(i32),
+    U64(u64),
+    I64(i64),
+    Bool(bool),
+    AbstractInt(i64),
+    AbstractFloat(u64),
 }
-impl Eq for crate::Literal {}
-impl std::hash::Hash for crate::Literal {
-    fn hash<H: std::hash::Hasher>(&self, hasher: &mut H) {
-        match *self {
-            Self::F64(v) | Self::AbstractFloat(v) => {
-                hasher.write_u8(0);
-                v.to_bits().hash(hasher);
-            }
-            Self::F32(v) => {
-                hasher.write_u8(1);
-                v.to_bits().hash(hasher);
-            }
-            Self::U32(v) => {
-                hasher.write_u8(2);
-                v.hash(hasher);
-            }
-            Self::I32(v) => {
-                hasher.write_u8(3);
-                v.hash(hasher);
-            }
-            Self::Bool(v) => {
-                hasher.write_u8(4);
-                v.hash(hasher);
-            }
-            Self::I64(v) => {
-                hasher.write_u8(5);
-                v.hash(hasher);
-            }
-            Self::U64(v) => {
-                hasher.write_u8(6);
-                v.hash(hasher);
-            }
-            Self::AbstractInt(v) => {
-                hasher.write_u8(7);
-                v.hash(hasher);
-            }
+
+impl From<crate::Literal> for HashableLiteral {
+    fn from(l: crate::Literal) -> Self {
+        match l {
+            crate::Literal::F64(v) => Self::F64(v.to_bits()),
+            crate::Literal::F32(v) => Self::F32(v.to_bits()),
+            crate::Literal::U32(v) => Self::U32(v),
+            crate::Literal::I32(v) => Self::I32(v),
+            crate::Literal::U64(v) => Self::U64(v),
+            crate::Literal::I64(v) => Self::I64(v),
+            crate::Literal::Bool(v) => Self::Bool(v),
+            crate::Literal::AbstractInt(v) => Self::AbstractInt(v),
+            crate::Literal::AbstractFloat(v) => Self::AbstractFloat(v.to_bits()),
         }
     }
 }