Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement Array.Initialize in C# #77336

Merged
merged 18 commits into from
Oct 25, 2022
Merged

Implement Array.Initialize in C# #77336

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8dcad5f
Implement Array.Initialize in C#
reflectronic Oct 22, 2022
cf4ede4
Add KeepAlive for good measure
reflectronic Oct 22, 2022
08220af
Reimplement according to feedback
reflectronic Oct 22, 2022
97f72db
Add Mono implementation of Initialize
reflectronic Oct 22, 2022
cd85e02
Fix test failure
reflectronic Oct 23, 2022
266378a
Fix for real this time
reflectronic Oct 23, 2022
2407b33
Sigh
reflectronic Oct 23, 2022
003c215
Address review feedback
reflectronic Oct 23, 2022
cb435ce
Rework according to feedback
reflectronic Oct 25, 2022
ad26d52
Implement for NativeAOT
reflectronic Oct 25, 2022
07a28d7
Add RequiresUnreferencedCode
reflectronic Oct 25, 2022
a945257
Needs to be in ref too
reflectronic Oct 25, 2022
ba27fd2
Update src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
jkotas Oct 25, 2022
e1d6ec9
Update src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.…
jkotas Oct 25, 2022
4b5a4ec
Update src/mono/System.Private.CoreLib/src/System/Array.Mono.cs
jkotas Oct 25, 2022
7988ae3
Revert RequiresUnreferenceCode
jkotas Oct 25, 2022
b0d6af6
Use unboxing entrypoint
jkotas Oct 25, 2022
be576ad
Merge branch 'main' into array-initialize
jkotas Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,43 @@ private unsafe bool IsValueOfElementType(object value)
// if this is an array of value classes and that value class has a default constructor
// then this calls this default constructor on every element in the value class array.
// otherwise this is a no-op. Generally this method is called automatically by the compiler
public unsafe void Initialize()
Copy link
Member

@jkotas jkotas Oct 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dotnet/ilc-contrib Should Array.Initialize be marked with RequiresUnreferenceCode, or is the value type default constructor the special-cased by the trimmer and always preserved?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should:

var b = new ElementType[1];
b.Initialize();

struct ElementType
{
    public ElementType()
    {
        Console.WriteLine(".ctor called");
    }
}

dotnet run prints out ".ctor called".
But trimmed the app doesn't print out anything.

Related question - what should default do in this case, for example:

var c = new ElementType[1];
c[0] = default; // Should this call the default .ctor?

Running it seems like it will NOT call the .ctor - but what does it do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes for AOT - published as Native AOT the app above also doesn't print out anything.

Copy link
Contributor

@teo-tsirpanis teo-tsirpanis Oct 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related question - what should default do in this case, for example:

According to the specification, default ignores the parameterless constructor and generates a zeroed instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, adding RequiresUnreferencedCode on Array.Initialize has a ripple effect. It introduces warnings in situations where array type is passed into DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All).

I guess it will need more careful thought.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #77426

reflectronic marked this conversation as resolved.
Show resolved Hide resolved
{
MethodTable* pArrayMT = RuntimeHelpers.GetMethodTable(this);
var thElem = pArrayMT->GetArrayElementTypeHandle();
reflectronic marked this conversation as resolved.
Show resolved Hide resolved
if (thElem.IsTypeDesc)
{
return;
}

var pElemMT = thElem.AsMethodTable();
if (!pElemMT->HasDefaultConstructor || !pElemMT->IsValueType)
{
return;
}

ref byte arrayRef = ref MemoryMarshal.GetArrayDataReference(this);
delegate*<ref byte, MethodTable*, void> ctorFtn = GetConstructorSlot(pElemMT);

nuint offset = 0;
for (var i = 0; i < Length; i++)
{
// Since GetConstructorSlot() is not idempotent and may have returned
reflectronic marked this conversation as resolved.
Show resolved Hide resolved
// a non-optimal entry-point the first time round.
if (i == 1)
{
ctorFtn = GetConstructorSlot(pElemMT);
}

ctorFtn(ref Unsafe.Add(ref arrayRef, offset), pElemMT);
offset += pArrayMT->ComponentSize;
}

GC.KeepAlive(this);
reflectronic marked this conversation as resolved.
Show resolved Hide resolved
}

[MethodImpl(MethodImplOptions.InternalCall)]
public extern void Initialize();
private static extern unsafe delegate*<ref byte, MethodTable*, void> GetConstructorSlot(MethodTable* pMT);
}

#pragma warning disable CA1822 // Mark members as static
Expand Down
11 changes: 11 additions & 0 deletions ...eclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,9 @@ internal unsafe struct MethodTable
[FieldOffset(InterfaceMapOffset)]
public MethodTable** InterfaceMap;

// WFLAGS_LOW_ENUM
private const uint enum_flag_HasDefaultCtor = 0x00000200;

// WFLAGS_HIGH_ENUM
private const uint enum_flag_ContainsPointers = 0x01000000;
private const uint enum_flag_HasComponentSize = 0x80000000;
Expand Down Expand Up @@ -560,6 +563,14 @@ public bool HasTypeEquivalence
}
}

public bool HasDefaultConstructor
{
get
{
return ((HasComponentSize ? 0 : Flags) & enum_flag_HasDefaultCtor) != 0;
}
}

public bool IsMultiDimensionalArray
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
108 changes: 4 additions & 104 deletions src/coreclr/classlibnative/bcltype/arraynative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,117 +28,17 @@ FCIMPL1(INT32, ArrayNative::GetCorElementTypeOfElementType, ArrayBase* arrayUNSA
}
FCIMPLEND

// array is GC protected by caller
void ArrayInitializeWorker(ARRAYBASEREF * arrayRef,
MethodTable* pArrayMT,
MethodTable* pElemMT)
{
STATIC_CONTRACT_MODE_COOPERATIVE;

// Ensure that the array element type is fully loaded before executing its code
pElemMT->EnsureInstanceActive();
reflectronic marked this conversation as resolved.
Show resolved Hide resolved

//can not use contract here because of SEH
_ASSERTE(IsProtectedByGCFrame (arrayRef));

SIZE_T offset = ArrayBase::GetDataPtrOffset(pArrayMT);
SIZE_T size = pArrayMT->GetComponentSize();
SIZE_T cElements = (*arrayRef)->GetNumComponents();

MethodTable * pCanonMT = pElemMT->GetCanonicalMethodTable();
WORD slot = pCanonMT->GetDefaultConstructorSlot();

PCODE ctorFtn = pCanonMT->GetSlot(slot);

#if defined(TARGET_X86) && !defined(TARGET_UNIX)
BEGIN_CALL_TO_MANAGED();


for (SIZE_T i = 0; i < cElements; i++)
{
// Since GetSlot() is not idempotent and may have returned
// a non-optimal entry-point the first time round.
if (i == 1)
{
ctorFtn = pCanonMT->GetSlot(slot);
}

BYTE* thisPtr = (((BYTE*) OBJECTREFToObject (*arrayRef)) + offset);

#ifdef _DEBUG
__asm {
mov ECX, thisPtr
mov EDX, pElemMT // Instantiation argument if the type is generic
call [ctorFtn]
nop // Mark the fact that we can call managed code
}
#else // _DEBUG
typedef void (__fastcall * CtorFtnType)(BYTE*, BYTE*);
(*(CtorFtnType)ctorFtn)(thisPtr, (BYTE*)pElemMT);
#endif // _DEBUG

offset += size;
}

END_CALL_TO_MANAGED();
#else // TARGET_X86 && !TARGET_UNIX
//
// This is quite a bit slower, but it is portable.
//

for (SIZE_T i =0; i < cElements; i++)
{
// Since GetSlot() is not idempotent and may have returned
// a non-optimal entry-point the first time round.
if (i == 1)
{
ctorFtn = pCanonMT->GetSlot(slot);
}

BYTE* thisPtr = (((BYTE*) OBJECTREFToObject (*arrayRef)) + offset);

PREPARE_NONVIRTUAL_CALLSITE_USING_CODE(ctorFtn);
DECLARE_ARGHOLDER_ARRAY(args, 2);
args[ARGNUM_0] = PTR_TO_ARGHOLDER(thisPtr);
args[ARGNUM_1] = PTR_TO_ARGHOLDER(pElemMT); // Instantiation argument if the type is generic
CALL_MANAGED_METHOD_NORET(args);

offset += size;
}
#endif // !TARGET_X86 || TARGET_UNIX
}


FCIMPL1(void, ArrayNative::Initialize, ArrayBase* array)
FCIMPL1(PCODE, ArrayNative::GetConstructorSlot, MethodTable* pMT)
{
FCALL_CONTRACT;

if (array == NULL)
{
FCThrowVoid(kNullReferenceException);
}


MethodTable* pArrayMT = array->GetMethodTable();

TypeHandle thElem = pArrayMT->GetArrayElementTypeHandle();
if (thElem.IsTypeDesc())
return;

MethodTable * pElemMT = thElem.AsMethodTable();
if (!pElemMT->HasDefaultConstructor() || !pElemMT->IsValueType())
return;

ARRAYBASEREF arrayRef (array);
HELPER_METHOD_FRAME_BEGIN_1(arrayRef);

ArrayInitializeWorker(&arrayRef, pArrayMT, pElemMT);
MethodTable* pCanonMT = pMT->GetCanonicalMethodTable();
WORD slot = pCanonMT->GetDefaultConstructorSlot();
reflectronic marked this conversation as resolved.
Show resolved Hide resolved

HELPER_METHOD_FRAME_END();
return pCanonMT->GetSlot(slot);
}
FCIMPLEND


// Returns whether you can directly copy an array of srcType into destType.
FCIMPL2(FC_BOOL_RET, ArrayNative::IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/classlibnative/bcltype/arraynative.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ArrayNative
public:
static FCDECL1(INT32, GetCorElementTypeOfElementType, ArrayBase* arrayUNSAFE);

static FCDECL1(void, Initialize, ArrayBase* pArray);
static FCDECL1(PCODE, GetConstructorSlot, MethodTable* pMT);

static FCDECL2(FC_BOOL_RET, IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst);
static FCDECL5(void, CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase* pDst, INT32 iDstIndex, INT32 iLength);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/ecalllist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ FCFuncEnd()

FCFuncStart(gArrayFuncs)
FCFuncElement("GetCorElementTypeOfElementType", ArrayNative::GetCorElementTypeOfElementType)
FCFuncElement("Initialize", ArrayNative::Initialize)
FCFuncElement("GetConstructorSlot", ArrayNative::GetConstructorSlot)
FCFuncElement("IsSimpleCopy", ArrayNative::IsSimpleCopy)
FCFuncElement("CopySlow", ArrayNative::CopySlow)
FCFuncElement("InternalCreate", ArrayNative::CreateInstance)
Expand Down
52 changes: 37 additions & 15 deletions src/libraries/System.Runtime/tests/System/ArrayTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1614,7 +1614,7 @@ public static IEnumerable<object[]> Copy_UnreliableConversion_CantPerform_TestDa
yield return new object[] { new object[] { "1" }, new int[1] };

IEquatable<int>[] interfaceArray1 = new IEquatable<int>[10] { 0, 0, 0, 0, new NotInt32(), 0, 0, 0, 0, 0 };
yield return new object[] { interfaceArray1, new int[10]};
yield return new object[] { interfaceArray1, new int[10] };

IEquatable<int>[] interfaceArray2 = new IEquatable<int>[10] { 0, 0, 0, 0, new NotInt32(), 0, 0, 0, 0, 0 };
yield return new object[] { interfaceArray2, new int[10] };
Expand Down Expand Up @@ -3178,7 +3178,7 @@ public static IEnumerable<object[]> Reverse_TestData()

// Char
yield return new object[] { new char[] { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '1', '2', '3', '4', '5', '6', '7' }, 0, 33, new char[] { '7', '6', '5', '4', '3', '2', '1', 'z', 'y', 'x', 'w', 'v', 'u', 't', 's', 'r', 'q', 'p', 'o', 'n', 'm', 'l', 'k', 'j', 'i', 'h', 'g', 'f', 'e', 'd', 'c', 'b', 'a' } };
yield return new object[] { new char[] { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q' }, 0, 17, new char[] { 'q', 'p', 'o', 'n', 'm', 'l', 'k', 'j', 'i', 'h', 'g', 'f', 'e', 'd', 'c', 'b', 'a' } };
yield return new object[] { new char[] { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q' }, 0, 17, new char[] { 'q', 'p', 'o', 'n', 'm', 'l', 'k', 'j', 'i', 'h', 'g', 'f', 'e', 'd', 'c', 'b', 'a' } };
yield return new object[] { new char[] { '1', '2', '3', '4', '5' }, 2, 3, new char[] { '1', '2', '5', '4', '3' } };
yield return new object[] { new char[] { '1', '2', '3', '4', '5' }, 0, 0, new char[] { '1', '2', '3', '4', '5' } };
yield return new object[] { new char[] { '1', '2', '3', '4', '5' }, 5, 0, new char[] { '1', '2', '3', '4', '5' } };
Expand Down Expand Up @@ -3225,8 +3225,8 @@ public static IEnumerable<object[]> Reverse_TestData()
var enumArray = new Int32Enum[] { Int32Enum.Case1, Int32Enum.Case2, Int32Enum.Case3, Int32Enum.Case1 };
yield return new object[] { enumArray, 0, 4, new Int32Enum[] { Int32Enum.Case1, Int32Enum.Case3, Int32Enum.Case2, Int32Enum.Case1 } };
yield return new object[] { enumArray, 2, 2, new Int32Enum[] { Int32Enum.Case1, Int32Enum.Case2, Int32Enum.Case1, Int32Enum.Case3 } };
yield return new object[] { enumArray, 0, 0, enumArray};
yield return new object[] { enumArray, 4, 0, enumArray};
yield return new object[] { enumArray, 0, 0, enumArray };
yield return new object[] { enumArray, 4, 0, enumArray };

// ValueType array
ComparableValueType[] valueTypeArray = new ComparableValueType[] { new ComparableValueType(0), new ComparableValueType(1) };
Expand Down Expand Up @@ -3331,18 +3331,18 @@ public static IEnumerable<object[]> Sort_Array_TestData()
expectedPartial.SetValue(2, 3);
expectedPartial.SetValue(4, 4);

yield return new object[] { actual, 1, 4, null, expectedFull };
yield return new object[] { actual, 1, 4, new IntegerComparer(), expectedFull };
yield return new object[] { actual, 2, 2, null, expectedPartial };
yield return new object[] { actual, 2, 2, new IntegerComparer(), expectedPartial };
yield return new object[] { actual, 1, 0, null, actual };
yield return new object[] { actual, 1, 0, new IntegerComparer(), actual };
yield return new object[] { actual, 1, 4, null, expectedFull };
yield return new object[] { actual, 1, 4, new IntegerComparer(), expectedFull };
yield return new object[] { actual, 2, 2, null, expectedPartial };
yield return new object[] { actual, 2, 2, new IntegerComparer(), expectedPartial };
yield return new object[] { actual, 1, 0, null, actual };
yield return new object[] { actual, 1, 0, new IntegerComparer(), actual };
}
}

public static IEnumerable<object[]> Sort_SZArray_TestData()
{
// Int
// Int
yield return new object[] { new int[0], 0, 0, new IntegerComparer(), new int[0] };
yield return new object[] { new int[] { 5 }, 0, 1, new IntegerComparer(), new int[] { 5 } };
yield return new object[] { new int[] { 5, 2 }, 0, 2, new IntegerComparer(), new int[] { 2, 5 } };
Expand All @@ -3359,17 +3359,17 @@ public static IEnumerable<object[]> Sort_SZArray_TestData()
yield return new object[] { new int[] { 4, 3, 2 }, 0, 3, new IntegerComparer(), new int[] { 2, 3, 4 } };
yield return new object[] { new int[] { 4, 3, 2 }, 0, 3, null, new int[] { 2, 3, 4 } };

// String
// String
yield return new object[] { new string[0], 0, 0, null, new string[0] };
yield return new object[] { new string[0], 0, 0, new StringComparer(), new string[0] };
yield return new object[] { new string[] { "5" }, 0, 1, null, new string[] { "5" } };
yield return new object[] { new string[] { "5" }, 0, 1, new StringComparer(), new string[] { "5" } };
yield return new object[] { new string[] { "5", "2" }, 0, 2, null, new string[] { "2", "5" } };
yield return new object[] { new string[] { "5", "2" }, 0, 2, new StringComparer(), new string[] { "2", "5" } };
yield return new object[] { new string[] { "5", "2", "3" }, 0, 3, null, new string[] { "2", "3", "5" } };
yield return new object[] { new string[] { "5", "2", "3"}, 0, 3, new StringComparer(), new string[] { "2", "3", "5" } };
yield return new object[] { new string[] { "5", "2", null }, 0, 3, null, new string[] {null, "2", "5" } };
yield return new object[] { new string[] { "5", "2", null}, 0, 3, new StringComparer(), new string[] { null, "2", "5" } };
yield return new object[] { new string[] { "5", "2", "3" }, 0, 3, new StringComparer(), new string[] { "2", "3", "5" } };
yield return new object[] { new string[] { "5", "2", null }, 0, 3, null, new string[] { null, "2", "5" } };
yield return new object[] { new string[] { "5", "2", null }, 0, 3, new StringComparer(), new string[] { null, "2", "5" } };
yield return new object[] { new string[] { "5", "2", "9", "8", "4", "3", "2", "4", "6" }, 0, 9, new StringComparer(), new string[] { "2", "2", "3", "4", "4", "5", "6", "8", "9" } };
yield return new object[] { new string[] { "5", null, "2", "9", "8", "4", "3", "2", "4", "6" }, 0, 10, new StringComparer(), new string[] { null, "2", "2", "3", "4", "4", "5", "6", "8", "9" } };
yield return new object[] { new string[] { "5", null, "2", "9", "8", "4", "3", "2", "4", "6" }, 3, 4, new StringComparer(), new string[] { "5", null, "2", "3", "4", "8", "9", "2", "4", "6" } };
Expand Down Expand Up @@ -4446,6 +4446,18 @@ public static void MaxSizes()
Assert.Equal(0x7FFFFFC7, Array.MaxLength);
}

[Fact]
public static void Array_Initialize()
{
var array = new StructWithParameterlessConstructor[10];
array.Initialize();
Assert.All(array, a => Assert.True(a.Constructed));

var array2 = new NonGenericClass1[10];
array2.Initialize();
Assert.All(array2, Assert.Null);
}

private static void VerifyArray(Array array, Type elementType, int[] lengths, int[] lowerBounds, object repeatedValue)
{
VerifyArray(array, elementType, lengths, lowerBounds);
Expand Down Expand Up @@ -4590,6 +4602,16 @@ private struct NonGenericStruct
public int z;
}

private struct StructWithParameterlessConstructor
{
public StructWithParameterlessConstructor()
{
Constructed = true;
}

public bool Constructed;
}

private class IntegerComparer : IComparer, IComparer<int>, IEqualityComparer
{
public int Compare(object x, object y) => Compare((int)x, (int)y);
Expand Down