fix #35

[lofcz]

No description provided.

[Copilot]

Pull request overview

This PR addresses issue #35 by making FastCloner’s set-cloning path handle non-generic concrete types that implement ISet<T> via an interface (where Type.GetGenericArguments() can be empty), and adds regression tests for that scenario.

Changes:

• Refactors IsSetType to use a shared GetSetInterface(Type) helper.
• Updates set processor generation to derive the element type from the implemented ISet<T> interface when the concrete type has no generic arguments.
• Adds tests covering cloning a non-generic ISet<string> implementation directly and as a nested member.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/FastCloner/Code/FastClonerExprGenerator.cs	Adds GetSetInterface helper and updates set-processing to avoid failures on non-generic set implementations.
src/FastCloner.Tests/CollectionTests.cs	Adds a non-generic ISet<string> implementation and regression tests validating deep cloning behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

elementType is chosen from type.GenericArguments()[0] when the concrete type is generic. That can be incorrect if the set element type comes from the ISet<T> interface but isn’t the first generic parameter of the concrete type (e.g., a MySet<TKey, TValue> : ISet<TValue>). Since setInterface is already computed, derive elementType from setInterface.GetGenericArguments()[0] (or otherwise map the interface’s T) to ensure correctness for all implementations.

Suggested change

	Type[] genericArguments = type.GenericArguments();
	Type elementType = genericArguments.Length > 0
	? genericArguments[0]
	: setInterface.GetGenericArguments()[0];
	Type elementType = setInterface.GetGenericArguments()[0];

[Copilot]

GenerateProcessMethod already branches into GenerateProcessSetMethod only when IsSetType(type) is true, and IsSetType uses GetSetInterface(type). GenerateProcessSetMethod then calls GetSetInterface(type) again and has a setInterface is null fallback that should be unreachable. Consider computing the set interface once (e.g., return it from the predicate or pass it into GenerateProcessSetMethod) to avoid the extra GetInterfaces() scan and dead branch.

Suggested change

	Type? setInterface = GetSetInterface(type);
	if (setInterface is null)
	return GenerateMemberwiseCloner(type, position);
	Type setInterface = GetSetInterface(type)!;

[github-actions]

Deep Clone Benchmarks

• OS: ubuntu-latest
• Generated (UTC): 2026-03-28 22:20:09

Current FastCloner vs DeepCloner

Benchmark	DeepCloner	FastCloner	Delta Time	DC Alloc	FC Alloc	Delta Alloc
SmallObject	82.77 ns	68.87 ns	-17% faster	184 B	48 B	-74% less
StringArray_1000	488.64 ns	525.06 ns	+7% slower	8,160 B	8,024 B	~same
FileSpec	492.03 ns	300.54 ns	-39% faster	920 B	416 B	-55% less
SmallObjectWithCollections	653.04 ns	380.64 ns	-42% faster	1,096 B	576 B	-47% less
DynamicWithDictionary	1,353.95 ns	1,012.39 ns	-25% faster	2,712 B	1,600 B	-41% less
MediumNestedObject	1,673.99 ns	1,086.87 ns	-35% faster	3,416 B	1,616 B	-53% less
DynamicWithNestedObject	1,925.46 ns	1,274.30 ns	-34% faster	3,560 B	1,776 B	-50% less
DynamicWithArray	5,407.30 ns	4,411.80 ns	-18% faster	8,800 B	2,744 B	-69% less
LargeEventDocument_10MB	72,822.21 ns	38,358.15 ns	-47% faster	129,792 B	49,824 B	-62% less
ObjectList_100	165,489.89 ns	108,863.36 ns	-34% faster	318,888 B	149,816 B	-53% less
ObjectDictionary_50	828,470.97 ns	180,160.95 ns	-78% faster	549,660 B	218,768 B	-60% less
LargeLogBatch_10MB	5,839,330.42 ns	3,595,455.59 ns	-38% faster	3,564,654 B	2,649,104 B	-26% less

FastCloner vs latest next baseline

• Baseline generated (UTC): 2026-03-10 04:17:08
• Regression thresholds: time > 5%, alloc > 5%

Status	Benchmark	Delta Time	Delta Alloc
🔴	DynamicWithArray	+8% slower	~same
⚪	DynamicWithDictionary	~same	~same
⚪	DynamicWithNestedObject	~same	~same
⚪	FileSpec	+5% slower	~same
⚪	LargeEventDocument_10MB	~same	~same
⚪	LargeLogBatch_10MB	~same	~same
⚪	MediumNestedObject	+4% slower	~same
⚪	ObjectDictionary_50	+5% slower	~same
🔴	ObjectList_100	+7% slower	~same
🔴	SmallObject	+7% slower	~same
⚪	SmallObjectWithCollections	+3% slower	~same
🔴	StringArray_1000	+20% slower	~same

Regressions

• DynamicWithArray: time +8% slower, alloc ~same
• ObjectList_100: time +7% slower, alloc ~same
• SmallObject: time +7% slower, alloc ~same
• StringArray_1000: time +20% slower, alloc ~same

Improvements

• none

Mixed changes

• none

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment claims TTag=List<int> forces the iterate-and-clone path by affecting stable hash semantics, but the set fast/slow path decision is based on the ISet element type (here string, which has stable hash semantics). Consider rewording to reflect the actual regression being tested: ensuring the ISet element type is derived from the implemented interface (not type.GetGenericArguments()[0]).

Suggested change

	// TTag=List<int> (mutable, no stable hash semantics) forces the iterate-and-clone
	// path rather than the memberwise fast path, exposing the wrong element type.
	// Regression test: generic type where the ISet<T> element type (string) is not
	// the first generic argument (TTag=List<int>). Ensures the element type is
	// derived from the implemented ISet<T> interface, not type.GetGenericArguments()[0].