Cleanup and improve name formatting in reflection (#20781)

* Delete internal Array.UnsafeCreateInstance method

* Delete binary serialization specific type name formatting

* Use ValueStringBuilder to format method names in reflection

10 files changed, 80 insertions, 131 deletions

diff --git a/src/System.Private.CoreLib/src/System/Array.cs b/src/System.Private.CoreLib/src/System/Array.cs
index 5cb7f1da20..b181afccf8 100644
--- a/src/System.Private.CoreLib/src/System/Array.cs
+++ b/src/System.Private.CoreLib/src/System/Array.cs
@@ -204,11 +204,6 @@ namespace System
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         private static extern unsafe Array InternalCreate(void* elementType, int rank, int* pLengths, int* pLowerBounds);
 
-        internal static Array UnsafeCreateInstance(Type elementType, int length)
-        {
-            return CreateInstance(elementType, length);
-        }
-
         // Copies length elements from sourceArray, starting at index 0, to
         // destinationArray, starting at index 0.
         //
diff --git a/src/System.Private.CoreLib/src/System/Attribute.cs b/src/System.Private.CoreLib/src/System/Attribute.cs
index fd84dd33b0..f69582a939 100644
--- a/src/System.Private.CoreLib/src/System/Attribute.cs
+++ b/src/System.Private.CoreLib/src/System/Attribute.cs
@@ -436,7 +436,7 @@ namespace System
 
         private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount)
         {
-            return (Attribute[])Array.UnsafeCreateInstance(elementType, elementCount);
+            return (Attribute[])Array.CreateInstance(elementType, elementCount);
         }
         #endregion
 
diff --git a/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs b/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
index c1c6e4f033..616646187d 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
@@ -1846,7 +1846,7 @@ namespace System.Reflection
 
         private static object[] CreateAttributeArrayHelper(Type elementType, int elementCount)
         {
-            return (object[])Array.UnsafeCreateInstance(elementType, elementCount);
+            return (object[])Array.CreateInstance(elementType, elementCount);
         }
         #endregion
     }
diff --git a/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs b/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs
index ce329b56d7..f87e4779f7 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs
@@ -2,21 +2,16 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// 
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Globalization;
+using System.Runtime.CompilerServices;
+using System.Security;
+using System.Text;
+using System.Threading;
 
 namespace System.Reflection.Emit
 {
-    using System;
-    using System.Collections.Generic;
-    using CultureInfo = System.Globalization.CultureInfo;
-    using System.Reflection;
-    using System.Security;
-    using System.Threading;
-    using System.Runtime.CompilerServices;
-    using System.Runtime.Versioning;
-    using System.Diagnostics;
-    using System.Runtime.InteropServices;
-
     public sealed class DynamicMethod : MethodInfo
     {
         private RuntimeType[] m_parameterTypes;
@@ -585,7 +580,17 @@ namespace System.Reflection.Emit
             //
             public override string ToString()
             {
-                return ReturnType.FormatTypeName() + " " + FormatNameAndSig();
+                var sbName = new ValueStringBuilder(MethodNameBufferSize);
+
+                sbName.Append(ReturnType.FormatTypeName());
+                sbName.Append(' ');
+                sbName.Append(Name);
+
+                sbName.Append('(');
+                AppendParameters(ref sbName, GetParameterTypes(), CallingConvention);
+                sbName.Append(')');
+
+                return sbName.ToString();
             }
 
             public override string Name
diff --git a/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs
index c6f01c67f6..8f3e909e0a 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Diagnostics;
 using System.Globalization;
 using System.Text;
 using System.Threading;
@@ -54,9 +53,10 @@ namespace System.Reflection
         #region Internal Methods
         // helper method to construct the string representation of the parameter list
 
-        internal static string ConstructParameters(Type[] parameterTypes, CallingConventions callingConvention, bool serialization)
+        internal const int MethodNameBufferSize = 100;
+
+        internal static void AppendParameters(ref ValueStringBuilder sbParamList, Type[] parameterTypes, CallingConventions callingConvention)
         {
-            StringBuilder sbParamList = new StringBuilder();
             string comma = "";
 
             for (int i = 0; i < parameterTypes.Length; i++)
@@ -65,12 +65,12 @@ namespace System.Reflection
 
                 sbParamList.Append(comma);
 
-                string typeName = t.FormatTypeName(serialization);
+                string typeName = t.FormatTypeName();
 
                 // Legacy: Why use "ByRef" for by ref parameters? What language is this? 
                 // VB uses "ByRef" but it should precede (not follow) the parameter name.
                 // Why don't we just use "&"?
-                if (t.IsByRef && !serialization)
+                if (t.IsByRef)
                 {
                     sbParamList.Append(typeName.TrimEnd('&'));
                     sbParamList.Append(" ByRef");
@@ -88,32 +88,6 @@ namespace System.Reflection
                 sbParamList.Append(comma);
                 sbParamList.Append("...");
             }
-
-            return sbParamList.ToString();
-        }
-
-        internal string FullName
-        {
-            get
-            {
-                return string.Format("{0}.{1}", DeclaringType.FullName, FormatNameAndSig());
-            }
-        }
-        internal string FormatNameAndSig()
-        {
-            return FormatNameAndSig(false);
-        }
-
-        internal virtual string FormatNameAndSig(bool serialization)
-        {
-            // Serialization uses ToString to resolve MethodInfo overloads.
-            StringBuilder sbName = new StringBuilder(Name);
-
-            sbName.Append("(");
-            sbName.Append(ConstructParameters(GetParameterTypes(), CallingConvention, serialization));
-            sbName.Append(")");
-
-            return sbName.ToString();
         }
 
         internal virtual Type[] GetParameterTypes()
diff --git a/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs
index c0f40bfbb1..ba50df5995 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Globalization;
+using System.Text;
 using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
 
 namespace System.Reflection
@@ -140,9 +141,21 @@ namespace System.Reflection
         #region Object Overrides
         public override string ToString()
         {
-            // "Void" really doesn't make sense here. But we'll keep it for compat reasons.
             if (m_toString == null)
-                m_toString = "Void " + FormatNameAndSig();
+            {
+                var sbName = new ValueStringBuilder(MethodNameBufferSize);
+
+                // "Void" really doesn't make sense here. But we'll keep it for compat reasons.
+                sbName.Append("Void ");
+
+                sbName.Append(Name);
+
+                sbName.Append('(');
+                AppendParameters(ref sbName, GetParameterTypes(), CallingConvention);
+                sbName.Append(')');
+
+                m_toString = sbName.ToString();
+            }
 
             return m_toString;
         }
diff --git a/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs b/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs
index 743d55f5a7..d3b0969572 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs
@@ -124,25 +124,6 @@ namespace System.Reflection
         #endregion
 
         #region Internal Members
-        internal override string FormatNameAndSig(bool serialization)
-        {
-            // Serialization uses ToString to resolve MethodInfo overloads.
-            StringBuilder sbName = new StringBuilder(Name);
-
-            // serialization == true: use unambiguous (except for assembly name) type names to distinguish between overloads.
-            // serialization == false: use basic format to maintain backward compatibility of MethodInfo.ToString().
-            TypeNameFormatFlags format = serialization ? TypeNameFormatFlags.FormatSerialization : TypeNameFormatFlags.FormatBasic;
-
-            if (IsGenericMethod)
-                sbName.Append(RuntimeMethodHandle.ConstructInstantiation(this, format));
-
-            sbName.Append("(");
-            sbName.Append(ConstructParameters(GetParameterTypes(), CallingConvention, serialization));
-            sbName.Append(")");
-
-            return sbName.ToString();
-        }
-
         internal override bool CacheEquals(object o)
         {
             RuntimeMethodInfo m = o as RuntimeMethodInfo;
@@ -197,7 +178,22 @@ namespace System.Reflection
         public override string ToString()
         {
             if (m_toString == null)
-                m_toString = ReturnType.FormatTypeName() + " " + FormatNameAndSig();
+            {
+                var sbName = new ValueStringBuilder(MethodNameBufferSize);
+
+                sbName.Append(ReturnType.FormatTypeName());
+                sbName.Append(' ');
+                sbName.Append(Name);
+
+                if (IsGenericMethod)
+                    sbName.Append(RuntimeMethodHandle.ConstructInstantiation(this, TypeNameFormatFlags.FormatBasic));
+
+                sbName.Append('(');
+                AppendParameters(ref sbName, GetParameterTypes(), CallingConvention);
+                sbName.Append(')');
+
+                m_toString = sbName.ToString();
+            }
 
             return m_toString;
         }
diff --git a/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs
index 880e826bbc..7f118d372b 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs
@@ -122,22 +122,18 @@ namespace System.Reflection
         #region Object Overrides
         public override string ToString()
         {
-            return FormatNameAndSig(false);
-        }
-
-        private string FormatNameAndSig(bool serialization)
-        {
-            StringBuilder sbName = new StringBuilder(PropertyType.FormatTypeName(serialization));
+            var sbName = new ValueStringBuilder(MethodBase.MethodNameBufferSize);
 
-            sbName.Append(" ");
+            sbName.Append(PropertyType.FormatTypeName());
+            sbName.Append(' ');
             sbName.Append(Name);
 
             RuntimeType[] arguments = Signature.Arguments;
             if (arguments.Length > 0)
             {
                 sbName.Append(" [");
-                sbName.Append(MethodBase.ConstructParameters(arguments, Signature.CallingConvention, serialization));
-                sbName.Append("]");
+                MethodBase.AppendParameters(ref sbName, arguments, Signature.CallingConvention);
+                sbName.Append(']');
             }
 
             return sbName.ToString();
diff --git a/src/System.Private.CoreLib/src/System/RtType.cs b/src/System.Private.CoreLib/src/System/RtType.cs
index 743e2291e5..59da425541 100644
--- a/src/System.Private.CoreLib/src/System/RtType.cs
+++ b/src/System.Private.CoreLib/src/System/RtType.cs
@@ -52,20 +52,12 @@ namespace System
         FormatAngleBrackets = 0x00000040, // Whether generic types are C<T> or C[T]
         FormatStubInfo = 0x00000080, // Include stub info like {unbox-stub}
         FormatGenericParam = 0x00000100, // Use !name and !!name for generic type and method parameters
-
-        // If we want to be able to distinguish between overloads whose parameter types have the same name but come from different assemblies,
-        // we can add FormatAssembly | FormatNoVersion to FormatSerialization. But we are omitting it because it is not a useful scenario
-        // and including the assembly name will normally increase the size of the serialized data and also decrease the performance.
-        FormatSerialization = FormatNamespace |
-                              FormatGenericParam |
-                              FormatFullInst
     }
 
     internal enum TypeNameKind
     {
         Name,
         ToString,
-        SerializationName,
         FullName,
     }
 
@@ -1448,7 +1440,6 @@ namespace System
             private string m_fullname;
             private string m_toString;
             private string m_namespace;
-            private string m_serializationname;
             private bool m_isGlobal;
             private bool m_bIsDomainInitialized;
             private MemberInfoCache<RuntimeMethodInfo> m_methodInfoCache;
@@ -1545,13 +1536,6 @@ namespace System
                         // No full instantiation and assembly.
                         return ConstructName(ref m_toString, TypeNameFormatFlags.FormatNamespace);
 
-                    case TypeNameKind.SerializationName:
-                        // Use FormatGenericParam in serialization. Otherwise we won't be able 
-                        // to distinguish between a generic parameter and a normal type with the same name.
-                        // e.g. Foo<T>.Bar(T t), the parameter type T could be !1 or a real type named "T".
-                        // Excluding the version number in the assembly name for VTS.
-                        return ConstructName(ref m_serializationname, TypeNameFormatFlags.FormatSerialization);
-
                     default:
                         throw new InvalidOperationException();
                 }
@@ -3576,7 +3560,7 @@ namespace System
             ulong[] values = Enum.InternalGetValues(this);
 
             // Create a generic Array
-            Array ret = Array.UnsafeCreateInstance(this, values.Length);
+            Array ret = Array.CreateInstance(this, values.Length);
 
             for (int i = 0; i < values.Length; i++)
             {
@@ -4496,36 +4480,27 @@ namespace System
         //  3. Remove the namespace ("System") for all primitive types, which is not language neutral.
         //  4. MethodBase.ToString() use "ByRef" for byref parameters which is different than Type.ToString().
         //  5. ConstructorInfo.ToString() outputs "Void" as the return type. Why Void?
-        // Since it could be a breaking changes to fix these legacy behaviors, we only use the better and more unambiguous format
-        // in serialization (MemberInfoSerializationHolder).
-        internal override string FormatTypeName(bool serialization)
+        internal override string FormatTypeName()
         {
-            if (serialization)
-            {
-                return GetCachedName(TypeNameKind.SerializationName);
-            }
-            else
-            {
-                Type elementType = GetRootElementType();
+            Type elementType = GetRootElementType();
 
-                // Legacy: this doesn't make sense, why use only Name for nested types but otherwise
-                // ToString() which contains namespace.
-                if (elementType.IsNested)
-                    return Name;
+            // Legacy: this doesn't make sense, why use only Name for nested types but otherwise
+            // ToString() which contains namespace.
+            if (elementType.IsNested)
+                return Name;
 
-                string typeName = ToString();
+            string typeName = ToString();
 
-                // Legacy: why removing "System"? Is it just because C# has keywords for these types?
-                // If so why don't we change it to lower case to match the C# keyword casing?
-                if (elementType.IsPrimitive ||
-                    elementType == typeof(void) ||
-                    elementType == typeof(TypedReference))
-                {
-                    typeName = typeName.Substring(@"System.".Length);
-                }
-
-                return typeName;
+            // Legacy: why removing "System"? Is it just because C# has keywords for these types?
+            // If so why don't we change it to lower case to match the C# keyword casing?
+            if (elementType.IsPrimitive ||
+                elementType == typeof(void) ||
+                elementType == typeof(TypedReference))
+            {
+                typeName = typeName.Substring(@"System.".Length);
             }
+
+            return typeName;
         }
 
         // This method looks like an attractive inline but expands to two calls,
@@ -4997,7 +4972,7 @@ namespace System
                     // Allocate the new array of wrappers.
                     Array oldArray = (Array)aArgs[i];
                     int numElems = oldArray.Length;
-                    Object[] newArray = (Object[])Array.UnsafeCreateInstance(wrapperType, numElems);
+                    Object[] newArray = (Object[])Array.CreateInstance(wrapperType, numElems);
 
                     // Retrieve the ConstructorInfo for the wrapper type.
                     ConstructorInfo wrapperCons;
diff --git a/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs
index 9cbc0b98fb..8d062fe498 100644
--- a/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs
+++ b/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs
@@ -142,12 +142,7 @@ namespace System
 #endif // FEATURE_COMINTEROP
 
         // This is only ever called on RuntimeType objects.
-        internal string FormatTypeName()
-        {
-            return FormatTypeName(false);
-        }
-
-        internal virtual string FormatTypeName(bool serialization)
+        internal virtual string FormatTypeName()
         {
             throw new NotImplementedException();
         }