Runtime: Relax Array.Copy to only require that source is assignment-compatible with destination

[VSadov]

Note that this only affects the case where the destination element type is a valuetype.
Since arrays of enums of the same underlying types are assignment compatible (from CLR point of view), it is easy to fall into a trap where one array would be assignable to another, but not "copyable" form the CopyTo prospective. That results in issues as in https://github.com/dotnet/corefx/issues/2241

It seems reasonable for the CopyTo to allow assignment-compatibile copying for enums and underlying types.

Please reactivate https://github.com/dotnet/corefx/issues/2241, when this is fixed (or rejected)

[karelz]

This is blocking another issue - is such change to CopyTo desirable?
cc: @AlexGhiondea @joperezr @stephentoub @jkotas

[jkotas]

Array.CopyTo is a thin wrapper over Array.Copy. I assume that this change would be actually made in Array.Copy.

The copy is allowed from array of enums to array of underlying primitive type today. Relaxing it to allow other direction sounds fine to me.

var a1 = new MyEnum[1];
var a2 = new int[1];

Array.Copy(a1, a2, 1); // works today
Array.Copy(a2, a1, 1); // fails today, we want to make it work

It makes it consistent with how similar cases are handled in other parts of the system, for example boxing/unboxing works in both directions:

Object b1 = MyEnum.Value;
int x = (int)b1; // works

Object b2 = 1;
MyEnum y = (MyEnum)b2; // works

[jamesqo]

@jkotas @VSadov, there was discussion in the other thread that a generic overload of Copy be added, so people have to be very explicit when they want to copy arrays that are assignment-compatible but not the same type. Does adding the generic overload with a new behavior, and preserving current behavior in the original sound like a good plan?

e.g. If we added a generic overload, these would get the new behavior:

Array.Copy((int[])(object)uints, 0, ints, 0, ints.Length);
IList<int> listOfInts = (int[])(object)uints;
listOfInts.CopyTo(ints, 0); // Now calls generic Copy under the hood

these would not:

Array.Copy(uints, 0, ints, 0, ints.Length);
uints.CopyTo(ints, 0);

[jkotas]

This is correcting behavior in a subtle corner cases. It is pretty unlikely that anybody is depending on the throwing behavior in this subtle corner case. I do not think that correcting behavior in a subtle corner case is a good argument for adding a new API.

[jamesqo]

@jkotas

It is pretty unlikely that anybody is depending on the throwing behavior in this subtle corner case.

Yes, that's true; I may have misworded. I was trying to point out that with this change, it would be easier to make errors; in addition to Copy not giving a compile-time error if the types are mismatched, it will now also silently copy over the data at runtime if the types turn out to be assignment-compatible. Having a generic method would force the programmer to do an explicit cast, to indicate that s/he really wants this.

[karelz]

silently copy over the data at runtime if the types turn out to be assignment-compatible

What is bad on that? I view it as goodness.

[jnm2]

One thing worth noting is that between arrays of reference types, or compatible enum/primitive types, Array.Copy can blit directly from one array's memory to the other's. However, for arrays of value types being copied to arrays of, say, IFormattable, every element will have to be boxed first. That may really not be goodness.

[jkotas]

every element will have to be boxed first. That may really not be goodness.

For better or worse, Array.Copy does many of these conversions today:

var a1 = new int[1];
var a2 = new IFormattable[1];
Array.Copy(a1, a2, 1); // works today

[jkotas]

Here is what needs to be done for this:

• Change ArrayNative::CanAssignArrayTypeNoGC to use GetVerifierCorElementType (https://github.com/dotnet/coreclr/blob/32f0f9721afb584b4a14d69135bea7ddc129f755/src/classlibnative/bcltype/arraynative.cpp#L320)
• Delete the one direction check for enums in CanAssignArrayTypeNoGC because of it is no longer needed (https://github.com/dotnet/coreclr/blob/32f0f9721afb584b4a14d69135bea7ddc129f755/src/classlibnative/bcltype/arraynative.cpp#L351)
• Same changes in ArrayNative::CanAssignArrayType
• Run all coreclr and corefx tests, and update them as necessary - this is where the actual work is

[jkotas]

https://github.com/dotnet/coreclr/pull/11266