[API Proposal]: Create array from array type (#88620)

* Create array from array type * InternalCreateFromArrayType * code review: - add missing comments for new public APIs - use the new API in DataTypeImplementation (and solve a TODO) - fix typo - remove code that was commented out - improve test name * Fix perf and corner-case behaviors * Delete superfluous validation * Skip handling of void arrays for this PR --------- Co-authored-by: Adam Sitnik <[email protected]> Co-authored-by: Jan Kotas <[email protected]>
dotnet · Nov 16, 2023 · 115199d · 115199d
1 parent 4325acc
commit 115199d
Show file tree

Hide file tree

Showing 15 changed files with 388 additions and 139 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
@@ -4,7 +4,6 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
@@ -15,9 +14,25 @@ namespace System
     // IList<U> and IReadOnlyList<U>, where T : U dynamically.  See the SZArrayHelper class for details.
     public abstract partial class Array : ICloneable, IList, IStructuralComparable, IStructuralEquatable
     {
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern unsafe Array InternalCreate(RuntimeType elementType, int rank, int* pLengths, int* pLowerBounds);
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Array_CreateInstance")]
+        private static unsafe partial void InternalCreate(QCallTypeHandle type, int rank, int* pLengths, int* pLowerBounds,
+            [MarshalAs(UnmanagedType.Bool)] bool fromArrayType, ObjectHandleOnStack retArray);
+
+        private static unsafe Array InternalCreate(RuntimeType elementType, int rank, int* pLengths, int* pLowerBounds)
+        {
+            Array? retArray = null;
+            InternalCreate(new QCallTypeHandle(ref elementType), rank, pLengths, pLowerBounds,
+                fromArrayType: false, ObjectHandleOnStack.Create(ref retArray));
+            return retArray!;
+        }
 
+        private static unsafe Array InternalCreateFromArrayType(RuntimeType arrayType, int rank, int* pLengths, int* pLowerBounds)
+        {
+            Array? retArray = null;
+            InternalCreate(new QCallTypeHandle(ref arrayType), rank, pLengths, pLowerBounds,
+                fromArrayType: true, ObjectHandleOnStack.Create(ref retArray));
+            return retArray!;
+        }
 
         private static unsafe void CopyImpl(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length, bool reliable)
         {

diff --git a/src/coreclr/classlibnative/bcltype/arraynative.cpp b/src/coreclr/classlibnative/bcltype/arraynative.cpp
@@ -720,14 +720,8 @@ FCIMPLEND
 
 
 // Check we're allowed to create an array with the given element type.
-void ArrayNative::CheckElementType(TypeHandle elementType)
+static void CheckElementType(TypeHandle elementType)
 {
-    // Checks apply recursively for arrays of arrays etc.
-    while (elementType.IsArray())
-    {
-        elementType = elementType.GetArrayElementTypeHandle();
-    }
-
     // Check for simple types first.
     if (!elementType.IsTypeDesc())
     {
@@ -738,7 +732,7 @@ void ArrayNative::CheckElementType(TypeHandle elementType)
             COMPlusThrow(kNotSupportedException, W("NotSupported_ByRefLikeArray"));
 
         // Check for open generic types.
-        if (pMT->IsGenericTypeDefinition() || pMT->ContainsGenericVariables())
+        if (pMT->ContainsGenericVariables())
             COMPlusThrow(kNotSupportedException, W("NotSupported_OpenType"));
 
         // Check for Void.
@@ -753,62 +747,67 @@ void ArrayNative::CheckElementType(TypeHandle elementType)
     }
 }
 
-FCIMPL4(Object*, ArrayNative::CreateInstance, ReflectClassBaseObject* pElementTypeUNSAFE, INT32 rank, INT32* pLengths, INT32* pLowerBounds)
+void QCALLTYPE Array_CreateInstance(QCall::TypeHandle pTypeHnd, INT32 rank, INT32* pLengths, INT32* pLowerBounds, BOOL createFromArrayType, QCall::ObjectHandleOnStack retArray)
 {
     CONTRACTL {
-        FCALL_CHECK;
+        QCALL_CHECK;
         PRECONDITION(rank > 0);
         PRECONDITION(CheckPointer(pLengths));
         PRECONDITION(CheckPointer(pLowerBounds, NULL_OK));
     } CONTRACTL_END;
 
-    OBJECTREF pRet = NULL;
-
-    REFLECTCLASSBASEREF pElementType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pElementTypeUNSAFE);
-
-    // pLengths and pLowerBounds are pinned buffers. No need to protect them.
-    HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(pElementType);
-
-    TypeHandle elementType(pElementType->GetType());
+    BEGIN_QCALL;
 
-    CheckElementType(elementType);
+    TypeHandle typeHnd = pTypeHnd.AsTypeHandle();
 
-    CorElementType CorType = elementType.GetSignatureCorElementType();
+    if (createFromArrayType)
+    {
+        _ASSERTE((INT32)typeHnd.GetRank() == rank);
+        _ASSERTE(typeHnd.IsArray());
 
-    CorElementType kind = ELEMENT_TYPE_ARRAY;
+        if (typeHnd.GetArrayElementTypeHandle().ContainsGenericVariables())
+            COMPlusThrow(kNotSupportedException, W("NotSupported_OpenType"));
 
-    // Is it ELEMENT_TYPE_SZARRAY array?
-    if (rank == 1 && (pLowerBounds == NULL || pLowerBounds[0] == 0)
-#ifdef FEATURE_64BIT_ALIGNMENT
-        // On platforms where 64-bit types require 64-bit alignment and don't obtain it naturally force us
-        // through the slow path where this will be handled.
-        && (CorType != ELEMENT_TYPE_I8)
-        && (CorType != ELEMENT_TYPE_U8)
-        && (CorType != ELEMENT_TYPE_R8)
-#endif
-        )
-    {
-        // Shortcut for common cases
-        if (CorTypeInfo::IsPrimitiveType(CorType))
+        if (!typeHnd.AsMethodTable()->IsMultiDimArray())
         {
-            pRet = AllocatePrimitiveArray(CorType,pLengths[0]);
+            _ASSERTE(pLowerBounds == NULL || pLowerBounds[0] == 0);
+
+            GCX_COOP();
+            retArray.Set(AllocateSzArray(typeHnd, pLengths[0]));
             goto Done;
         }
-        else
-        if (CorTypeInfo::IsObjRef(CorType))
+    }
+    else
+    {
+        CheckElementType(typeHnd);
+
+        // Is it ELEMENT_TYPE_SZARRAY array?
+        if (rank == 1 && (pLowerBounds == NULL || pLowerBounds[0] == 0))
         {
-            pRet = AllocateObjectArray(pLengths[0],elementType);
-            goto Done;
+            CorElementType corType = typeHnd.GetSignatureCorElementType();
+
+            // Shortcut for common cases
+            if (CorTypeInfo::IsPrimitiveType(corType))
+            {
+                GCX_COOP();
+                retArray.Set(AllocatePrimitiveArray(corType, pLengths[0]));
+                goto Done;
+            }
+
+            typeHnd = ClassLoader::LoadArrayTypeThrowing(typeHnd);
+
+            {
+                GCX_COOP();
+                retArray.Set(AllocateSzArray(typeHnd, pLengths[0]));
+                goto Done;
+            }
         }
 
-        kind = ELEMENT_TYPE_SZARRAY;
-        pLowerBounds = NULL;
+        // Find the Array class...
+        typeHnd = ClassLoader::LoadArrayTypeThrowing(typeHnd, ELEMENT_TYPE_ARRAY, rank);
     }
 
     {
-        // Find the Array class...
-        TypeHandle typeHnd = ClassLoader::LoadArrayTypeThrowing(elementType, kind, rank);
-
         _ASSERTE(rank <= MAX_RANK); // Ensures that the stack buffer size allocations below won't overflow
 
         DWORD boundsSize = 0;
@@ -834,15 +833,15 @@ FCIMPL4(Object*, ArrayNative::CreateInstance, ReflectClassBaseObject* pElementTy
                 bounds[i] = pLengths[i];
         }
 
-        pRet = AllocateArrayEx(typeHnd, bounds, boundsSize);
+        {
+            GCX_COOP();
+            retArray.Set(AllocateArrayEx(typeHnd, bounds, boundsSize));
+        }
     }
 
 Done: ;
-    HELPER_METHOD_FRAME_END();
-
-    return OBJECTREFToObject(pRet);
+    END_QCALL;
 }
-FCIMPLEND
 
 FCIMPL3(void, ArrayNative::SetValue, ArrayBase* refThisUNSAFE, Object* objUNSAFE, INT_PTR flattenedIndex)
 {

diff --git a/src/coreclr/classlibnative/bcltype/arraynative.h b/src/coreclr/classlibnative/bcltype/arraynative.h
@@ -30,8 +30,6 @@ class ArrayNative
     static FCDECL2(FC_BOOL_RET, IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst);
     static FCDECL5(void, CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase* pDst, INT32 iDstIndex, INT32 iLength);
 
-    static FCDECL4(Object*, CreateInstance, ReflectClassBaseObject* pElementTypeUNSAFE, INT32 rank, INT32* pLengths, INT32* pBounds);
-
     // This set of methods will set a value in an array
     static FCDECL3(void, SetValue, ArrayBase* refThisUNSAFE, Object* objUNSAFE, INT_PTR flattenedIndex);
 
@@ -44,9 +42,6 @@ class ArrayNative
     static FCDECL3_VVI(void*, GetSpanDataFrom, FCALLRuntimeFieldHandle structField, FCALLRuntimeTypeHandle targetTypeUnsafe, INT32* count);
 
 private:
-    // Helper for CreateInstance
-    static void CheckElementType(TypeHandle elementType);
-
     // Return values for CanAssignArrayType
     enum AssignArrayEnum
     {
@@ -66,6 +61,7 @@ class ArrayNative
 
 };
 
+extern "C" void QCALLTYPE Array_CreateInstance(QCall::TypeHandle pTypeHnd, INT32 rank, INT32* pLengths, INT32* pBounds, BOOL createFromArrayType, QCall::ObjectHandleOnStack retArray);
 extern "C" PCODE QCALLTYPE Array_GetElementConstructorEntrypoint(QCall::TypeHandle pArrayTypeHnd);
 
 #endif // _ARRAYNATIVE_H_
diff --git a/...ate.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs b/...ate.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs
@@ -126,8 +126,6 @@ public static Attribute Instantiate(this CustomAttributeData cad)
         //
         // Convert the argument value reported by Reflection into an actual object.
         //
-        [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode",
-            Justification = "The AOT compiler ensures array types required by custom attribute blobs are generated.")]
         private static object? Convert(this CustomAttributeTypedArgument typedArgument)
         {
             Type argumentType = typedArgument.ArgumentType;
@@ -144,8 +142,7 @@ public static Attribute Instantiate(this CustomAttributeData cad)
                 IList<CustomAttributeTypedArgument>? typedElements = (IList<CustomAttributeTypedArgument>?)(typedArgument.Value);
                 if (typedElements == null)
                     return null;
-                Type? elementType = argumentType.GetElementType();
-                Array array = Array.CreateInstance(elementType, typedElements.Count);
+                Array array = Array.CreateInstanceFromArrayType(argumentType, typedElements.Count);
                 for (int i = 0; i < typedElements.Count; i++)
                 {
                     object? elementValue = typedElements[i].Convert();

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs
@@ -59,7 +59,12 @@ internal unsafe bool IsSzArray
         [RequiresDynamicCode("The code for an array of the specified type might not be available.")]
         private static unsafe Array InternalCreate(RuntimeType elementType, int rank, int* pLengths, int* pLowerBounds)
         {
-            ValidateElementType(elementType);
+            if (elementType.IsByRef || elementType.IsByRefLike)
+                throw new NotSupportedException(SR.NotSupported_ByRefLikeArray);
+            if (elementType == typeof(void))
+                throw new NotSupportedException(SR.NotSupported_VoidArray);
+            if (elementType.ContainsGenericParameters)
+                throw new NotSupportedException(SR.NotSupported_OpenType);
 
             if (pLowerBounds != null)
             {
@@ -73,34 +78,59 @@ private static unsafe Array InternalCreate(RuntimeType elementType, int rank, in
             if (rank == 1)
             {
                 return RuntimeImports.RhNewArray(elementType.MakeArrayType().TypeHandle.ToEETypePtr(), pLengths[0]);
-
             }
             else
             {
-                // Create a local copy of the lengths that cannot be motified by the caller
+                Type arrayType = elementType.MakeArrayType(rank);
+
+                // Create a local copy of the lengths that cannot be modified by the caller
                 int* pImmutableLengths = stackalloc int[rank];
                 for (int i = 0; i < rank; i++)
                     pImmutableLengths[i] = pLengths[i];
 
-                return NewMultiDimArray(elementType.MakeArrayType(rank).TypeHandle.ToEETypePtr(), pImmutableLengths, rank);
+                return NewMultiDimArray(arrayType.TypeHandle.ToEETypePtr(), pImmutableLengths, rank);
             }
         }
 
-#pragma warning disable CA1859 // https://github.com/dotnet/roslyn-analyzers/issues/6451
-        private static void ValidateElementType(Type elementType)
+        [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode",
+            Justification = "The compiler ensures that if we have a TypeHandle of a Rank-1 MdArray, we also generated the SzArray.")]
+        private static unsafe Array InternalCreateFromArrayType(RuntimeType arrayType, int rank, int* pLengths, int* pLowerBounds)
         {
-            while (elementType.IsArray)
+            Debug.Assert(arrayType.IsArray);
+            Debug.Assert(arrayType.GetArrayRank() == rank);
+
+            if (arrayType.ContainsGenericParameters)
+                throw new NotSupportedException(SR.NotSupported_OpenType);
+
+            if (pLowerBounds != null)
             {
-                elementType = elementType.GetElementType()!;
+                for (int i = 0; i < rank; i++)
+                {
+                    if (pLowerBounds[i] != 0)
+                        throw new PlatformNotSupportedException(SR.PlatformNotSupported_NonZeroLowerBound);
+                }
+            }
+
+            EETypePtr eeType = arrayType.TypeHandle.ToEETypePtr();
+            if (rank == 1)
+            {
+                // Multidimensional array of rank 1 with 0 lower bounds gets actually allocated
+                // as an SzArray. SzArray is castable to MdArray rank 1.
+                if (!eeType.IsSzArray)
+                    eeType = arrayType.GetElementType().MakeArrayType().TypeHandle.ToEETypePtr();
+
+                return RuntimeImports.RhNewArray(eeType, pLengths[0]);
+            }
+            else
+            {
+                // Create a local copy of the lengths that cannot be modified by the caller
+                int* pImmutableLengths = stackalloc int[rank];
+                for (int i = 0; i < rank; i++)
+                    pImmutableLengths[i] = pLengths[i];
+
+                return NewMultiDimArray(eeType, pImmutableLengths, rank);
             }
-            if (elementType.IsByRef || elementType.IsByRefLike)
-                throw new NotSupportedException(SR.NotSupported_ByRefLikeArray);
-            if (elementType == typeof(void))
-                throw new NotSupportedException(SR.NotSupported_VoidArray);
-            if (elementType.ContainsGenericParameters)
-                throw new NotSupportedException(SR.NotSupported_OpenType);
         }
-#pragma warning restore CA1859
 
         public unsafe void Initialize()
         {

diff --git a/src/coreclr/vm/comsynchronizable.h b/src/coreclr/vm/comsynchronizable.h
@@ -70,7 +70,6 @@ friend class ThreadBaseObject;
 
 
     static FCDECL0(INT32,   GetOptimalMaxSpinWaitsPerSpinIteration);
-    static FCDECL1(void,    SpinWait,                       int iterations);
     static FCDECL0(Object*, GetCurrentThread);
     static FCDECL1(void,    Finalize,                       ThreadBaseObject* pThis);
 #ifdef FEATURE_COMINTEROP

diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h
@@ -422,7 +422,6 @@ FCFuncStart(gArrayFuncs)
     FCFuncElement("GetCorElementTypeOfElementType", ArrayNative::GetCorElementTypeOfElementType)
     FCFuncElement("IsSimpleCopy", ArrayNative::IsSimpleCopy)
     FCFuncElement("CopySlow", ArrayNative::CopySlow)
-    FCFuncElement("InternalCreate", ArrayNative::CreateInstance)
     FCFuncElement("InternalSetValue", ArrayNative::SetValue)
 FCFuncEnd()
 

diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp
@@ -151,6 +151,7 @@ static const Entry s_QCall[] =
     DllImportEntry(TypeBuilder_SetConstantValue)
     DllImportEntry(TypeBuilder_DefineCustomAttribute)
     DllImportEntry(MdUtf8String_EqualsCaseInsensitive)
+    DllImportEntry(Array_CreateInstance)
     DllImportEntry(Array_GetElementConstructorEntrypoint)
     DllImportEntry(AssemblyName_InitializeAssemblySpec)
     DllImportEntry(AssemblyNative_GetFullName)