diff --git a/.codacy.yml b/.codacy.yml new file mode 100644 index 0000000..3adcbb1 --- /dev/null +++ b/.codacy.yml @@ -0,0 +1,5 @@ +--- +exclude_paths: + # Vendored third-party comparison code (the FastDeepCloner library), kept verbatim so the tests + # can compare behaviour against it. It is not ours to refactor. + - "DeepCloner.Tests/Imported/**" diff --git a/CLAUDE.md b/CLAUDE.md index 8d247da..d8793c4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,8 +16,9 @@ licensing and community files are governed by the open source PanoramicData.Nuge ## Scope and boundaries -- Do not weaken `TreatWarningsAsErrors`, delete or skip tests to make a build pass, or bypass - CI/CD checks. +- Do not weaken `TreatWarningsAsErrors`. +- Do not delete or skip tests to make a build pass. +- Do not bypass CI/CD checks. - Do not commit secrets, credentials, or API tokens. - Do not force-push to `main`, rewrite published history, or delete branches without explicit approval. diff --git a/DeepCloner.Tests/ArraysSpec.cs b/DeepCloner.Tests/ArraysSpec.cs index 16abd2f..d8658b0 100644 --- a/DeepCloner.Tests/ArraysSpec.cs +++ b/DeepCloner.Tests/ArraysSpec.cs @@ -1,4 +1,5 @@ -#nullable disable +#pragma warning disable S1104 // Public fields are deliberate: these fixtures exist to test field cloning +#nullable disable using Xunit; using PanoramicData.DeepCloner; diff --git a/DeepCloner.Tests/ConstructorsSpec.cs b/DeepCloner.Tests/ConstructorsSpec.cs index 2bfdd81..821b013 100644 --- a/DeepCloner.Tests/ConstructorsSpec.cs +++ b/DeepCloner.Tests/ConstructorsSpec.cs @@ -1,4 +1,5 @@ -#nullable disable +#pragma warning disable S3877 // Equals throws deliberately: the test checks that cloning never calls it +#nullable disable using Xunit; using PanoramicData.DeepCloner; diff --git a/DeepCloner.Tests/CopyToObjectSpec.cs b/DeepCloner.Tests/CopyToObjectSpec.cs index afeb512..ff1ad7e 100644 --- a/DeepCloner.Tests/CopyToObjectSpec.cs +++ b/DeepCloner.Tests/CopyToObjectSpec.cs @@ -369,6 +369,35 @@ public void MultiDim_Array_Should_Be_Cloned(bool isDeep) Assert.Equal(1, arrTo.GetValue(0, 0)); } + public readonly record struct StructWithReference(int[] Items); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void TwoDim_Array_Of_Structs_With_References_Should_Be_Cloned(bool isDeep) + { + var arrFrom = new[,] + { + { new StructWithReference([1]), new StructWithReference([2]) }, + { new StructWithReference([3]), new StructWithReference([4]) } + }; + var arrTo = new StructWithReference[2, 2]; + + if (isDeep) + { + arrFrom.DeepCloneTo(arrTo); + } + else + { + arrFrom.ShallowCloneTo(arrTo); + } + + Assert.Equal([4], arrTo[1, 1].Items); + Assert.Equal([1], arrTo[0, 0].Items); + Assert.Equal([3], arrTo[1, 0].Items); + Assert.Equal(isDeep, !ReferenceEquals(arrFrom[1, 1].Items, arrTo[1, 1].Items)); + } + [Theory] [InlineData(false)] [InlineData(true)] diff --git a/DeepCloner.Tests/InheritanceSpec.cs b/DeepCloner.Tests/InheritanceSpec.cs index ccddca2..a3fa17b 100644 --- a/DeepCloner.Tests/InheritanceSpec.cs +++ b/DeepCloner.Tests/InheritanceSpec.cs @@ -1,4 +1,5 @@ -#nullable disable +#pragma warning disable S1104, S2357 // Public fields are deliberate: these fixtures exist to test field cloning +#nullable disable using Xunit; using PanoramicData.DeepCloner; diff --git a/DeepCloner.Tests/Objects/DoableStruct1.cs b/DeepCloner.Tests/Objects/DoableStruct1.cs index 924a5a8..b5d96ee 100644 --- a/DeepCloner.Tests/Objects/DoableStruct1.cs +++ b/DeepCloner.Tests/Objects/DoableStruct1.cs @@ -1,4 +1,5 @@ -namespace PanoramicData.DeepCloner.Test.Objects; +#pragma warning disable S1104 // Public fields are deliberate: these fixtures exist to test field cloning +namespace PanoramicData.DeepCloner.Test.Objects; public struct DoableStruct1 : IDoable, IEquatable { diff --git a/DeepCloner.Tests/ReadonlyStructSpec.cs b/DeepCloner.Tests/ReadonlyStructSpec.cs new file mode 100644 index 0000000..0e21a13 --- /dev/null +++ b/DeepCloner.Tests/ReadonlyStructSpec.cs @@ -0,0 +1,119 @@ +#nullable disable + +using Xunit; + +namespace PanoramicData.DeepCloner.Test; + +/// +/// A struct whose fields are all readonly (a readonly struct or readonly record struct) must be +/// deep cloned like any other struct: its reference-type fields must not be shared with the source. +/// +public class ReadonlyStructSpec() : BaseTest(true) +{ + public readonly record struct Holder(int[] Items); + + public sealed class Box + { + public int Value { get; set; } + } + + public readonly record struct BoxHolder(Box Box); + + public record struct MutableHolder + { + public int[] Items { get; set; } + } + + public sealed class HoldsReadonlyStruct + { + public Holder Holder { get; set; } + } + + [Fact] + public void Readonly_Struct_With_Array_Should_Be_Deep_Cloned() + { + var source = new Holder([1, 2, 3]); + + var clone = source.DeepClone(); + + Assert.Equal([1, 2, 3], clone.Items); + Assert.NotSame(source.Items, clone.Items); + } + + [Fact] + public void Readonly_Struct_With_Class_Should_Be_Deep_Cloned() + { + var source = new BoxHolder(new Box { Value = 7 }); + + var clone = source.DeepClone(); + + Assert.Equal(7, clone.Box.Value); + Assert.NotSame(source.Box, clone.Box); + } + + [Fact] + public void Readonly_Struct_Inside_Class_Should_Be_Deep_Cloned() + { + var source = new HoldsReadonlyStruct { Holder = new Holder([4, 5]) }; + + var clone = source.DeepClone(); + + Assert.Equal([4, 5], clone.Holder.Items); + Assert.NotSame(source.Holder.Items, clone.Holder.Items); + } + + [Fact] + public void One_Dim_Array_Of_Readonly_Structs_Should_Be_Deep_Cloned() + { + var source = new[] { new Holder([1]), new Holder([2]) }; + + var clone = source.DeepClone(); + + Assert.Equal([2], clone[1].Items); + Assert.NotSame(source[1].Items, clone[1].Items); + } + + [Fact] + public void Two_Dim_Array_Of_Readonly_Structs_Should_Be_Deep_Cloned() + { + var source = new[,] { { new Holder([1]), new Holder([2]) }, { new Holder([3]), new Holder([4]) } }; + + var clone = source.DeepClone(); + + Assert.Equal([4], clone[1, 1].Items); + Assert.NotSame(source[1, 1].Items, clone[1, 1].Items); + } + + [Fact] + public void Deep_Clone_To_Existing_Array_Of_Readonly_Structs_Should_Be_Deep() + { + var source = new[,] { { new Holder([1]), new Holder([2]) }, { new Holder([3]), new Holder([4]) } }; + var target = new Holder[2, 2]; + + source.DeepCloneTo(target); + + Assert.Equal([4], target[1, 1].Items); + Assert.NotSame(source[1, 1].Items, target[1, 1].Items); + } + + [Fact] + public void Shallow_Clone_Of_Readonly_Struct_Should_Share_References() + { + var source = new Holder([1, 2, 3]); + + var clone = source.ShallowClone(); + + Assert.Same(source.Items, clone.Items); + } + + [Fact] + public void Mutable_Struct_With_Array_Should_Still_Be_Deep_Cloned() + { + var source = new MutableHolder { Items = [1, 2, 3] }; + + var clone = source.DeepClone(); + + Assert.Equal([1, 2, 3], clone.Items); + Assert.NotSame(source.Items, clone.Items); + } +} diff --git a/DeepCloner.Tests/SimpleObjectSpec.cs b/DeepCloner.Tests/SimpleObjectSpec.cs index 3e96b67..33c1fe9 100644 --- a/DeepCloner.Tests/SimpleObjectSpec.cs +++ b/DeepCloner.Tests/SimpleObjectSpec.cs @@ -1,4 +1,5 @@ -#nullable disable +#pragma warning disable S1104, S2357 // Public fields are deliberate: these fixtures exist to test field cloning +#nullable disable using Xunit; using PanoramicData.DeepCloner.Test.Objects; diff --git a/DeepCloner/Helpers/ClonerToExprGenerator.cs b/DeepCloner/Helpers/ClonerToExprGenerator.cs index 696e984..c9a3449 100644 --- a/DeepCloner/Helpers/ClonerToExprGenerator.cs +++ b/DeepCloner/Helpers/ClonerToExprGenerator.cs @@ -13,7 +13,10 @@ internal static class ClonerToExprGenerator internal static object GenerateClonerInternal(Type realType, bool isDeepClone) { if (realType.IsValueType()) + { throw new InvalidOperationException("Operation is valid only for reference types"); + } + return GenerateProcessMethod(realType, isDeepClone); } @@ -28,91 +31,100 @@ private static object GenerateProcessMethod(Type type, bool isDeepClone) var expressionList = new List(); - ParameterExpression from = Expression.Parameter(methodType); - var fromLocal = from; + var from = Expression.Parameter(methodType); var to = Expression.Parameter(methodType); - var toLocal = to; var state = Expression.Parameter(typeof(DeepCloneState)); - // if (!type.IsValueType()) + var fromLocal = Expression.Variable(type); + var toLocal = Expression.Variable(type); + + // fromLocal = (T)from + expressionList.Add(Expression.Assign(fromLocal, Expression.Convert(from, type))); + expressionList.Add(Expression.Assign(toLocal, Expression.Convert(to, type))); + + if (isDeepClone) { - fromLocal = Expression.Variable(type); - toLocal = Expression.Variable(type); - // fromLocal = (T)from - expressionList.Add(Expression.Assign(fromLocal, Expression.Convert(from, type))); - expressionList.Add(Expression.Assign(toLocal, Expression.Convert(to, type))); + // added from -> to binding to ensure reference loop handling + // structs cannot loop here + // state.AddKnownRef(from, to) + expressionList.Add(Expression.Call(state, typeof(DeepCloneState).GetMethod("AddKnownRef"), from, to)); + } - if (isDeepClone) - { - // added from -> to binding to ensure reference loop handling - // structs cannot loop here - // state.AddKnownRef(from, to) - expressionList.Add(Expression.Call(state, typeof(DeepCloneState).GetMethod("AddKnownRef"), from, to)); - } + foreach (var fieldInfo in GetAllFields(type)) + { + expressionList.Add(GenerateFieldCopy(fieldInfo, fromLocal, toLocal, state, isDeepClone)); } - List fi = new List(); + expressionList.Add(Expression.Convert(toLocal, methodType)); + + var funcType = typeof(Func<,,,>).MakeGenericType(methodType, methodType, typeof(DeepCloneState), methodType); + + return Expression.Lambda(funcType, Expression.Block(new[] { fromLocal, toLocal }, expressionList), from, to, state).Compile(); + } + + private static List GetAllFields(Type type) + { + var fields = new List(); var tp = type; do { #if !NETCORE // don't do anything with this dark magic! - if (tp == typeof(ContextBoundObject)) break; + if (tp == typeof(ContextBoundObject)) + { + break; + } #else - if (tp.Name == "ContextBoundObject") break; + if (tp.Name == "ContextBoundObject") + { + break; + } #endif - fi.AddRange(tp.GetDeclaredFields()); + fields.AddRange(tp.GetDeclaredFields()); tp = tp.BaseType(); } while (tp != null); - foreach (var fieldInfo in fi) + return fields; + } + + private static Expression GenerateFieldCopy( + FieldInfo fieldInfo, + ParameterExpression fromLocal, + ParameterExpression toLocal, + ParameterExpression state, + bool isDeepClone) + { + if (!isDeepClone || DeepClonerSafeTypes.CanReturnSameObject(fieldInfo.FieldType)) { - if (isDeepClone && !DeepClonerSafeTypes.CanReturnSameObject(fieldInfo.FieldType)) - { - var methodInfo = fieldInfo.FieldType.IsValueType() - ? typeof(DeepClonerGenerator).GetPrivateStaticMethod("CloneStructInternal") - .MakeGenericMethod(fieldInfo.FieldType) - : typeof(DeepClonerGenerator).GetPrivateStaticMethod("CloneClassInternal"); - - var get = Expression.Field(fromLocal, fieldInfo); - - // toLocal.Field = Clone...Internal(fromLocal.Field) - var call = (Expression)Expression.Call(methodInfo, get, state); - if (!fieldInfo.FieldType.IsValueType()) - call = Expression.Convert(call, fieldInfo.FieldType); - - // should handle specially - // todo: think about optimization, but it rare case - if (fieldInfo.IsInitOnly) - { - // var setMethod = fieldInfo.GetType().GetMethod("SetValue", new[] { typeof(object), typeof(object) }); - // expressionList.Add(Expression.Call(Expression.Constant(fieldInfo), setMethod, toLocal, call)); - var setMethod = typeof(DeepClonerExprGenerator).GetPrivateStaticMethod("ForceSetField"); - expressionList.Add(Expression.Call(setMethod, Expression.Constant(fieldInfo), - Expression.Convert(toLocal, typeof(object)), Expression.Convert(call, typeof(object)))); - } - else - { - expressionList.Add(Expression.Assign(Expression.Field(toLocal, fieldInfo), call)); - } - } - else - { - expressionList.Add(Expression.Assign(Expression.Field(toLocal, fieldInfo), Expression.Field(fromLocal, fieldInfo))); - } + return Expression.Assign(Expression.Field(toLocal, fieldInfo), Expression.Field(fromLocal, fieldInfo)); } - expressionList.Add(Expression.Convert(toLocal, methodType)); + var methodInfo = fieldInfo.FieldType.IsValueType() + ? typeof(DeepClonerGenerator).GetPrivateStaticMethod("CloneStructInternal") + .MakeGenericMethod(fieldInfo.FieldType) + : typeof(DeepClonerGenerator).GetPrivateStaticMethod("CloneClassInternal"); - var funcType = typeof(Func<,,,>).MakeGenericType(methodType, methodType, typeof(DeepCloneState), methodType); + var get = Expression.Field(fromLocal, fieldInfo); - var blockParams = new List(); - if (from != fromLocal) blockParams.Add(fromLocal); - if (to != toLocal) blockParams.Add(toLocal); + // toLocal.Field = Clone...Internal(fromLocal.Field) + var call = (Expression)Expression.Call(methodInfo, get, state); + if (!fieldInfo.FieldType.IsValueType()) + { + call = Expression.Convert(call, fieldInfo.FieldType); + } - return Expression.Lambda(funcType, Expression.Block(blockParams, expressionList), from, to, state).Compile(); + // should handle specially + // todo: think about optimization, but it rare case + if (fieldInfo.IsInitOnly) + { + var setMethod = typeof(DeepClonerExprGenerator).GetPrivateStaticMethod("ForceSetField"); + return Expression.Call(setMethod, Expression.Constant(fieldInfo), + Expression.Convert(toLocal, typeof(object)), Expression.Convert(call, typeof(object))); + } + + return Expression.Assign(Expression.Field(toLocal, fieldInfo), call); } private static object GenerateProcessArrayMethod(Type type, bool isDeep) @@ -128,35 +140,48 @@ private static object GenerateProcessArrayMethod(Type type, bool isDeep) if (rank == 1 && type == elementType.MakeArrayType()) { - if (!isDeep) - { - var callS = Expression.Call( - typeof(ClonerToExprGenerator).GetPrivateStaticMethod("ShallowClone1DimArraySafeInternal") - .MakeGenericMethod(elementType), Expression.Convert(from, type), Expression.Convert(to, type)); - return Expression.Lambda(funcType, callS, from, to, state).Compile(); - } - else - { - var methodName = "Clone1DimArrayClassInternal"; - if (DeepClonerSafeTypes.CanReturnSameObject(elementType)) methodName = "Clone1DimArraySafeInternal"; - else if (elementType.IsValueType()) methodName = "Clone1DimArrayStructInternal"; - var methodInfo = typeof(ClonerToExprGenerator).GetPrivateStaticMethod(methodName).MakeGenericMethod(elementType); - var callS = Expression.Call(methodInfo, Expression.Convert(from, type), Expression.Convert(to, type), state); - return Expression.Lambda(funcType, callS, from, to, state).Compile(); - } + return GenerateProcessOneDimArrayMethod(type, elementType, isDeep, funcType, from, to, state); } - else + + // multidim or not zero-based arrays + var methodInfo = rank == 2 && type == elementType.MakeArrayType(2) + ? typeof(ClonerToExprGenerator).GetPrivateStaticMethod("Clone2DimArrayInternal").MakeGenericMethod(elementType) + : typeof(ClonerToExprGenerator).GetPrivateStaticMethod("CloneAbstractArrayInternal"); + + var callS = Expression.Call(methodInfo, Expression.Convert(from, type), Expression.Convert(to, type), state, Expression.Constant(isDeep)); + return Expression.Lambda(funcType, callS, from, to, state).Compile(); + } + + private static object GenerateProcessOneDimArrayMethod( + Type type, + Type elementType, + bool isDeep, + Type funcType, + ParameterExpression from, + ParameterExpression to, + ParameterExpression state) + { + if (!isDeep) { - // multidim or not zero-based arrays - MethodInfo methodInfo; - if (rank == 2 && type == elementType.MakeArrayType(2)) - methodInfo = typeof(ClonerToExprGenerator).GetPrivateStaticMethod("Clone2DimArrayInternal").MakeGenericMethod(elementType); - else - methodInfo = typeof(ClonerToExprGenerator).GetPrivateStaticMethod("CloneAbstractArrayInternal"); - - var callS = Expression.Call(methodInfo, Expression.Convert(from, type), Expression.Convert(to, type), state, Expression.Constant(isDeep)); + var callS = Expression.Call( + typeof(ClonerToExprGenerator).GetPrivateStaticMethod("ShallowClone1DimArraySafeInternal") + .MakeGenericMethod(elementType), Expression.Convert(from, type), Expression.Convert(to, type)); return Expression.Lambda(funcType, callS, from, to, state).Compile(); } + + var methodName = "Clone1DimArrayClassInternal"; + if (DeepClonerSafeTypes.CanReturnSameObject(elementType)) + { + methodName = "Clone1DimArraySafeInternal"; + } + else if (elementType.IsValueType()) + { + methodName = "Clone1DimArrayStructInternal"; + } + + var methodInfo = typeof(ClonerToExprGenerator).GetPrivateStaticMethod(methodName).MakeGenericMethod(elementType); + var callD = Expression.Call(methodInfo, Expression.Convert(from, type), Expression.Convert(to, type), state); + return Expression.Lambda(funcType, callD, from, to, state).Compile(); } // when we can't use code generation, we can use these methods @@ -179,12 +204,18 @@ internal static T[] Clone1DimArraySafeInternal(T[] objFrom, T[] objTo, DeepCl internal static T[] Clone1DimArrayStructInternal(T[] objFrom, T[] objTo, DeepCloneState state) { // not null from called method, but will check it anyway - if (objFrom == null || objTo == null) return null; + if (objFrom == null || objTo == null) + { + return null; + } + var l = Math.Min(objFrom.Length, objTo.Length); state.AddKnownRef(objFrom, objTo); var cloner = DeepClonerGenerator.GetClonerForValueType(); for (var i = 0; i < l; i++) + { objTo[i] = cloner(objTo[i], state); + } return objTo; } @@ -192,11 +223,17 @@ internal static T[] Clone1DimArrayStructInternal(T[] objFrom, T[] objTo, Deep internal static T[] Clone1DimArrayClassInternal(T[] objFrom, T[] objTo, DeepCloneState state) { // not null from called method, but will check it anyway - if (objFrom == null || objTo == null) return null; + if (objFrom == null || objTo == null) + { + return null; + } + var l = Math.Min(objFrom.Length, objTo.Length); state.AddKnownRef(objFrom, objTo); for (var i = 0; i < l; i++) + { objTo[i] = (T)DeepClonerGenerator.CloneClassInternal(objFrom[i], state); + } return objTo; } @@ -204,17 +241,20 @@ internal static T[] Clone1DimArrayClassInternal(T[] objFrom, T[] objTo, DeepC internal static T[,] Clone2DimArrayInternal(T[,] objFrom, T[,] objTo, DeepCloneState state, bool isDeep) { // not null from called method, but will check it anyway - if (objFrom == null || objTo == null) return null; - if (objFrom.GetLowerBound(0) != 0 || objFrom.GetLowerBound(1) != 0 - || objTo.GetLowerBound(0) != 0 || objTo.GetLowerBound(1) != 0) + if (objFrom == null || objTo == null) + { + return null; + } + + if (HasNonZeroLowerBound(objFrom) || HasNonZeroLowerBound(objTo)) + { return (T[,])CloneAbstractArrayInternal(objFrom, objTo, state, isDeep); + } var l1 = Math.Min(objFrom.GetLength(0), objTo.GetLength(0)); var l2 = Math.Min(objFrom.GetLength(1), objTo.GetLength(1)); state.AddKnownRef(objFrom, objTo); - if ((!isDeep || DeepClonerSafeTypes.CanReturnSameObject(typeof(T))) - && objFrom.GetLength(0) == objTo.GetLength(0) - && objFrom.GetLength(1) == objTo.GetLength(1)) + if ((!isDeep || DeepClonerSafeTypes.CanReturnSameObject(typeof(T))) && HaveSameShape(objFrom, objTo)) { Array.Copy(objFrom, objTo, objFrom.Length); return objTo; @@ -222,38 +262,76 @@ internal static T[] Clone1DimArrayClassInternal(T[] objFrom, T[] objTo, DeepC if (!isDeep) { - for (var i = 0; i < l1; i++) - for (var k = 0; k < l2; k++) - objTo[i, k] = objFrom[i, k]; - return objTo; + Copy2DimArray(objFrom, objTo, l1, l2); } - - if (typeof(T).IsValueType()) + else if (typeof(T).IsValueType()) { - var cloner = DeepClonerGenerator.GetClonerForValueType(); - for (var i = 0; i < l1; i++) - for (var k = 0; k < l2; k++) - objTo[i, k] = cloner(objFrom[i, k], state); + Clone2DimArrayStructs(objFrom, objTo, l1, l2, state); } else { - for (var i = 0; i < l1; i++) - for (var k = 0; k < l2; k++) - objTo[i, k] = (T)DeepClonerGenerator.CloneClassInternal(objFrom[i, k], state); + Clone2DimArrayClasses(objFrom, objTo, l1, l2, state); } return objTo; } + private static bool HasNonZeroLowerBound(Array array) + => array.GetLowerBound(0) != 0 || array.GetLowerBound(1) != 0; + + private static bool HaveSameShape(Array first, Array second) + => first.GetLength(0) == second.GetLength(0) && first.GetLength(1) == second.GetLength(1); + + private static void Copy2DimArray(T[,] objFrom, T[,] objTo, int l1, int l2) + { + for (var i = 0; i < l1; i++) + { + for (var k = 0; k < l2; k++) + { + objTo[i, k] = objFrom[i, k]; + } + } + } + + private static void Clone2DimArrayStructs(T[,] objFrom, T[,] objTo, int l1, int l2, DeepCloneState state) + { + var cloner = DeepClonerGenerator.GetClonerForValueType(); + for (var i = 0; i < l1; i++) + { + for (var k = 0; k < l2; k++) + { + objTo[i, k] = cloner(objFrom[i, k], state); + } + } + } + + private static void Clone2DimArrayClasses(T[,] objFrom, T[,] objTo, int l1, int l2, DeepCloneState state) + { + for (var i = 0; i < l1; i++) + { + for (var k = 0; k < l2; k++) + { + objTo[i, k] = (T)DeepClonerGenerator.CloneClassInternal(objFrom[i, k], state); + } + } + } + // rare cases, very slow cloning. currently it's ok internal static Array CloneAbstractArrayInternal(Array objFrom, Array objTo, DeepCloneState state, bool isDeep) { // not null from called method, but will check it anyway - if (objFrom == null || objTo == null) return null; + if (objFrom == null || objTo == null) + { + return null; + } + var rank = objFrom.Rank; if (objTo.Rank != rank) + { throw new InvalidOperationException("Invalid rank of target array"); + } + var lowerBoundsFrom = Enumerable.Range(0, rank).Select(objFrom.GetLowerBound).ToArray(); var lowerBoundsTo = Enumerable.Range(0, rank).Select(objTo.GetLowerBound).ToArray(); var lengths = Enumerable.Range(0, rank).Select(x => Math.Min(objFrom.GetLength(x), objTo.GetLength(x))).ToArray(); @@ -264,30 +342,40 @@ internal static Array CloneAbstractArrayInternal(Array objFrom, Array objTo, Dee // unable to copy any element if (lengths.Any(x => x == 0)) + { return objTo; + } - while (true) + do { - if (isDeep) - objTo.SetValue(DeepClonerGenerator.CloneClassInternal(objFrom.GetValue(idxesFrom), state), idxesTo); - else - objTo.SetValue(objFrom.GetValue(idxesFrom), idxesTo); - var ofs = rank - 1; - while (true) + var value = objFrom.GetValue(idxesFrom); + objTo.SetValue(isDeep ? DeepClonerGenerator.CloneClassInternal(value, state) : value, idxesTo); + } + while (AdvanceIndexes(idxesFrom, idxesTo, lowerBoundsFrom, lowerBoundsTo, lengths)); + + return objTo; + } + + /// + /// Moves both index vectors to the next element, odometer style. Returns false once every element has been visited. + /// + private static bool AdvanceIndexes(int[] idxesFrom, int[] idxesTo, int[] lowerBoundsFrom, int[] lowerBoundsTo, int[] lengths) + { + var ofs = idxesFrom.Length - 1; + while (ofs >= 0) + { + idxesFrom[ofs]++; + idxesTo[ofs]++; + if (idxesFrom[ofs] < lowerBoundsFrom[ofs] + lengths[ofs]) { - idxesFrom[ofs]++; - idxesTo[ofs]++; - if (idxesFrom[ofs] >= lowerBoundsFrom[ofs] + lengths[ofs]) - { - idxesFrom[ofs] = lowerBoundsFrom[ofs]; - idxesTo[ofs] = lowerBoundsTo[ofs]; - ofs--; - if (ofs < 0) return objTo; - } - else - break; + return true; } + + idxesFrom[ofs] = lowerBoundsFrom[ofs]; + idxesTo[ofs] = lowerBoundsTo[ofs]; + ofs--; } - } -} \ No newline at end of file + return false; + } +} diff --git a/DeepCloner/Helpers/DeepClonerExprGenerator.cs b/DeepCloner/Helpers/DeepClonerExprGenerator.cs index 62ffae6..cf96e10 100644 --- a/DeepCloner/Helpers/DeepClonerExprGenerator.cs +++ b/DeepCloner/Helpers/DeepClonerExprGenerator.cs @@ -225,21 +225,21 @@ private static void AddFieldCloneExpression(List expressionList, Fie { // fieldInfo.SetValue(toLocal, value): FieldInfo.SetValue is an INSTANCE method, so the // FieldInfo constant is the call target, not the first argument. - expressionList.Add(Expression.Call( + expressionList.Add(SetReadonlyField(toLocal, boxed => Expression.Call( fieldConstant, _fieldSetMethod!, - Expression.Convert(toLocal, typeof(object)), - Expression.Convert(call, typeof(object)))); + boxed, + Expression.Convert(call, typeof(object))))); } else { var setMethod = typeof(DeepClonerExprGenerator).GetPrivateStaticMethod("ForceSetField")!; // ForceSetField(fieldInfo, toLocal, value): a STATIC method, so there is no call target. - expressionList.Add(Expression.Call( + expressionList.Add(SetReadonlyField(toLocal, boxed => Expression.Call( setMethod, fieldConstant, - Expression.Convert(toLocal, typeof(object)), - Expression.Convert(call, typeof(object)))); + boxed, + Expression.Convert(call, typeof(object))))); } } else @@ -248,6 +248,26 @@ private static void AddFieldCloneExpression(List expressionList, Fie } } + /// + /// Builds the expression that writes a readonly field through reflection, which needs the target as an object. + /// Boxing a class just passes the reference, but boxing a struct copies it, so the write would land on the copy + /// and be thrown away. For a struct the target is therefore boxed once, written, and unboxed back. + /// + private static Expression SetReadonlyField(Expression toLocal, Func setField) + { + if (!toLocal.Type.IsValueType()) + { + return setField(Expression.Convert(toLocal, typeof(object))); + } + + var boxed = Expression.Variable(typeof(object)); + return Expression.Block( + [boxed], + Expression.Assign(boxed, Expression.Convert(toLocal, typeof(object))), + setField(boxed), + Expression.Assign(toLocal, Expression.Unbox(boxed, toLocal.Type))); + } + private static object GenerateProcessArrayMethod(Type type) { var elementType = type.GetElementType()!; diff --git a/DeepCloner/Helpers/ShallowClonerGenerator.cs b/DeepCloner/Helpers/ShallowClonerGenerator.cs index 113844d..dc50b85 100644 --- a/DeepCloner/Helpers/ShallowClonerGenerator.cs +++ b/DeepCloner/Helpers/ShallowClonerGenerator.cs @@ -22,10 +22,14 @@ public static T CloneObject(T obj) } if (ReferenceEquals(obj, null)) + { return (T)(object)null; + } if (DeepClonerSafeTypes.CanReturnSameObject(obj.GetType())) + { return obj; + } return (T)ShallowObjectCloner.CloneObject(obj); }