From f27478ca00703cc514e1683958f21e2fb4122fc3 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 14 Mar 2024 09:59:44 -0700 Subject: [PATCH 1/3] Fix: Using TypeBuilder as generic argument for a parent type throws --- .../Reflection/Emit/ModuleBuilderImpl.cs | 16 +++++++-------- .../System/Reflection/Emit/TypeBuilderImpl.cs | 20 ++++++++++++++----- .../AssemblySaveTypeBuilderTests.cs | 17 ++++++++++++++++ 3 files changed, 40 insertions(+), 13 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 c79e95962e3db..946a1269aa0e2 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 @@ -609,7 +609,7 @@ private static MemberInfo GetOriginalMemberIfConstructedType(MethodBase methodBa Type declaringType = methodBase.DeclaringType!; if (declaringType.IsConstructedGenericType && declaringType.GetGenericTypeDefinition() is not TypeBuilderImpl && - !ContainsNotBakedTypeBuilder(declaringType.GetGenericArguments())) + !ContainsTypeBuilder(declaringType.GetGenericArguments())) { return declaringType.GetGenericTypeDefinition().GetMemberWithSameMetadataDefinitionAs(methodBase); } @@ -883,7 +883,7 @@ private static int GetTokenForHandle(EntityHandle handle) private EntityHandle GetHandleForMember(MemberInfo member) { - if (IsConstructedFromNotBakedTypeBuilder(member.DeclaringType!)) + if (IsConstructedFromTypeBuilder(member.DeclaringType!)) { return default; } @@ -891,11 +891,11 @@ private EntityHandle GetHandleForMember(MemberInfo member) return GetMemberReferenceHandle(member); } - private static bool IsConstructedFromNotBakedTypeBuilder(Type type) => type.IsConstructedGenericType && + private static bool IsConstructedFromTypeBuilder(Type type) => type.IsConstructedGenericType && (type.GetGenericTypeDefinition() is TypeBuilderImpl || - ContainsNotBakedTypeBuilder(type.GetGenericArguments())); + ContainsTypeBuilder(type.GetGenericArguments())); - private static bool ContainsNotBakedTypeBuilder(Type[] genericArguments) + internal static bool ContainsTypeBuilder(Type[] genericArguments) { foreach (Type type in genericArguments) { @@ -904,7 +904,7 @@ private static bool ContainsNotBakedTypeBuilder(Type[] genericArguments) return true; } - if (IsConstructedFromNotBakedTypeBuilder(type)) + if (IsConstructedFromTypeBuilder(type)) { return true; } @@ -925,7 +925,7 @@ internal EntityHandle TryGetTypeHandle(Type type) return eb._typeBuilder._handle; } - if (IsConstructedFromNotBakedTypeBuilder(type)) + if (IsConstructedFromTypeBuilder(type)) { return default; } @@ -967,7 +967,7 @@ private static bool IsArrayMethodFromNotBakedTypeBuilder(MethodInfo method) => m arrayMethod.DeclaringType!.GetElementType() is TypeBuilderImpl tb && tb._handle == default; private static bool IsConstructedMethodFromNotBakedMethodBuilder(MethodInfo method) => method.IsConstructedGenericMethod && - (method.GetGenericMethodDefinition() is MethodBuilderImpl mb && mb._handle == default || ContainsNotBakedTypeBuilder(method.GetGenericArguments())); + (method.GetGenericMethodDefinition() is MethodBuilderImpl mb && mb._handle == default || ContainsTypeBuilder(method.GetGenericArguments())); internal EntityHandle TryGetMethodHandle(MethodInfo method, Type[] optionalParameterTypes) { 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 4aea4ab3ee2c6..d70adf02611b1 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 @@ -210,13 +210,23 @@ protected override ConstructorBuilder DefineDefaultConstructorCore(MethodAttribu private ConstructorBuilderImpl DefineDefaultConstructorInternal(MethodAttributes attributes) { // Get the parent class's default constructor and add it to the IL - ConstructorInfo? con; - if (_typeParent!.IsConstructedGenericType && _typeParent.GetGenericTypeDefinition() is TypeBuilderImpl typeBuilder) + ConstructorInfo? con = null; + if (_typeParent!.IsConstructedGenericType) { - con = GetConstructor(_typeParent, typeBuilder.GetConstructor( - BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); + Type typeDefinition = _typeParent.GetGenericTypeDefinition(); + if (typeDefinition is TypeBuilderImpl typeBuilder) + { + con = GetConstructor(_typeParent, typeBuilder.GetConstructor( + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); + } + else if (ModuleBuilderImpl.ContainsTypeBuilder(_typeParent.GetGenericArguments())) + { + con = GetConstructor(_typeParent, typeDefinition.GetConstructor( + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); + } } - else + + if (con == null) { con = _typeParent.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null); } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index 1713d91ba4578..31536bc266c0a 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -649,6 +649,23 @@ public void MethodBuilderGetParametersReturnParameterTest() Assert.Null(method3.ReturnParameter.Name); Assert.True(method3.ReturnParameter.IsRetval); } + + public class BaseType { } + + [Fact] + public void GenericTypeWithTypeBuilderGenericParameter_UsedAsParent() + { + PersistedAssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndTypeBuilder(out TypeBuilder typeBuilder); + + Type type = typeBuilder.CreateType(); + var baseType = typeof(BaseType<>).GetGenericTypeDefinition().MakeGenericType(type); + + var typeBuilder2 = ab.GetDynamicModule("MyModule") + .DefineType("TestService", TypeAttributes.Public | TypeAttributes.Class, baseType); + typeBuilder2.CreateType(); + + Assert.NotNull(type.GetConstructor(Type.EmptyTypes)); // Default constructor created + } } // Test Types From 387a7c6ac3535209dd526e8c9e44c0776bf44f8f Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 14 Mar 2024 17:13:30 -0700 Subject: [PATCH 2/3] No need to branch out for constructed generic parent --- .../System/Reflection/Emit/TypeBuilderImpl.cs | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) 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 d70adf02611b1..a698d744212a6 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 @@ -210,23 +210,15 @@ protected override ConstructorBuilder DefineDefaultConstructorCore(MethodAttribu private ConstructorBuilderImpl DefineDefaultConstructorInternal(MethodAttributes attributes) { // Get the parent class's default constructor and add it to the IL - ConstructorInfo? con = null; - if (_typeParent!.IsConstructedGenericType) + ConstructorInfo? con; + if (_typeParent!.IsConstructedGenericType && + (_typeParent.GetGenericTypeDefinition() is TypeBuilderImpl || ModuleBuilderImpl.ContainsTypeBuilder(_typeParent.GetGenericArguments()))) { - Type typeDefinition = _typeParent.GetGenericTypeDefinition(); - if (typeDefinition is TypeBuilderImpl typeBuilder) - { - con = GetConstructor(_typeParent, typeBuilder.GetConstructor( - BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); - } - else if (ModuleBuilderImpl.ContainsTypeBuilder(_typeParent.GetGenericArguments())) - { - con = GetConstructor(_typeParent, typeDefinition.GetConstructor( - BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); - } + // When TypeBuilder involved need to construct the parent constructor using TypeBuilder.GetConstructor() static method + con = GetConstructor(_typeParent, _typeParent.GetGenericTypeDefinition().GetConstructor( + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null)!); } - - if (con == null) + else { con = _typeParent.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, EmptyTypes, null); } From 94991f0c2ed41f988923d4ef0041e87147d461d1 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 14 Mar 2024 21:05:55 -0700 Subject: [PATCH 3/3] Update method name --- .../System/Reflection/Emit/ModuleBuilderImpl.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 946a1269aa0e2..0349186b717fe 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 @@ -954,8 +954,8 @@ internal EntityHandle TryGetMethodHandle(MethodInfo method) return mb._handle; } - if (IsConstructedMethodFromNotBakedMethodBuilder(method) || - IsArrayMethodFromNotBakedTypeBuilder(method)) + if (IsConstructedFromMethodBuilderOrTypeBuilder(method) || + IsArrayMethodTypeIsTypeBuilder(method)) { return default; } @@ -963,11 +963,11 @@ internal EntityHandle TryGetMethodHandle(MethodInfo method) return GetHandleForMember(method); } - private static bool IsArrayMethodFromNotBakedTypeBuilder(MethodInfo method) => method is ArrayMethod arrayMethod && - arrayMethod.DeclaringType!.GetElementType() is TypeBuilderImpl tb && tb._handle == default; + private static bool IsArrayMethodTypeIsTypeBuilder(MethodInfo method) => method is ArrayMethod arrayMethod && + arrayMethod.DeclaringType!.GetElementType() is TypeBuilderImpl; - private static bool IsConstructedMethodFromNotBakedMethodBuilder(MethodInfo method) => method.IsConstructedGenericMethod && - (method.GetGenericMethodDefinition() is MethodBuilderImpl mb && mb._handle == default || ContainsTypeBuilder(method.GetGenericArguments())); + private static bool IsConstructedFromMethodBuilderOrTypeBuilder(MethodInfo method) => method.IsConstructedGenericMethod && + (method.GetGenericMethodDefinition() is MethodBuilderImpl || ContainsTypeBuilder(method.GetGenericArguments())); internal EntityHandle TryGetMethodHandle(MethodInfo method, Type[] optionalParameterTypes) { @@ -982,7 +982,7 @@ internal EntityHandle TryGetMethodHandle(MethodInfo method, Type[] optionalParam return mb._handle; } - if (IsConstructedMethodFromNotBakedMethodBuilder(method)) + if (IsConstructedFromMethodBuilderOrTypeBuilder(method)) { return default; }