From 93fc44eade4c2b135a69c00f1ea03a4d5f32eecf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jul 2023 09:54:30 +0200 Subject: [PATCH] Remove horrible hack So, as the comment suspected, this did come back to bite me! Fortunately not in a bad way, as I found this comment pretty quick while figuring out what was wrong with the new code I wrote. But honestly, I'm surprised that it lasted this long (almost 10 months!). To my delight, removing this hack required no changes to any code! I take that as a sign that we're in a much better position today, architecture-wise, then we were back then. --- crates/fj-core/src/storage/handle.rs | 37 ---------------------------- 1 file changed, 37 deletions(-) diff --git a/crates/fj-core/src/storage/handle.rs b/crates/fj-core/src/storage/handle.rs index 98f4b25f2..15b5f8d18 100644 --- a/crates/fj-core/src/storage/handle.rs +++ b/crates/fj-core/src/storage/handle.rs @@ -219,61 +219,24 @@ impl Eq for HandleWrapper {} impl PartialEq for HandleWrapper { fn eq(&self, other: &Self) -> bool { - // The purpose of `HandleWrapper` is to provide equality (and other - // traits) for `Handle`s that would otherwise not have them. We use - // `Handle::id` to do this. This means, objects that are not identical - // are not equal. - // - // This is desirable for the most part, but it does become horribly - // inconvenient in test code. Tests, by design, create equal (but not - // identical) objects and compare them against objects produced by the - // code under test. Under such a use case, one would rather ignore any - // objects wrapped by `HandleWrapper`. - // - // And the following bit of code does just that. This might be a - // horrible hack that will comes back to bite us later (I honestly don't - // know), but it is certainly a very economical solution to this - // problem. - if cfg!(test) { - return true; - } - self.0.id().eq(&other.0.id()) } } impl Hash for HandleWrapper { fn hash(&self, state: &mut H) { - // This piece of code exists to keep this implementation in sync with - // the `PartialEq` implementation. See comment there. - if cfg!(test) { - return; - } - self.0.id().hash(state); } } impl Ord for HandleWrapper { fn cmp(&self, other: &Self) -> Ordering { - // This piece of code exists to keep this implementation in sync with - // the `PartialEq` implementation. See comment there. - if cfg!(test) { - return Ordering::Equal; - } - self.0.id().cmp(&other.0.id()) } } impl PartialOrd for HandleWrapper { fn partial_cmp(&self, other: &Self) -> Option { - // This piece of code exists to keep this implementation in sync with - // the `PartialEq` implementation. See comment there. - if cfg!(test) { - return Some(Ordering::Equal); - } - self.0.id().partial_cmp(&other.0.id()) } }