From 32b8f1020753857f2dfe46b3d48dca2e1c706a08 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 3 Dec 2024 13:11:55 -0600 Subject: [PATCH 1/5] Support generic fields in PersistedAssemblyBuilder --- .../Reflection/Emit/ModuleBuilderImpl.cs | 26 +++++- .../AssemblySaveTypeBuilderTests.cs | 84 ++++++++++++++++++- 2 files changed, 105 insertions(+), 5 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 bbd52a5b7a0dfa..5e9921925a8c7f 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 @@ -734,13 +734,15 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo) { case FieldInfo field: Type declaringType = field.DeclaringType!; - if (field.DeclaringType!.IsGenericTypeDefinition) + if (declaringType.IsGenericTypeDefinition) { //The type of the field has to be fully instantiated type. declaringType = declaringType.MakeGenericType(declaringType.GetGenericArguments()); } + memberHandle = AddMemberReference(field.Name, GetTypeHandle(declaringType), - MetadataSignatureHelper.GetFieldSignature(field.FieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this)); + MetadataSignatureHelper.GetFieldSignature(GetFieldType(field), field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this)); + break; case ConstructorInfo ctor: ctor = (ConstructorInfo)GetOriginalMemberIfConstructedType(ctor); @@ -770,6 +772,26 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo) } return memberHandle; + + // The field type has to be the original open generic field type, not the closed type. + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", Justification = "Internal reflection implementation")] + static Type GetFieldType(FieldInfo fieldInfo) + { + if (fieldInfo.DeclaringType!.IsGenericType) + { + fieldInfo = fieldInfo.DeclaringType!.GetGenericTypeDefinition().GetField(fieldInfo.Name, GetBindingFlags(fieldInfo))!; + } + + return fieldInfo.FieldType; + + static BindingFlags GetBindingFlags(FieldInfo fieldInfo) + { + BindingFlags flags = BindingFlags.Default; + flags |= fieldInfo.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic; + flags |= fieldInfo.IsStatic ? BindingFlags.Static : BindingFlags.Instance; + return flags; + } + } } private EntityHandle GetMethodReference(MethodInfo methodInfo, Type[] optionalParameterTypes) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index 31536bc266c0a1..248630820601d2 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -5,6 +5,8 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; using Xunit; namespace System.Reflection.Emit.Tests @@ -124,11 +126,15 @@ public void CreateMembersThatUsesTypeLoadedFromCoreAssemblyTest() } } - private static TypeBuilder CreateAssemblyAndDefineType(out PersistedAssemblyBuilder assemblyBuilder) + private static ModuleBuilder CreateAssembly(out PersistedAssemblyBuilder assemblyBuilder) { assemblyBuilder = AssemblySaveTools.PopulateAssemblyBuilder(s_assemblyName); - return assemblyBuilder.DefineDynamicModule("MyModule") - .DefineType("TestInterface", TypeAttributes.Interface | TypeAttributes.Abstract); + return assemblyBuilder.DefineDynamicModule("MyModule"); + } + + private static TypeBuilder CreateAssemblyAndDefineType(out PersistedAssemblyBuilder assemblyBuilder) + { + return CreateAssembly(out assemblyBuilder).DefineType("TestInterface", TypeAttributes.Interface | TypeAttributes.Abstract); } [Fact] @@ -205,6 +211,78 @@ public void SaveGenericTypeParametersForAType(string[] typeParamNames) } } + private class GenericClassWithGenericField + { + public T F; + } + + private class GenericClassWithNonGenericField + { + public int F; + } + + public static IEnumerable GenericTypesWithField() + { + yield return new object[] { typeof(GenericClassWithGenericField), true }; + yield return new object[] { typeof(GenericClassWithNonGenericField), false }; + } + + [Theory] + [MemberData(nameof(GenericTypesWithField))] + public void SaveGenericField(Type declaringType, bool shouldFieldBeGeneric) + { + using (TempFile file = TempFile.Create()) + { + ModuleBuilder mb = CreateAssembly(out PersistedAssemblyBuilder assemblyBuilder); + TypeBuilder tb = mb.DefineType("C", TypeAttributes.Class); + MethodBuilder method = tb.DefineMethod("TestMethod", MethodAttributes.Public, returnType: typeof(int), parameterTypes: null); + ILGenerator il = method.GetILGenerator(); + il.Emit(OpCodes.Newobj, declaringType.GetConstructor([])); + il.Emit(OpCodes.Ldfld, declaringType.GetField("F")); + il.Emit(OpCodes.Ret); + Type createdType = tb.CreateType(); + assemblyBuilder.Save(file.Path); + + using (FileStream stream = File.OpenRead(file.Path)) + { + using (PEReader peReader = new PEReader(stream)) + { + bool found = false; + MetadataReader metadataReader = peReader.GetMetadataReader(); + foreach (MemberReferenceHandle memberRefHandle in metadataReader.MemberReferences) + { + MemberReference memberRef = metadataReader.GetMemberReference(memberRefHandle); + if (memberRef.GetKind() == MemberReferenceKind.Field) + { + Assert.False(found); + found = true; + + Assert.Equal("F", metadataReader.GetString(memberRef.Name)); + + // The field handle should point to the open generic field, and not the resolved generic type. + Assert.True(IsGenericField(metadataReader.GetBlobReader(memberRef.Signature))); + } + } + + Assert.Equal(shouldFieldBeGeneric, found); + } + } + } + + static bool IsGenericField(BlobReader signatureReader) + { + while (signatureReader.RemainingBytes > 0) + { + if (signatureReader.ReadSignatureTypeCode() == SignatureTypeCode.GenericTypeParameter) + { + return true; + } + } + + return false; + } + } + private static void SetVariousGenericParameterValues(GenericTypeParameterBuilder[] typeParams) { typeParams[0].SetInterfaceConstraints([typeof(IAccess), typeof(INoMethod)]); From 989fcc63dd3ddc7a833f4f4a6dd3e78a91432ea0 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 3 Dec 2024 14:48:58 -0600 Subject: [PATCH 2/5] Add the pragmas for unused fields --- .../PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index 248630820601d2..c31158b6252ef8 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -213,12 +213,16 @@ public void SaveGenericTypeParametersForAType(string[] typeParamNames) private class GenericClassWithGenericField { +//#pragma warning disable CS0649 public T F; +//#pragma warning restore CS0649 } private class GenericClassWithNonGenericField { +//#pragma warning disable CS0649 public int F; +//#pragma warning restore CS0649 } public static IEnumerable GenericTypesWithField() From dc4c0af4cb4eb43384ececca40b095310cb8b414 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 3 Dec 2024 16:16:58 -0600 Subject: [PATCH 3/5] Add the pragmas for unused fields 2 --- .../AssemblySaveTypeBuilderTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index c31158b6252ef8..397df091ea1d5e 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -213,16 +213,16 @@ public void SaveGenericTypeParametersForAType(string[] typeParamNames) private class GenericClassWithGenericField { -//#pragma warning disable CS0649 +#pragma warning disable CS0649 public T F; -//#pragma warning restore CS0649 +#pragma warning restore CS0649 } private class GenericClassWithNonGenericField { -//#pragma warning disable CS0649 +#pragma warning disable CS0649 public int F; -//#pragma warning restore CS0649 +#pragma warning restore CS0649 } public static IEnumerable GenericTypesWithField() From 709cfcebd9c9502ab07c28f44f083673b10d7398 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 4 Dec 2024 08:03:06 -0600 Subject: [PATCH 4/5] Fix test for the non-generic case --- .../AssemblySaveTypeBuilderTests.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index 397df091ea1d5e..f508da07364f26 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -263,12 +263,12 @@ public void SaveGenericField(Type declaringType, bool shouldFieldBeGeneric) Assert.Equal("F", metadataReader.GetString(memberRef.Name)); - // The field handle should point to the open generic field, and not the resolved generic type. - Assert.True(IsGenericField(metadataReader.GetBlobReader(memberRef.Signature))); + // A reference to a generic field should point to the open generic field, and not the resolved generic type. + Assert.Equal(shouldFieldBeGeneric, IsGenericField(metadataReader.GetBlobReader(memberRef.Signature))); } } - Assert.Equal(shouldFieldBeGeneric, found); + Assert.True(found); } } } @@ -277,7 +277,8 @@ static bool IsGenericField(BlobReader signatureReader) { while (signatureReader.RemainingBytes > 0) { - if (signatureReader.ReadSignatureTypeCode() == SignatureTypeCode.GenericTypeParameter) + SignatureTypeCode typeCode = signatureReader.ReadSignatureTypeCode(); + if (typeCode == SignatureTypeCode.GenericTypeParameter) { return true; } From 6296219f2a6d2b739fc646ad054ff5ba592f6e4e Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 18 Dec 2024 12:17:47 -0600 Subject: [PATCH 5/5] Re-use existing GetOriginalMemberIfConstructedType() --- .../Reflection/Emit/ModuleBuilderImpl.cs | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 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 5e9921925a8c7f..2ead8405732022 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 @@ -740,8 +740,9 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo) declaringType = declaringType.MakeGenericType(declaringType.GetGenericArguments()); } + Type fieldType = ((FieldInfo)GetOriginalMemberIfConstructedType(field)).FieldType; memberHandle = AddMemberReference(field.Name, GetTypeHandle(declaringType), - MetadataSignatureHelper.GetFieldSignature(GetFieldType(field), field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this)); + MetadataSignatureHelper.GetFieldSignature(fieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this)); break; case ConstructorInfo ctor: @@ -772,26 +773,6 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo) } return memberHandle; - - // The field type has to be the original open generic field type, not the closed type. - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", Justification = "Internal reflection implementation")] - static Type GetFieldType(FieldInfo fieldInfo) - { - if (fieldInfo.DeclaringType!.IsGenericType) - { - fieldInfo = fieldInfo.DeclaringType!.GetGenericTypeDefinition().GetField(fieldInfo.Name, GetBindingFlags(fieldInfo))!; - } - - return fieldInfo.FieldType; - - static BindingFlags GetBindingFlags(FieldInfo fieldInfo) - { - BindingFlags flags = BindingFlags.Default; - flags |= fieldInfo.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic; - flags |= fieldInfo.IsStatic ? BindingFlags.Static : BindingFlags.Instance; - return flags; - } - } } private EntityHandle GetMethodReference(MethodInfo methodInfo, Type[] optionalParameterTypes) @@ -831,17 +812,17 @@ internal static SignatureCallingConvention GetSignatureConvention(CallingConvent return convention; } - private MemberInfo GetOriginalMemberIfConstructedType(MethodBase methodBase) + private MemberInfo GetOriginalMemberIfConstructedType(MemberInfo memberInfo) { - Type declaringType = methodBase.DeclaringType!; + Type declaringType = memberInfo.DeclaringType!; if (declaringType.IsConstructedGenericType && declaringType.GetGenericTypeDefinition() is not TypeBuilderImpl && !ContainsTypeBuilder(declaringType.GetGenericArguments())) { - return declaringType.GetGenericTypeDefinition().GetMemberWithSameMetadataDefinitionAs(methodBase); + return declaringType.GetGenericTypeDefinition().GetMemberWithSameMetadataDefinitionAs(memberInfo); } - return methodBase; + return memberInfo; } private static Type[] ParameterTypes(ParameterInfo[] parameterInfos)