From 0ecf62e1dff6864a5990c9bef0615ca067c96f43 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 3 Apr 2023 13:50:02 -0700 Subject: [PATCH 1/7] Suppress linker warnings properly --- .../src/System/Reflection/Emit/MethodBuilderImpl.cs | 3 ++- .../src/System/Reflection/Emit/ModuleBuilderImpl.cs | 3 +-- .../src/System/Reflection/Emit/SignatureHelper.cs | 3 +++ .../src/System/Reflection/Emit/TypeBuilderImpl.cs | 8 +++++--- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs index 841377f596e8a..99aa83ec72785 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs @@ -4,7 +4,6 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection.Metadata; -using System.Reflection.Metadata.Ecma335; namespace System.Reflection.Emit { @@ -18,6 +17,8 @@ internal sealed class MethodBuilderImpl : MethodBuilder private readonly CallingConventions _callingConventions; private readonly TypeBuilderImpl _declaringType; + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Do not expect System.Void type will be trimmed from core assembly")] internal MethodBuilderImpl(string name, MethodAttributes attributes, CallingConventions callingConventions, Type? returnType, Type[]? parameterTypes, ModuleBuilderImpl module, TypeBuilderImpl declaringType) { diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index 0ed48f0c1a050..df60470665e2e 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -26,6 +26,7 @@ internal ModuleBuilderImpl(string name, Assembly coreAssembly) _name = name; } + [RequiresUnreferencedCode("Types might be removed")] internal Type GetTypeFromCoreAssembly(string name) { Type? type; @@ -33,9 +34,7 @@ internal Type GetTypeFromCoreAssembly(string name) // TODO: Use Enum as the key for perf if (!_coreTypes.TryGetValue(name, out type)) { -#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code type = _coreAssembly.GetType(name, throwOnError: true)!; -#pragma warning restore IL2026 _coreTypes.Add(name, type); } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index a17a1a86f4600..2fb19720307dd 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; @@ -18,6 +19,8 @@ internal static BlobBuilder FieldSignatureEncoder(Type fieldType) return fieldSignature; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Do not expect System.Void type will be trimmed from core assembly")] internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl _module, Type[]? parameters, Type? returnType, bool isInstance) { // Encoding return type and parameters. diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs index e640c86790333..ece7ff7bbc8b6 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs @@ -71,7 +71,11 @@ protected override MethodBuilder DefineMethodCore(string name, MethodAttributes protected override void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute) => throw new NotImplementedException(); protected override void SetCustomAttributeCore(CustomAttributeBuilder customBuilder) => throw new NotImplementedException(); - protected override void SetParentCore([DynamicallyAccessedMembers((DynamicallyAccessedMemberTypes)(-1))] Type? parent) + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:DynamicallyAccessedMembers", + Justification = "No need to propogate the attriubte for called method")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Do not expect System.Object type will be trimmed from core assembly")] + protected override void SetParentCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent) { if (parent != null) { @@ -86,9 +90,7 @@ protected override void SetParentCore([DynamicallyAccessedMembers((DynamicallyAc { if ((_attributes & TypeAttributes.Interface) != TypeAttributes.Interface) { -#pragma warning disable IL2074 // Value stored in field does not satisfy 'DynamicallyAccessedMembersAttribute' requirements. The return value of the source method does not have matching annotations. _typeParent = _module.GetTypeFromCoreAssembly("System.Object"); -#pragma warning restore IL2074 } else { From 0c5cbdc22d7444047b7136e58c8e3bc659ac83d8 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 3 Apr 2023 18:41:43 -0700 Subject: [PATCH 2/7] Update core assembly types handling --- .../System/Reflection/Emit/MetadataHelper.cs | 28 ++-- .../Reflection/Emit/MethodBuilderImpl.cs | 4 +- .../Reflection/Emit/ModuleBuilderImpl.cs | 25 ++-- .../System/Reflection/Emit/SignatureHelper.cs | 123 ++++++++++++------ .../System/Reflection/Emit/TypeBuilderImpl.cs | 6 +- .../AssemblySaveTestsWithVariousTypes.cs | 2 +- 6 files changed, 115 insertions(+), 73 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MetadataHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MetadataHelper.cs index 782e14e4446c2..869c5ef3f1113 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MetadataHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MetadataHelper.cs @@ -6,7 +6,7 @@ namespace System.Reflection.Emit { - // This static helper class adds common entities to a Metadata Builder. + // This static helper class adds common entities to a MetadataBuilder. internal static class MetadataHelper { internal static AssemblyReferenceHandle AddAssemblyReference(Assembly assembly, MetadataBuilder metadata) @@ -34,7 +34,7 @@ internal static TypeDefinitionHandle AddTypeDefinition(MetadataBuilder metadata, // Add type metadata return metadata.AddTypeDefinition( attributes: typeBuilder.Attributes, - (typeBuilder.Namespace == null) ? default : metadata.GetOrAddString(typeBuilder.Namespace), + @namespace: (typeBuilder.Namespace == null) ? default : metadata.GetOrAddString(typeBuilder.Namespace), name: metadata.GetOrAddString(typeBuilder.Name), baseType: baseType, fieldList: MetadataTokens.FieldDefinitionHandle(fieldToken), @@ -49,28 +49,30 @@ internal static TypeReferenceHandle AddTypeReference(MetadataBuilder metadata, T internal static TypeReferenceHandle AddTypeReference(MetadataBuilder metadata, AssemblyReferenceHandle parent, string name, string? nameSpace) { return metadata.AddTypeReference( - parent, - (nameSpace == null) ? default : metadata.GetOrAddString(nameSpace), - metadata.GetOrAddString(name) + resolutionScope: parent, + @namespace: (nameSpace == null) ? default : metadata.GetOrAddString(nameSpace), + name: metadata.GetOrAddString(name) ); } internal static MethodDefinitionHandle AddMethodDefinition(MetadataBuilder metadata, MethodBuilderImpl methodBuilder, BlobBuilder methodSignatureBlob) { return metadata.AddMethodDefinition( - methodBuilder.Attributes, - MethodImplAttributes.IL, - metadata.GetOrAddString(methodBuilder.Name), - metadata.GetOrAddBlob(methodSignatureBlob), - -1, // No body supported yet + attributes: methodBuilder.Attributes, + implAttributes: MethodImplAttributes.IL, + name: metadata.GetOrAddString(methodBuilder.Name), + signature: metadata.GetOrAddBlob(methodSignatureBlob), + bodyOffset: -1, // No body supported yet parameterList: MetadataTokens.ParameterHandle(1) ); } - internal static FieldDefinitionHandle AddFieldDefinition(MetadataBuilder metadata, FieldInfo field) + internal static FieldDefinitionHandle AddFieldDefinition(MetadataBuilder metadata, FieldInfo field, BlobBuilder fieldSignatureBlob) { - return metadata.AddFieldDefinition(field.Attributes, metadata.GetOrAddString(field.Name), - metadata.GetOrAddBlob(MetadataSignatureHelper.FieldSignatureEncoder(field.FieldType))); + return metadata.AddFieldDefinition( + attributes: field.Attributes, + name: metadata.GetOrAddString(field.Name), + signature: metadata.GetOrAddBlob(fieldSignatureBlob)); } } } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs index 99aa83ec72785..8121af09c9629 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs @@ -17,13 +17,11 @@ internal sealed class MethodBuilderImpl : MethodBuilder private readonly CallingConventions _callingConventions; private readonly TypeBuilderImpl _declaringType; - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "Do not expect System.Void type will be trimmed from core assembly")] internal MethodBuilderImpl(string name, MethodAttributes attributes, CallingConventions callingConventions, Type? returnType, Type[]? parameterTypes, ModuleBuilderImpl module, TypeBuilderImpl declaringType) { _module = module; - _returnType = returnType ?? _module.GetTypeFromCoreAssembly("System.Void"); ; + _returnType = returnType ?? _module.GetTypeFromCoreAssembly(CoreTypeId.Void)!; _name = name; _attributes = attributes; _callingConventions = callingConventions; diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index df60470665e2e..ace11ef1b9fb8 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -13,32 +13,29 @@ internal sealed class ModuleBuilderImpl : ModuleBuilder { private readonly Assembly _coreAssembly; private readonly string _name; - private readonly Dictionary _coreTypes = new(); + private Type?[]? _coreTypes; private readonly Dictionary _assemblyRefStore = new(); private readonly Dictionary _typeRefStore = new(); private readonly List _typeDefStore = new(); private int _nextMethodDefRowId = 1; private int _nextFieldDefRowId = 1; - + private static readonly Type[] s_coreTypes = { typeof(void), typeof(object), typeof(bool), typeof(char), typeof(sbyte), typeof(byte), typeof(short), + typeof(ushort), typeof(int), typeof(uint), typeof(long), typeof(ulong), typeof(float), typeof(double), + typeof(decimal), typeof(DateTime), typeof(string), typeof(nint), typeof(nuint) }; internal ModuleBuilderImpl(string name, Assembly coreAssembly) { _coreAssembly = coreAssembly; _name = name; } - [RequiresUnreferencedCode("Types might be removed")] - internal Type GetTypeFromCoreAssembly(string name) + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] + internal Type? GetTypeFromCoreAssembly(CoreTypeId typeId) { - Type? type; - - // TODO: Use Enum as the key for perf - if (!_coreTypes.TryGetValue(name, out type)) - { - type = _coreAssembly.GetType(name, throwOnError: true)!; - _coreTypes.Add(name, type); - } + int index = (int)typeId; + // Use s_coreTypes directly for runtime reflection + _coreTypes ??= (_coreAssembly == typeof(object).Assembly) ? s_coreTypes : new Type[s_coreTypes.Length]; - return type; + return _coreTypes[index] ?? (_coreTypes[index] = _coreAssembly.GetType(s_coreTypes[index].FullName!, throwOnError: true)); } internal void AppendMetadata(MetadataBuilder metadata) @@ -81,7 +78,7 @@ internal void AppendMetadata(MetadataBuilder metadata) foreach (FieldBuilderImpl field in typeBuilder._fieldDefStore) { - MetadataHelper.AddFieldDefinition(metadata, field); + MetadataHelper.AddFieldDefinition(metadata, field, MetadataSignatureHelper.FieldSignatureEncoder(field.FieldType, this)); _nextFieldDefRowId++; } } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index 2fb19720307dd..dbdc1b4479e5b 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -10,18 +10,16 @@ namespace System.Reflection.Emit // TODO: Only support simple signatures. More complex signatures will be added. internal static class MetadataSignatureHelper { - internal static BlobBuilder FieldSignatureEncoder(Type fieldType) + internal static BlobBuilder FieldSignatureEncoder(Type fieldType, ModuleBuilderImpl module) { BlobBuilder fieldSignature = new(); - WriteSignatureTypeForReflectionType(new BlobEncoder(fieldSignature).FieldSignature(), fieldType); + WriteSignatureTypeForReflectionType(new BlobEncoder(fieldSignature).FieldSignature(), fieldType, module); return fieldSignature; } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "Do not expect System.Void type will be trimmed from core assembly")] - internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl _module, Type[]? parameters, Type? returnType, bool isInstance) + internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl module, Type[]? parameters, Type? returnType, bool isInstance) { // Encoding return type and parameters. BlobBuilder methodSignature = new(); @@ -33,9 +31,9 @@ internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl _module, Ty MethodSignature(isInstanceMethod: isInstance). Parameters((parameters == null) ? 0 : parameters.Length, out retEncoder, out parEncoder); - if (returnType != null && returnType != _module.GetTypeFromCoreAssembly("System.Void")) + if (returnType != null && returnType != module.GetTypeFromCoreAssembly(CoreTypeId.Void)) { - WriteSignatureTypeForReflectionType(retEncoder.Type(), returnType); + WriteSignatureTypeForReflectionType(retEncoder.Type(), returnType, module); } else // If null mark ReturnTypeEncoder as void { @@ -46,46 +44,95 @@ internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl _module, Ty { foreach (Type parameter in parameters) { - WriteSignatureTypeForReflectionType(parEncoder.AddParameter().Type(), parameter); + WriteSignatureTypeForReflectionType(parEncoder.AddParameter().Type(), parameter, module); } } return methodSignature; } - private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder signature, Type type) + private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module) { - // We need to translate from Reflection.Type to SignatureTypeEncoder. Most common types for proof of concept. More types will be added. - // TODO: This switch should be done by comparing Type objects, without fetching FullName. - switch (type.FullName) + // We need to translate from Reflection.Type to SignatureTypeEncoder. + if (Enum.TryParse(type.Name, out CoreTypeId typeId) && type.Equals(module.GetTypeFromCoreAssembly(typeId))) { - case "System.Boolean": - signature.Boolean(); - break; - case "System.Byte": - signature.Byte(); - break; - case "System.Char": - signature.Char(); - break; - case "System.Double": - signature.Double(); - break; - case "System.Int32": - signature.Int32(); - break; - case "System.Int64": - signature.Int64(); - break; - case "System.Object": - signature.Object(); - break; - case "System.String": - signature.String(); - break; - - default: throw new NotSupportedException(SR.Format(SR.NotSupported_Signature, type.FullName)); + switch (typeId) + { + case CoreTypeId.Boolean: + signature.Boolean(); + return; + case CoreTypeId.Byte: + signature.Byte(); + return; + case CoreTypeId.SByte: + signature.SByte(); + return; + case CoreTypeId.Char: + signature.Char(); + return; + case CoreTypeId.Int16: + signature.Int16(); + break; + case CoreTypeId.UInt16: + signature.UInt16(); + return; + case CoreTypeId.Int32: + signature.Int32(); + return; + case CoreTypeId.UInt32: + signature.UInt32(); + return; + case CoreTypeId.Int64: + signature.Int64(); + return; + case CoreTypeId.UInt64: + signature.UInt64(); + return; + case CoreTypeId.Single: + signature.Single(); + return; + case CoreTypeId.Double: + signature.Double(); + return; + case CoreTypeId.IntPtr: + signature.IntPtr(); + return; + case CoreTypeId.UIntPtr: + signature.UIntPtr(); + return; + case CoreTypeId.Object: + signature.Object(); + return; + case CoreTypeId.String: + signature.String(); + return; + } } + + throw new NotSupportedException(SR.Format(SR.NotSupported_Signature, type.FullName)); } } + + internal enum CoreTypeId + { + Void = 0, + Object = 1, + Boolean = 2, + Char = 3, + SByte = 4, + Byte = 5, + Int16 = 6, + UInt16 = 7, + Int32 = 8, + UInt32 = 9, + Int64 = 10, + UInt64 = 11, + Single = 12, + Double = 13, + Decimal = 14, + DateTime = 15, + String = 16, + IntPtr = 17, + UIntPtr = 18, + } } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs index ece7ff7bbc8b6..f702fb4c92e90 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs @@ -72,9 +72,7 @@ protected override MethodBuilder DefineMethodCore(string name, MethodAttributes protected override void SetCustomAttributeCore(CustomAttributeBuilder customBuilder) => throw new NotImplementedException(); [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:DynamicallyAccessedMembers", - Justification = "No need to propogate the attriubte for called method")] - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "Do not expect System.Object type will be trimmed from core assembly")] + Justification = "No need to propogate the attribute to the called method")] protected override void SetParentCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent) { if (parent != null) @@ -90,7 +88,7 @@ protected override void SetParentCore([DynamicallyAccessedMembers(DynamicallyAcc { if ((_attributes & TypeAttributes.Interface) != TypeAttributes.Interface) { - _typeParent = _module.GetTypeFromCoreAssembly("System.Object"); + _typeParent = _module.GetTypeFromCoreAssembly(CoreTypeId.Object); } else { diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTestsWithVariousTypes.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTestsWithVariousTypes.cs index 0cff8c2327224..c459a9be15a56 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTestsWithVariousTypes.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTestsWithVariousTypes.cs @@ -63,7 +63,7 @@ public static IEnumerable VariousInterfacesStructsTestData() yield return new object[] { new Type[] { typeof(EmptyStruct) } }; yield return new object[] { new Type[] { typeof(StructWithField) } }; yield return new object[] { new Type[] { typeof(StructWithField), typeof(EmptyStruct) } }; - yield return new object[] { new Type[] { typeof(IMultipleMethod), typeof(EmptyStruct) , typeof(INoMethod2), typeof(StructWithField) } }; + yield return new object[] { new Type[] { typeof(IMultipleMethod), typeof(EmptyStruct), typeof(INoMethod2), typeof(StructWithField) } }; } [Theory] From 955eeda85e43342eb922046fe71959b8e84681cf Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 3 Apr 2023 21:37:17 -0700 Subject: [PATCH 3/7] Apply feedbacks --- .../Reflection/Emit/MethodBuilderImpl.cs | 2 +- .../Reflection/Emit/ModuleBuilderImpl.cs | 36 ++++++++++++---- .../System/Reflection/Emit/SignatureHelper.cs | 42 +++++++++---------- 3 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs index 8121af09c9629..c63e4c26e5d47 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs @@ -21,7 +21,7 @@ internal MethodBuilderImpl(string name, MethodAttributes attributes, CallingConv Type[]? parameterTypes, ModuleBuilderImpl module, TypeBuilderImpl declaringType) { _module = module; - _returnType = returnType ?? _module.GetTypeFromCoreAssembly(CoreTypeId.Void)!; + _returnType = returnType ?? _module.GetTypeFromCoreAssembly(CoreTypeId.Void); _name = name; _attributes = attributes; _callingConventions = callingConventions; diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index ace11ef1b9fb8..da23b3ce3d372 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -13,29 +13,49 @@ internal sealed class ModuleBuilderImpl : ModuleBuilder { private readonly Assembly _coreAssembly; private readonly string _name; - private Type?[]? _coreTypes; + private Type?[] _coreTypes; private readonly Dictionary _assemblyRefStore = new(); private readonly Dictionary _typeRefStore = new(); private readonly List _typeDefStore = new(); private int _nextMethodDefRowId = 1; private int _nextFieldDefRowId = 1; - private static readonly Type[] s_coreTypes = { typeof(void), typeof(object), typeof(bool), typeof(char), typeof(sbyte), typeof(byte), typeof(short), - typeof(ushort), typeof(int), typeof(uint), typeof(long), typeof(ulong), typeof(float), typeof(double), - typeof(decimal), typeof(DateTime), typeof(string), typeof(nint), typeof(nuint) }; + private static readonly Type[] s_coreTypes = { typeof(void), typeof(object), typeof(bool), typeof(char), typeof(sbyte), typeof(byte), typeof(short), typeof(ushort), typeof(int), + typeof(uint), typeof(long), typeof(ulong), typeof(float), typeof(double), typeof(string), typeof(nint), typeof(nuint) }; internal ModuleBuilderImpl(string name, Assembly coreAssembly) { _coreAssembly = coreAssembly; + // Use s_coreTypes directly for runtime reflection + _coreTypes = (_coreAssembly == typeof(object).Assembly) ? s_coreTypes : new Type[s_coreTypes.Length]; _name = name; } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] - internal Type? GetTypeFromCoreAssembly(CoreTypeId typeId) + internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) { int index = (int)typeId; - // Use s_coreTypes directly for runtime reflection - _coreTypes ??= (_coreAssembly == typeof(object).Assembly) ? s_coreTypes : new Type[s_coreTypes.Length]; - return _coreTypes[index] ?? (_coreTypes[index] = _coreAssembly.GetType(s_coreTypes[index].FullName!, throwOnError: true)); + return _coreTypes[index] ?? (_coreTypes[index] = _coreAssembly.GetType(s_coreTypes[index].FullName!, throwOnError: true)!); + } + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] + internal int GetTypeIdFromCoreTypes(Type type) + { + for(int i=0; i < _coreTypes.Length; i++) + { + if (_coreTypes[i] == type) + { + return i; + } + } + + Type? foundType = _coreAssembly.GetType(type.FullName!, throwOnError: true); + + if (foundType == null || !Enum.TryParse(foundType.Name, out CoreTypeId result)) + { + return -1; + } + + return (int)result; } internal void AppendMetadata(MetadataBuilder metadata) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index dbdc1b4479e5b..fbda54bc36015 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -53,10 +53,12 @@ internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl module, Typ private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module) { + int id = module.GetTypeIdFromCoreTypes(type); + // We need to translate from Reflection.Type to SignatureTypeEncoder. - if (Enum.TryParse(type.Name, out CoreTypeId typeId) && type.Equals(module.GetTypeFromCoreAssembly(typeId))) + if (id > -1) { - switch (typeId) + switch ((CoreTypeId)id) { case CoreTypeId.Boolean: signature.Boolean(); @@ -115,24 +117,22 @@ private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder sig internal enum CoreTypeId { - Void = 0, - Object = 1, - Boolean = 2, - Char = 3, - SByte = 4, - Byte = 5, - Int16 = 6, - UInt16 = 7, - Int32 = 8, - UInt32 = 9, - Int64 = 10, - UInt64 = 11, - Single = 12, - Double = 13, - Decimal = 14, - DateTime = 15, - String = 16, - IntPtr = 17, - UIntPtr = 18, + Void, + Object, + Boolean, + Char, + SByte, + Byte, + Int16, + UInt16, + Int32, + UInt32, + Int64, + UInt64, + Single, + Double, + String, + IntPtr, + UIntPtr, } } From 14d2561f5299e2653821d93e4d5cd026ab9a0b43 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 4 Apr 2023 11:14:04 -0700 Subject: [PATCH 4/7] Apply feedbacks --- .../Reflection/Emit/ModuleBuilderImpl.cs | 55 ++++++++++++++----- .../System/Reflection/Emit/SignatureHelper.cs | 6 +- .../System/Reflection/Emit/TypeBuilderImpl.cs | 2 +- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index da23b3ce3d372..c0739e32408fd 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -13,7 +13,7 @@ internal sealed class ModuleBuilderImpl : ModuleBuilder { private readonly Assembly _coreAssembly; private readonly string _name; - private Type?[] _coreTypes; + private Type[]? _coreTypes; private readonly Dictionary _assemblyRefStore = new(); private readonly Dictionary _typeRefStore = new(); private readonly List _typeDefStore = new(); @@ -24,38 +24,63 @@ internal sealed class ModuleBuilderImpl : ModuleBuilder internal ModuleBuilderImpl(string name, Assembly coreAssembly) { _coreAssembly = coreAssembly; - // Use s_coreTypes directly for runtime reflection - _coreTypes = (_coreAssembly == typeof(object).Assembly) ? s_coreTypes : new Type[s_coreTypes.Length]; _name = name; } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] - internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) + internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) { - int index = (int)typeId; + if (_coreTypes == null) + { + // Use s_coreTypes directly for runtime reflection + if (_coreAssembly == typeof(object).Assembly) + { + _coreTypes = s_coreTypes; + } + else + { + _coreTypes = new Type[s_coreTypes.Length]; - return _coreTypes[index] ?? (_coreTypes[index] = _coreAssembly.GetType(s_coreTypes[index].FullName!, throwOnError: true)!); + for (int i = 0; i < s_coreTypes.Length; i++) + { + _coreTypes[i] = _coreAssembly.GetType(s_coreTypes[i].FullName!, throwOnError: true)!; + } + } + } + + return _coreTypes[(int)typeId]; } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] - internal int GetTypeIdFromCoreTypes(Type type) + internal CoreTypeId? GetTypeIdFromCoreTypes(Type type) { - for(int i=0; i < _coreTypes.Length; i++) + if (_coreTypes == null) { - if (_coreTypes[i] == type) + // Use s_coreTypes directly for runtime reflection + if (_coreAssembly == typeof(object).Assembly) { - return i; + _coreTypes = s_coreTypes; } - } + else + { + _coreTypes = new Type[s_coreTypes.Length]; - Type? foundType = _coreAssembly.GetType(type.FullName!, throwOnError: true); + for (int i = 0; i < s_coreTypes.Length; i++) + { + _coreTypes[i] = _coreAssembly.GetType(s_coreTypes[i].FullName!, throwOnError: true)!; + } + } + } - if (foundType == null || !Enum.TryParse(foundType.Name, out CoreTypeId result)) + for (int i = 0; i < _coreTypes.Length; i++) { - return -1; + if (_coreTypes[i] == type) + { + return (CoreTypeId)i; + } } - return (int)result; + return null; } internal void AppendMetadata(MetadataBuilder metadata) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index fbda54bc36015..9b3824f4dbb16 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -53,12 +53,12 @@ internal static BlobBuilder MethodSignatureEncoder(ModuleBuilderImpl module, Typ private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module) { - int id = module.GetTypeIdFromCoreTypes(type); + CoreTypeId? typeId = module.GetTypeIdFromCoreTypes(type); // We need to translate from Reflection.Type to SignatureTypeEncoder. - if (id > -1) + if (typeId.HasValue) { - switch ((CoreTypeId)id) + switch (typeId) { case CoreTypeId.Boolean: signature.Boolean(); diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs index f702fb4c92e90..71548abab22ef 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs @@ -72,7 +72,7 @@ protected override MethodBuilder DefineMethodCore(string name, MethodAttributes protected override void SetCustomAttributeCore(CustomAttributeBuilder customBuilder) => throw new NotImplementedException(); [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:DynamicallyAccessedMembers", - Justification = "No need to propogate the attribute to the called method")] + Justification = "TODO: Need to figure out how to preserve System.Object public constructor")] protected override void SetParentCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent) { if (parent != null) From d9d3d07032f34eda754e9d7c18bf4447bdb46326 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 4 Apr 2023 13:28:54 -0700 Subject: [PATCH 5/7] Applly more feedbacks --- .../Reflection/Emit/ModuleBuilderImpl.cs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index c0739e32408fd..07dde6a0746ee 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -13,14 +13,16 @@ internal sealed class ModuleBuilderImpl : ModuleBuilder { private readonly Assembly _coreAssembly; private readonly string _name; - private Type[]? _coreTypes; + private Type?[]? _coreTypes; private readonly Dictionary _assemblyRefStore = new(); private readonly Dictionary _typeRefStore = new(); private readonly List _typeDefStore = new(); private int _nextMethodDefRowId = 1; private int _nextFieldDefRowId = 1; + private bool _coreTypesFullPopulated; private static readonly Type[] s_coreTypes = { typeof(void), typeof(object), typeof(bool), typeof(char), typeof(sbyte), typeof(byte), typeof(short), typeof(ushort), typeof(int), typeof(uint), typeof(long), typeof(ulong), typeof(float), typeof(double), typeof(string), typeof(nint), typeof(nuint) }; + internal ModuleBuilderImpl(string name, Assembly coreAssembly) { _coreAssembly = coreAssembly; @@ -36,19 +38,16 @@ internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) if (_coreAssembly == typeof(object).Assembly) { _coreTypes = s_coreTypes; + _coreTypesFullPopulated = true; } else { _coreTypes = new Type[s_coreTypes.Length]; - - for (int i = 0; i < s_coreTypes.Length; i++) - { - _coreTypes[i] = _coreAssembly.GetType(s_coreTypes[i].FullName!, throwOnError: true)!; - } } } - return _coreTypes[(int)typeId]; + int index = (int)typeId; + return _coreTypes[index] ?? (_coreTypes[index] = _coreAssembly.GetType(s_coreTypes[index].FullName!, throwOnError: true)!); } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Types are preserved via s_coreTypes")] @@ -60,16 +59,24 @@ internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) if (_coreAssembly == typeof(object).Assembly) { _coreTypes = s_coreTypes; + _coreTypesFullPopulated = true; } else { _coreTypes = new Type[s_coreTypes.Length]; + } + } - for (int i = 0; i < s_coreTypes.Length; i++) + if (!_coreTypesFullPopulated) + { + for (int i = 0; i < _coreTypes.Length; i++) + { + if (_coreTypes[i] == null) { - _coreTypes[i] = _coreAssembly.GetType(s_coreTypes[i].FullName!, throwOnError: true)!; + _coreTypes[i] = _coreAssembly.GetType(s_coreTypes[i].FullName!, throwOnError: false)!; } } + _coreTypesFullPopulated = true; } for (int i = 0; i < _coreTypes.Length; i++) From 78b3c867e111d136a2026f585f142c4049fcc9d2 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Apr 2023 13:45:56 -0700 Subject: [PATCH 6/7] Apply suggestions from code review --- .../System/Reflection/Emit/SignatureHelper.cs | 105 +++++++++--------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index 9b3824f4dbb16..1fc54971527da 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -56,62 +56,61 @@ private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder sig CoreTypeId? typeId = module.GetTypeIdFromCoreTypes(type); // We need to translate from Reflection.Type to SignatureTypeEncoder. - if (typeId.HasValue) + switch (typeId) { - switch (typeId) - { - case CoreTypeId.Boolean: - signature.Boolean(); - return; - case CoreTypeId.Byte: - signature.Byte(); - return; - case CoreTypeId.SByte: - signature.SByte(); - return; - case CoreTypeId.Char: - signature.Char(); - return; - case CoreTypeId.Int16: - signature.Int16(); - break; - case CoreTypeId.UInt16: - signature.UInt16(); - return; - case CoreTypeId.Int32: - signature.Int32(); - return; - case CoreTypeId.UInt32: - signature.UInt32(); - return; - case CoreTypeId.Int64: - signature.Int64(); - return; - case CoreTypeId.UInt64: - signature.UInt64(); - return; - case CoreTypeId.Single: - signature.Single(); - return; - case CoreTypeId.Double: - signature.Double(); - return; - case CoreTypeId.IntPtr: - signature.IntPtr(); - return; - case CoreTypeId.UIntPtr: - signature.UIntPtr(); - return; - case CoreTypeId.Object: - signature.Object(); - return; - case CoreTypeId.String: - signature.String(); - return; - } + case CoreTypeId.Boolean: + signature.Boolean(); + break; + case CoreTypeId.Byte: + signature.Byte(); + break; + case CoreTypeId.SByte: + signature.SByte(); + break; + case CoreTypeId.Char: + signature.Char(); + break; + case CoreTypeId.Int16: + signature.Int16(); + break; + case CoreTypeId.UInt16: + signature.UInt16(); + break; + case CoreTypeId.Int32: + signature.Int32(); + break; + case CoreTypeId.UInt32: + signature.UInt32(); + break; + case CoreTypeId.Int64: + signature.Int64(); + break; + case CoreTypeId.UInt64: + signature.UInt64(); + break; + case CoreTypeId.Single: + signature.Single(); + break; + case CoreTypeId.Double: + signature.Double(); + break; + case CoreTypeId.IntPtr: + signature.IntPtr(); + break; + case CoreTypeId.UIntPtr: + signature.UIntPtr(); + break; + case CoreTypeId.Object: + signature.Object(); + break; + case CoreTypeId.String: + signature.String(); + break; + default: + throw new NotSupportedException(SR.Format(SR.NotSupported_Signature, type.FullName)); + } } - throw new NotSupportedException(SR.Format(SR.NotSupported_Signature, type.FullName)); } } From bae024378ee7b6612059db9de11a9e0518ae5cde Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Apr 2023 13:46:27 -0700 Subject: [PATCH 7/7] Update src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs --- .../src/System/Reflection/Emit/SignatureHelper.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index 1fc54971527da..ad950b15152ac 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -109,8 +109,6 @@ private static void WriteSignatureTypeForReflectionType(SignatureTypeEncoder sig default: throw new NotSupportedException(SR.Format(SR.NotSupported_Signature, type.FullName)); } - } - } }