Drop MemoHash's std Hash impl and route cache-identity hashing through CacheHash (#4209)

node-graph/graph-craft/src/document.rs

@@ -172,7 +172,7 @@ impl DocumentNode {
 }
 
 /// Represents the possible inputs to a node.
-#[derive(Debug, Clone, PartialEq, Hash, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, PartialEq, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 pub enum NodeInput {
 	/// A reference to another node in the same network from which this node can receive its input.
 	Node { node_id: NodeId, output_index: usize },

node-graph/graph-craft/src/proto.rs

@@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet};
 use std::fmt::Debug;
 use std::hash::Hash;
 
-#[derive(Debug, Default, PartialEq, Clone, Hash, Eq, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Default, PartialEq, Clone, Eq, serde::Serialize, serde::Deserialize)]
 /// A list of [`ProtoNode`]s, which is an intermediate step between the [`crate::document::NodeNetwork`] and the `BorrowTree` containing a single flattened network.
 pub struct ProtoNetwork {
 	// TODO: remove this since it seems to be unused?
@@ -90,7 +90,7 @@ impl PartialEq for ConstructionArgs {
 				use std::hash::Hasher;
 				let hash = |input: &Self| {
 					let mut hasher = rustc_hash::FxHasher::default();
-					input.hash(&mut hasher);
+					input.cache_hash(&mut hasher);
 					hasher.finish()
 				};
 				hash(self) == hash(other)
@@ -99,8 +99,8 @@ impl PartialEq for ConstructionArgs {
 	}
 }
 
-impl Hash for ConstructionArgs {
-	fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+impl CacheHash for ConstructionArgs {
+	fn cache_hash<H: std::hash::Hasher>(&self, state: &mut H) {
 		core::mem::discriminant(self).hash(state);
 		match self {
 			Self::Nodes(nodes) => {
@@ -108,7 +108,7 @@ impl Hash for ConstructionArgs {
 					node.hash(state);
 				}
 			}
-			Self::Value(value) => value.hash(state),
+			Self::Value(value) => value.cache_hash(state),
 			Self::Inline(inline) => inline.hash(state),
 		}
 	}
@@ -124,7 +124,7 @@ impl ConstructionArgs {
 	}
 }
 
-#[derive(Debug, Clone, PartialEq, Hash, Eq, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 /// A proto node is an intermediate step between the `DocumentNode` and the boxed struct that actually runs the node (found in the [`BorrowTree`]).
 /// At different stages in the compilation process, this struct will be transformed into a reduced (more restricted) form acting as a subset of its original form, but that restricted form is still valid in the earlier stage in the compilation process before it was transformed.
 pub struct ProtoNode {
@@ -157,7 +157,7 @@ impl ProtoNode {
 		let mut hasher = rustc_hash::FxHasher::default();
 
 		self.identifier.as_str().hash(&mut hasher);
-		self.construction_args.hash(&mut hasher);
+		self.construction_args.cache_hash(&mut hasher);
 		if self.skip_deduplication {
 			self.original_location.path.hash(&mut hasher);
 		}

node-graph/libraries/core-types/src/memo.rs

@@ -11,12 +11,34 @@ pub struct IORecord<I, O> {
 	pub output: O,
 }
 
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Debug)]
 pub struct MemoHash<T: CacheHash> {
 	hash: u64,
 	value: Arc<T>,
 }
 
+// Compare the value, not the cache `hash`: `CacheHash` is not guaranteed to be an equivalence relation,
+// so it can't back `eq`/`cmp`. Cache-identity hashing goes through `CacheHash`.
+impl<T: CacheHash + PartialEq> PartialEq for MemoHash<T> {
+	fn eq(&self, other: &Self) -> bool {
+		self.value == other.value
+	}
+}
+
+impl<T: CacheHash + Eq> Eq for MemoHash<T> {}
+
+impl<T: CacheHash + PartialOrd> PartialOrd for MemoHash<T> {
+	fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
+		self.value.partial_cmp(&other.value)
+	}
+}
+
+impl<T: CacheHash + Ord> Ord for MemoHash<T> {
+	fn cmp(&self, other: &Self) -> core::cmp::Ordering {
+		self.value.cmp(&other.value)
+	}
+}
+
 impl<'de, T: serde::Deserialize<'de> + CacheHash> serde::Deserialize<'de> for MemoHash<T> {
 	fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
 	where
@@ -67,12 +89,6 @@ impl<T: CacheHash> From<T> for MemoHash<T> {
 	}
 }
 
-impl<T: CacheHash> Hash for MemoHash<T> {
-	fn hash<H: Hasher>(&self, state: &mut H) {
-		self.hash.hash(state)
-	}
-}
-
 impl<T: CacheHash> CacheHash for MemoHash<T> {
 	fn cache_hash<H: Hasher>(&self, state: &mut H) {
 		self.hash.hash(state);

node-graph/libraries/core-types/src/transform.rs

@@ -31,7 +31,7 @@ pub trait Transform {
 	/// - `skew`: the horizontal shear coefficient (the raw matrix value, not an angle)
 	///
 	/// The original transform can be reconstructed as:
-	/// ```
+	/// ```ignore
 	/// DAffine2::from_scale_angle_translation(scale, rotation, translation) * DAffine2::from_cols_array(&[1., 0., skew, 1., 0., 0.])
 	/// ```
 	#[inline(always)]