From 8cf2d196d69508a09466ba3888695150ace83147 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 3 Nov 2018 09:45:30 -0700 Subject: [PATCH] Cleanup and improve name formatting in reflection (dotnet/coreclr#20781) * Delete internal Array.UnsafeCreateInstance method * Delete binary serialization specific type name formatting * Use ValueStringBuilder to format method names in reflection Commit migrated from https://github.com/dotnet/coreclr/commit/606c246d8da95258d148020e74d49a30d8fd74ee --- .../src/System.Private.CoreLib/src/System/Array.cs | 5 -- .../System.Private.CoreLib/src/System/Attribute.cs | 2 +- .../src/System/Reflection/CustomAttribute.cs | 2 +- .../src/System/Reflection/Emit/DynamicMethod.cs | 31 ++++++----- .../src/System/Reflection/MethodBase.CoreCLR.cs | 36 ++----------- .../System/Reflection/RuntimeConstructorInfo.cs | 17 +++++- .../src/System/Reflection/RuntimeMethodInfo.cs | 36 ++++++------- .../src/System/Reflection/RuntimePropertyInfo.cs | 14 ++--- .../System.Private.CoreLib/src/System/RtType.cs | 61 +++++++--------------- .../src/System/Type.CoreCLR.cs | 7 +-- 10 files changed, 80 insertions(+), 131 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Array.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Array.cs index 5cb7f1d..b181afc 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Array.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Attribute.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Attribute.cs index fd84dd33..f69582a 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Attribute.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs index c1c6e4f..6166461 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs index ce329b5..f87e477 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs index c6f01c6..8f3e909 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs index c0f40bf..ba50df5 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs index 743d55f..d3b0969 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index 880e826..7f118d3 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/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/coreclr/src/System.Private.CoreLib/src/System/RtType.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs index 743e229..59da425 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs @@ -52,20 +52,12 @@ namespace System FormatAngleBrackets = 0x00000040, // Whether generic types are C 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 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.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/coreclr/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs index 9cbc0b9..8d062fe 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs +++ b/src/coreclr/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(); } -- 2.7.4