Merge pull request #40 from lofcz/feat-better-stable-hash-detection

lofcz · web-flow · commit b77fcb49b869 · 2026-05-03T16:44:52.000+02:00
improve stable hash detection
diff --git a/README.md b/README.md
@@ -270,6 +270,26 @@ public struct MyHandle
 
 This is the default behavior for system types like `System.Net.Http.Headers.HeaderDescriptor` to prevent breaking internal framework logic. Use this attribute if your custom structs behave similarly.
 
+### Stable Hash Opt-in
+
+Hash-based collections (`HashSet<T>`, `Dictionary<TKey, TValue>`, …) are cloned via a fast memberwise path whenever the key/element type's `GetHashCode` is known to be value-based. FastCloner figures this out automatically (and falls back to rebuilding the collection for identity-based hashes), but it can also be told explicitly with `[FastClonerStableHash]`:
+
+```csharp
+[FastClonerStableHash]
+public sealed class CompositeKey
+{
+    public int Major { get; }
+    public int Minor { get; }
+    public override int GetHashCode() => HashCode.Combine(Major, Minor);
+    public override bool Equals(object? obj)
+        => obj is CompositeKey other && other.Major == Major && other.Minor == Minor;
+}
+```
+
+Use it when `GetHashCode` is a pure function of the type's fields (or returns a constant). The attribute skips the runtime probe entirely, which is useful for types the probe cannot construct (abstract bases, types whose default-state `GetHashCode` would throw, etc.) and as a way to lock in the fast path explicitly.
+
+> **Do not** apply this attribute if `GetHashCode` depends on object identity (e.g. `RuntimeHelpers.GetHashCode(this)`, or hashes a per-instance handle that isn't preserved through cloning) — the cloned collection will be unable to find its own contents.
+
 ### Identity Preservation
 
 By default, FastCloner prioritizes performance by not tracking object identity during cloning. This means if the same object instance appears multiple times in your graph, each reference becomes a separate clone.
diff --git a/src/FastCloner.Benchmark/FastCloner.Benchmark.csproj b/src/FastCloner.Benchmark/FastCloner.Benchmark.csproj
@@ -10,7 +10,7 @@
 
     <ItemGroup>
       <PackageReference Include="AnyClone" Version="1.1.6" />
-      <PackageReference Include="AutoMapper" Version="16.1.0" />
+      <PackageReference Include="AutoMapper" Version="16.1.1" />
       <PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
       <PackageReference Include="DeepCloner" Version="0.10.4" />
       <PackageReference Include="DeepCopier" Version="1.0.4" />
diff --git a/src/FastCloner.Tests/FailureHypothesisTests.cs b/src/FastCloner.Tests/FailureHypothesisTests.cs
@@ -1,12 +1,155 @@
 using System.Collections.Concurrent;
+using System.Collections.Frozen;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 using System.Threading.Channels;
 using System.Threading.Tasks.Dataflow;
 using System.Threading.Tasks;
 
 namespace FastCloner.Tests;
 public class FailureHypothesisTests
 {
+    private sealed class IdentityHashedKey
+    {
+        public string Tag { get; set; } = "";
+        public override int GetHashCode() => RuntimeHelpers.GetHashCode(this);
+        public override bool Equals(object? obj) => ReferenceEquals(this, obj);
+    }
+
+    [Test]
+    public async Task HashSet_With_IdentityBased_OverriddenGetHashCode_Should_Be_Lookupable_After_Clone()
+    {
+        IdentityHashedKey item = new IdentityHashedKey { Tag = "a" };
+        HashSet<IdentityHashedKey> original = [item];
+
+        HashSet<IdentityHashedKey> clone = original.DeepClone();
+
+        await Assert.That(clone).IsNotSameReferenceAs(original);
+        await Assert.That(clone.Count).IsEqualTo(1);
+
+        IdentityHashedKey cloneItem = clone.Single();
+        await Assert.That(cloneItem).IsNotSameReferenceAs(item)
+            .Because("Element is a reference type and should be deep-cloned");
+
+        await Assert.That(clone.Contains(cloneItem)).IsTrue()
+            .Because("Looking up the actual element of the cloned set must succeed; " +
+                     "FastCloner copies the original identity-based hash into the cloned bucket, " +
+                     "while the cloned element has a new identity hash, so lookup misses.");
+    }
+
+    [Test]
+    public async Task Dictionary_With_IdentityBased_OverriddenGetHashCode_Key_Should_Be_Lookupable_After_Clone()
+    {
+        IdentityHashedKey key = new IdentityHashedKey { Tag = "k" };
+        Dictionary<IdentityHashedKey, int> original = new Dictionary<IdentityHashedKey, int> { [key] = 42 };
+
+        Dictionary<IdentityHashedKey, int> clone = original.DeepClone();
+
+        await Assert.That(clone).IsNotSameReferenceAs(original);
+        await Assert.That(clone.Count).IsEqualTo(1);
+
+        IdentityHashedKey cloneKey = clone.Keys.Single();
+        await Assert.That(cloneKey).IsNotSameReferenceAs(key);
+
+        await Assert.That(clone.TryGetValue(cloneKey, out int value)).IsTrue()
+            .Because("The cloned dictionary must be able to find its own key. " +
+                     "FastCloner stores stale identity hashes from the original key in the cloned bucket.");
+        await Assert.That(value).IsEqualTo(42);
+    }
+    
+    [FastClonerStableHash]
+    private sealed class ProbeUnfriendlyButStableKey
+    {
+        public string Tag { get; set; } = "";
+        public override int GetHashCode() => Tag.ToUpperInvariant().GetHashCode();
+        public override bool Equals(object? obj)
+            => obj is ProbeUnfriendlyButStableKey other
+               && string.Equals(Tag, other.Tag, StringComparison.OrdinalIgnoreCase);
+    }
+    
+    private sealed class ProbeUnfriendlyKeyNoAttribute
+    {
+        public string Tag { get; set; } = "";
+        public override int GetHashCode() => Tag.ToUpperInvariant().GetHashCode();
+        public override bool Equals(object? obj)
+            => obj is ProbeUnfriendlyKeyNoAttribute other
+               && string.Equals(Tag, other.Tag, StringComparison.OrdinalIgnoreCase);
+    }
+
+    [Test]
+    public async Task FastClonerStableHashAttribute_Marks_Type_As_Stable()
+    {
+        await Assert.That(Code.FastClonerSafeTypes.HasStableHashSemantics(typeof(ProbeUnfriendlyButStableKey)))
+            .IsTrue()
+            .Because("[FastClonerStableHash] must short-circuit the probe and declare stable semantics, " +
+                     "even when GetHashCode would throw on default-state instances.");
+        
+        await Assert.That(Code.FastClonerSafeTypes.HasStableHashSemantics(typeof(ProbeUnfriendlyKeyNoAttribute)))
+            .IsTrue()
+            .Because("The smarter probe substitutes string.Empty for stable-hash reference fields, " +
+                     "so an override of Tag.ToUpperInvariant().GetHashCode() no longer NREs and is " +
+                     "correctly classified as stable even without the [FastClonerStableHash] opt-in.");
+    }
+
+    [Test]
+    public async Task FastClonerStableHashAttribute_Allows_FastPath_With_Correct_Lookup()
+    {
+        ProbeUnfriendlyButStableKey key = new ProbeUnfriendlyButStableKey { Tag = "Alpha" };
+        HashSet<ProbeUnfriendlyButStableKey> original = [key];
+
+        HashSet<ProbeUnfriendlyButStableKey> clone = original.DeepClone();
+
+        await Assert.That(clone).IsNotSameReferenceAs(original);
+        await Assert.That(clone.Count).IsEqualTo(1);
+
+        ProbeUnfriendlyButStableKey cloneKey = clone.Single();
+        await Assert.That(cloneKey).IsNotSameReferenceAs(key);
+        await Assert.That(clone.Contains(cloneKey)).IsTrue();
+
+        // Equality is case-insensitive, so a fresh key with different casing must also resolve.
+        await Assert.That(clone.Contains(new ProbeUnfriendlyButStableKey { Tag = "alpha" })).IsTrue()
+            .Because("Hash is value-based on Tag (case-insensitive) and survives the clone unchanged.");
+    }
+    
+    private sealed class IdentityNode
+    {
+        public string Tag { get; set; } = "";
+    }
+
+    private struct IdentityDelegatingStruct : IEquatable<IdentityDelegatingStruct>
+    {
+        public IdentityNode? Node;
+
+        public override int GetHashCode() => Node is null ? 0 : Node.GetHashCode();
+
+        public bool Equals(IdentityDelegatingStruct other) => ReferenceEquals(Node, other.Node);
+
+        public override bool Equals(object? obj)
+            => obj is IdentityDelegatingStruct other && Equals(other);
+    }
+
+    [Test]
+    public async Task HashSet_Of_Struct_With_IdentityHashed_Reference_Field_Should_Be_Lookupable_After_Clone()
+    {
+        IdentityNode node = new IdentityNode { Tag = "n" };
+        IdentityDelegatingStruct entry = new IdentityDelegatingStruct { Node = node };
+
+        HashSet<IdentityDelegatingStruct> original = [entry];
+        HashSet<IdentityDelegatingStruct> clone = original.DeepClone();
+
+        await Assert.That(clone).IsNotSameReferenceAs(original);
+        await Assert.That(clone.Count).IsEqualTo(1);
+
+        IdentityDelegatingStruct cloneEntry = clone.Single();
+        await Assert.That(cloneEntry.Node).IsNotSameReferenceAs(node)
+            .Because("Reference field inside the struct must be deep-cloned to a fresh instance.");
+
+        await Assert.That(clone.Contains(cloneEntry)).IsTrue()
+            .Because("FastCloner treats every value type as having stable hash semantics, but this struct's " +
+                     "hash delegates to an identity-hashed reference field that gets a new identity on clone, " +
+                     "so the bucket-stored hash no longer matches the element's actual hash and lookup misses.");
+    }
+
     [Test]
     public async Task BufferBlock_Should_Be_Deep_Cloned_Independently()
     {
@@ -227,6 +370,47 @@ public async Task ReaderWriterLockSlim_Should_Be_Deep_Cloned_Independently()
             // Ignore cleanup errors
         }
     }
+    
+    [Test]
+    public async Task FrozenSet_Should_Be_Cloned_Without_StackOverflow()
+    {
+        FrozenSet<string> original = new[] { "alpha", "beta", "gamma" }.ToFrozenSet();
+
+        FrozenSet<string> clone = original.DeepClone();
+
+        await Assert.That(clone.Count).IsEqualTo(3)
+            .Because("All elements must survive the clone.");
+        await Assert.That(clone.Contains("alpha")).IsTrue();
+        await Assert.That(clone.Contains("beta")).IsTrue();
+        await Assert.That(clone.Contains("gamma")).IsTrue();
+        await Assert.That(clone.Contains("delta")).IsFalse();
+    }
+
+    private sealed class StructWrappedSelfReferenceContainer
+    {
+        public List<int> Payload { get; set; } = [1, 2, 3];
+        public StructBackRef BackRef;
+    }
+
+    private struct StructBackRef
+    {
+        public StructWrappedSelfReferenceContainer? Owner;
+    }
+
+    [Test]
+    public async Task Container_With_StructMediated_SelfReference_Should_Clone_Without_Overflow()
+    {
+        StructWrappedSelfReferenceContainer original = new StructWrappedSelfReferenceContainer();
+        original.BackRef = new StructBackRef { Owner = original };
+        await Assert.That(original.BackRef.Owner).IsSameReferenceAs(original);
+
+        StructWrappedSelfReferenceContainer clone = original.DeepClone();
+
+        await Assert.That(clone).IsNotSameReferenceAs(original);
+        await Assert.That(clone.Payload).IsEquivalentTo(new[] { 1, 2, 3 });
+        await Assert.That(clone.BackRef.Owner).IsSameReferenceAs(clone)
+            .Because("The cycle must be rebound to the clone, not left dangling at the original.");
+    }
 
     [Test]
     public async Task Task_Should_Not_Be_Deep_Cloned()
diff --git a/src/FastCloner.Tests/FastCloner.Tests.csproj b/src/FastCloner.Tests/FastCloner.Tests.csproj
@@ -15,15 +15,15 @@
   <ItemGroup>
     <PackageReference Include="FluentNHibernate" Version="3.4.1" />
     <PackageReference Include="FluentValidation" Version="12.1.1" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.3.0" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.5.1" />
     <PackageReference Include="NHibernate" Version="5.6.0" />
-    <PackageReference Include="ObjectDumper.NET" Version="4.4.10-pre" />
+    <PackageReference Include="ObjectDumper.NET" Version="4.4.13-pre" />
     <PackageReference Include="System.Data.SqlClient" Version="4.9.1" />
     <PackageReference Include="System.Data.SQLite.Core" Version="1.0.119" />
     <PackageReference Include="System.Drawing.Common" Version="10.0.1" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing" Version="1.1.3" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing" Version="1.1.3" />
-    <PackageReference Include="TUnit" Version="1.22.6" />
+    <PackageReference Include="TUnit" Version="1.43.11" />
   </ItemGroup>
   
   <PropertyGroup>
diff --git a/src/FastCloner/Code/FastClonerGenerator.cs b/src/FastCloner/Code/FastClonerGenerator.cs
@@ -270,8 +270,8 @@ private static FastClonerCache.TypeCloneMetadata BuildTypeMetadata(Type type)
 
             if (cacheEntry.Cloner is not null)
             {
-                return cacheEntry.CanUseNoTrackingState ? 
-                    cacheEntry.Cloner(obj, FastCloneState.GetSimpleState()) : 
+                return cacheEntry.CanUseNoTrackingState ?
+                    cacheEntry.Cloner(obj, FastCloneState.GetSimpleState()) :
                     CloneRootWithTrackedState(obj, cacheEntry.Cloner, cacheEntry.Metadata!);
             }
         }
@@ -371,7 +371,7 @@ private static bool TryCloneSafeArrayRoot<T>(T obj, Type runtimeType, out T? clo
         {
             return obj;
         }
-
+        
         if (metadata.CanSkipReferenceTracking ||
             (metadata is { CyclePolicy: FastClonerCache.CyclePolicy.None, HasBehaviorSensitiveMembers: false } && !rootType.IsValueType))
             return cloner(obj, FastCloneState.GetSimpleState());
@@ -469,11 +469,14 @@ private static bool CalculateCanHaveCycles(Type type)
         Type? payloadType = GetCollectionPayloadType(type);
         if (payloadType is not null)
         {
-            if (FastClonerSafeTypes.CanReturnSameObject(payloadType))
-                return false;
+            bool payloadTriviallyAcyclic =
+                FastClonerSafeTypes.CanReturnSameObject(payloadType) ||
+                (payloadType.IsValueType && !ValueTypeContainsReferenceFieldsCached(payloadType));
 
-            if (payloadType.IsValueType && !ValueTypeContainsReferenceFieldsCached(payloadType))
-                return false;
+            if (payloadTriviallyAcyclic)
+            {
+                return ContainerHasStructMediatedSelfReference(type);
+            }
         }
 
         Type[] fieldTypes = GetCycleFieldTypes(type);
@@ -492,6 +495,61 @@ private static bool CalculateCanHaveCycles(Type type)
 
         return false;
     }
+    
+    private static bool ContainerHasStructMediatedSelfReference(Type rootType)
+    {
+        HashSet<Type> visited = [];
+        return HasStructMediatedSelfReferenceCore(rootType, rootType, visited);
+    }
+
+    private static bool HasStructMediatedSelfReferenceCore(Type current, Type rootType, HashSet<Type> visited)
+    {
+        if (!visited.Add(current))
+            return false;
+
+        Type[] fieldTypes = GetCycleFieldTypes(current);
+        for (int i = 0; i < fieldTypes.Length; i++)
+        {
+            Type fieldType = fieldTypes[i];
+
+            if (FastClonerSafeTypes.CanReturnSameObject(fieldType))
+                continue;
+
+            if (fieldType.IsValueType)
+            {
+                if (HasStructMediatedSelfReferenceCore(fieldType, rootType, visited))
+                    return true;
+                continue;
+            }
+
+            if (FieldTypeCouldReferenceRoot(fieldType, rootType))
+                return true;
+        }
+
+        return false;
+    }
+
+    private static bool FieldTypeCouldReferenceRoot(Type fieldType, Type rootType)
+    {
+        if (fieldType == rootType ||
+            fieldType.IsAssignableFrom(rootType) ||
+            rootType.IsAssignableFrom(fieldType))
+        {
+            return true;
+        }
+
+        if (fieldType.IsArray)
+        {
+            Type? element = fieldType.GetElementType();
+            if (element is not null &&
+                (element == rootType || element.IsAssignableFrom(rootType) || rootType.IsAssignableFrom(element)))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
 
     private static bool ValueTypeContainsReferenceFields(Type valueType, HashSet<Type> visited)
     {
diff --git a/src/FastCloner/Code/FastClonerSafeTypes.cs b/src/FastCloner/Code/FastClonerSafeTypes.cs
diff --git a/src/FastCloner/Code/FastClonerStableHashAttribute.cs b/src/FastCloner/Code/FastClonerStableHashAttribute.cs
