From b5710ee07c0e041a4a2cabec3c2fa23b09f7d525 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:25:44 +0000 Subject: [PATCH] Hash the sanitized form, so Equals and GetHashCode agree [patch] Equals compares numerically, so an un-sanitized 2.50 - significand 250 at exponent -2 - equals a sanitized 2.5. GetHashCode hashed the raw fields, so the two produced different hashes, violating the Equals/GetHashCode contract: they land in different buckets of a Dictionary or HashSet and lookups miss. Such instances are reachable through CreateFromComponents(sanitize: false) and MakeCommonized/MakeCommonizedWithExponent, all internal and all kept for ktsu.SignificantNumber, and all of which return un-normalized values directly rather than only as transient internal state. Strip the trailing zeros before combining. Zero hashes as the sanitized zero whatever exponent it was stored at, and the exponent adjustment is unchecked because a hash must not throw. Fixes #101 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FLcTd3PKEVEnz3UcufaB4o --- .gitignore | 18 ++++++++ PreciseNumber.Test/PreciseNumberTests.cs | 58 ++++++++++++++++++++++++ PreciseNumber/PreciseNumber.cs | 26 ++++++++++- 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/PreciseNumber.Test/PreciseNumberTests.cs b/PreciseNumber.Test/PreciseNumberTests.cs index c200c19..29e3be7 100644 --- a/PreciseNumber.Test/PreciseNumberTests.cs +++ b/PreciseNumber.Test/PreciseNumberTests.cs @@ -1173,6 +1173,64 @@ public void TestGetHashCode() Assert.AreEqual(negativeOne.GetHashCode(), PreciseNumber.NegativeOne.GetHashCode()); } + [TestMethod] + public void TestGetHashCodeAgreesWithEqualsForUnsanitizedValues() + { + // Equals compares numerically, so an un-sanitized 2.50 equals a sanitized 2.5. The hash + // has to agree, or the two land in different buckets of a dictionary and lookups miss. + (int Exponent, int Significand)[] unsanitized = + [ + (-2, 250), // 2.50 + (-3, 2500), // 2.500 + (0, 100), // 100 + (1, 10), // 100 + (-1, -250), // -25.0 + (0, 0), // zero + (5, 0), // zero, at a non-zero exponent + ]; + + foreach ((int exponent, int significand) in unsanitized) + { + PreciseNumber raw = PreciseNumber.CreateFromComponents(exponent, significand, sanitize: false); + PreciseNumber sanitized = PreciseNumber.CreateFromComponents(exponent, significand); + + Assert.IsTrue(raw.Equals(sanitized), $"({exponent}, {significand}) should equal its sanitized form"); + Assert.AreEqual( + sanitized.GetHashCode(), + raw.GetHashCode(), + $"({exponent}, {significand}) is Equals-equal to its sanitized form, so it must hash alike"); + } + } + + [TestMethod] + public void TestGetHashCodeAgreesWithEqualsAfterCommonizing() + { + // MakeCommonized hands back un-normalized values directly, which is the other way a + // caller can reach an instance whose stored fields carry trailing zeros. + PreciseNumber twoAndAHalf = PreciseNumber.CreateFromComponents(-1, 25); + PreciseNumber thousandths = PreciseNumber.CreateFromComponents(-3, 1); + + (PreciseNumber commonized, PreciseNumber _) = PreciseNumber.MakeCommonized(twoAndAHalf, thousandths); + + Assert.IsTrue(commonized.Equals(twoAndAHalf)); + Assert.AreEqual(twoAndAHalf.GetHashCode(), commonized.GetHashCode()); + } + + [TestMethod] + public void TestUnsanitizedValuesShareADictionarySlotWithTheirSanitizedForm() + { + // The contract violation as a caller meets it: a lookup that silently misses. + Dictionary lookup = new() + { + [PreciseNumber.CreateFromComponents(-1, 25)] = "two and a half", + }; + + PreciseNumber raw = PreciseNumber.CreateFromComponents(-2, 250, sanitize: false); + + Assert.IsTrue(lookup.TryGetValue(raw, out string? found), "2.50 should find the entry stored under 2.5"); + Assert.AreEqual("two and a half", found); + } + [TestMethod] public void TestEqualsObjectSameInstance() { diff --git a/PreciseNumber/PreciseNumber.cs b/PreciseNumber/PreciseNumber.cs index e1741cb..d243705 100644 --- a/PreciseNumber/PreciseNumber.cs +++ b/PreciseNumber/PreciseNumber.cs @@ -450,7 +450,31 @@ public bool Equals(PreciseNumber other) => Equal(this, other); /// - public override int GetHashCode() => HashCode.Combine(Exponent, Significand); + /// + /// Hashes the sanitized form rather than the stored fields. + /// compares numerically, so an un-sanitized 2.50 - significand 250 at exponent -2, reachable + /// through and the commonizing + /// helpers - equals a sanitized 2.5 and has to hash alike, or the two land in different buckets + /// of a hash table and lookups silently miss. + /// + public override int GetHashCode() + { + if (Significand.IsZero) + { + // Every zero is equal, whatever exponent it happens to be stored at. + return HashCode.Combine(0, BigInteger.Zero); + } + + // The leading digit is non-zero, so at most SignificantDigits - 1 zeros can trail. + int trailingZeros = CountTrailingZeros(Significand, SignificantDigits - 1); + + // Unchecked because a hash must not throw. The sanitizing constructor does this addition + // checked, so an exponent that could overflow here cannot survive ordinary arithmetic + // anyway, and a wrapped value still hashes Equals-equal pairs to the same bucket. + return trailingZeros == 0 + ? HashCode.Combine(Exponent, Significand) + : HashCode.Combine(unchecked(Exponent + trailingZeros), Significand / Pow10(trailingZeros)); + } /// public override string ToString() => ToString(this, null, null);