[API Proposal]: Abstract System.Reflection.Emit public types #78542

Activity

1. ghost added

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Nov 18, 2022
2. ghost added

   area-System.Reflection

   on Nov 18, 2022
4. buyaa-n

   added this to the 8.0.0 milestone on Nov 18, 2022
5. ghost removed

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Nov 18, 2022
6. buyaa-n

   added

   api-ready-for-reviewAPI is ready for review, it is NOT ready for implementationAPI is ready for review, it is NOT ready for implementation

   on Nov 18, 2022
7. ghost added

   in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

   on Nov 18, 2022
8. teo-tsirpanis

   added

   area-System.Reflection.Emit

   and removed

   area-System.Reflection

   on Nov 18, 2022
10. jkotas commented on Nov 18, 2022

    

    jkotas

    on Nov 18, 2022

    Last edited by jkotas

    Member

    More actions

    It may be worth pointing out that a simple non-abstract -> abstract change is a potential breaking change.

    You are unlikely to see the break in C# since C# tends to call non-virtual methods using callvirt IL instructions. It is easy to construct a repro for the problem calling these methods using call IL instruction using managed C++, IL, etc.

    The proposed PR avoids the potential break change by defining the methods as abstract in the reference assembly, but as virtual forwarders in the implementation. See https://github.com/dotnet/runtime/pull/78544/files#diff-6c7674e7748fd5ff0c1f949de79d769c7e9a6b4cf3453b22b58972c5250141c8R18-R21 for an example.

    Are we ok with having this permanent difference between reference and implementation assemblies? Or do we want to address this issue in some other way?

    cc @ericstj

    👍React with 👍1Reacted by Buyaa Namnan
11. bartonjs commented on Nov 18, 2022

    

    bartonjs

    on Nov 18, 2022

    Last edited by bartonjs

    Member

    More actions

    Are we ok with having this permanent difference between reference and implementation assemblies? Or do we want to address this issue in some other way?

    I think addressing the issue via the Template Method Pattern is better than a ref-vs-src divergence (and other trickery).

    For methods, it's a simple Foo gains a call to abstract FooCore. For properties it probably requires making an abstract GetFoo/SetFoo.

    -   public sealed partial class TypeBuilder : System.Reflection.TypeInfo
    +   public abstract partial class TypeBuilder : System.Reflection.TypeInfo
        {
    -        internal TypeBuilder()
    +        private protected TypeBuilder()
             {
                 ...
             }
    
    -        public System.Reflection.Emit.PackingSize PackingSize { get; }
    +        public System.Reflection.Emit.PackingSize PackingSize => PackingSizeCore;
    +        protected abstract System.Reflection.Emit.PackingSize PackingSizeCore { get; }
     
    ...
    
            public void AddInterfaceImplementation(
                [DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes.All)] Type interfaceType)
            {
                ArgumentNullException.ThrowIfNull(interfaceType);
     
                ThrowIfCreated();
    
    -           all the rest of the implementation
    +           AddInterfaceImplementationCore(interfaceType);
    +       }
    +       
    +       protected abstract void AddInterfaceImplementationCore(
    +           [DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes.All)] Type interfaceType);
    
    ...

    👍React with 👍1Reacted by Eric StJohn
12. ericstj commented on Nov 18, 2022

    

    ericstj

    on Nov 18, 2022

    Member

    More actions

    Another option would be to just leave them virtual in the reference assembly. If we thought folks would forget to override them, we could add an analyzer.
13. jkotas commented on Nov 19, 2022

    

    jkotas

    on Nov 19, 2022

    Member

    More actions

    Template Method Pattern

    Is the Template Method Pattern really applicable here? There is no interesting algorithm implemented in the base class (I do not consider argument null checks to be an interesting algorithm.)

    Another option would be to just leave them virtual in the reference assembly.

    I like this option better.

    The idea behind this change is to open Reflection.Emit for custom implementations in similar way how System.Type is open for custom implementations. Do we think that it is a good idea? We need this capability for our own purposes as @buyaa-n described, but it is not strictly required to make this capability available through public surface.
14. steveharter commented on Dec 8, 2022

    

    steveharter

    on Dec 8, 2022

    Last edited by steveharter

    Contributor

    More actions

    Another option would be to just leave them virtual in the reference assembly.

    This will also cause infinite recursion if base.Foo() is called:

    recursion sample

    using System.Reflection;
    using System.Reflection.Emit;
    
    namespace ConsoleApp1
    {
        internal class Program
        {
            static void Main()
            {
                MethodInfo mi = typeof(AssemblyBuilder).GetMethod(nameof(AssemblyBuilder.DoSomething));
                DynamicMethod dm = new(name: "yikes", returnType: null, parameterTypes: new Type[] { typeof(AssemblyBuilder) });
                ILGenerator il = dm.GetILGenerator();
                il.Emit(OpCodes.Ldarg_0);
                il.Emit(OpCodes.Call, mi); // direct call not virtual
                il.Emit(OpCodes.Ret);
                var m = dm.CreateDelegate<Action<AssemblyBuilder>>();
                m(new AssemblyBuilderDerived());
            }
        }
    
        public abstract class AssemblyBuilder
        {
            public virtual void DoSomething()
            {
                // shouldn't normally be called, but for binary compat it may be
                DoSomething(); // Call the virtual method
            }
        }
    
        internal sealed class AssemblyBuilderDerived : AssemblyBuilder
        {
            public override void DoSomething()
            {
                // Since base is not abstract, we are allowed to call it but causes an infinite loop
                base.DoSomething();
            }
        }
    }
    

    Options - (2) and (4) seem better:
    1) abstract in ref+impl
    Summary: don't do any extra work to avoid breaking changes. Potential runtime breaking change fo languages other than CSharp (or compilers that don't use callvirt by default). If caller re-compiles, there is no runtime breaking change since their code will not have callvirt instead of call
    Pros: no extra public\protected methods
    Cons: no binary compat if called by non-C# with "Opcodes.Call" (not Callvirt) - APIs will throw
    2) abstract in ref; virtual in impl. Allows a this.Foo() in impl base class to forward to the override.
    Summary: avoids the runtime breaking change in (1) by using "ref\impl trickery"
    Pros: binary compat; no extra public\protected methods
    Cons: not conventional; issues in future?
    3) virtual in ref+impl.
    Summary: an attempt at a more "standard" approach than (2) to avoid the breaking change (no trickery)
    but can cause issues with infinite loop
    Pros: binary compat; no extra public\protected methods
    Cons: can cause infinite loop if base.Foo() called
    Cons: derived class can forget to override
    4) Template Method Pattern which adds protected "impl" methods
    Summary: a more "standard" approach than (2) to avoid the breaking change (no trickery)**
    Pros: binary compat; more mainstream approach
    Cons: doubles the public\protected method count; slower
    5) Use a forwarding approach to a new impl class
    Summary: a more "standard" approach than (2) to avoid the breaking change but is heavy-weight**
    Pros: binary compat
    Cons: doubles the type, instance and method count; slower + more GC

    Does anyone foresee issues with option 2 with the different ref\impl, or have any reference implementations of that approach? If we go down that path and run into an issue in a future release, and have to make it truly abstract in the impl (i.e. move to option 1), then we would lose the binary compat since we'd have to remove the forwarding method.

    👍React with 👍1Reacted by Buyaa Namnan
15. 7 remaining items

    Load more

16. jkotas commented on Jan 10, 2023

    

    jkotas

    on Jan 10, 2023

    Member

    More actions

    For C# I believe call is generated when having both the caller and callee in the same module, otherwise callvirt is generated.

    I do not think that's the case. C# generates call when it can prove that the this pointer is non-null and the method is non-virtual. For example, try this static void M(ModuleBuilder m) => m?.CreateGlobalFunctions();. The CreateGlobalFunctions is in different module, but it is going to be called via call today since Roslyn can prove that the this pointer is non-null.

    👍React with 👍1Reacted by Joe4evr
17. bartonjs commented on Jan 17, 2023

    

    bartonjs

    on Jan 17, 2023

    Last edited by dotnet-api-review

    Member

    More actions

    Video

    • Everything that was proposed as (non-virtual) to abstract should instead be non-virtual to virtual (don't mismatch across the ref and src)
    • Consider instead using the Template Method Pattern, and introducing the virtualness through (e.g. protected abstract [AssemblyBuilder::]DefineDynamicModuleCore(string name))
    • Does everything that is being changed to virtual need to be? For example, the parameterless ConstructorBuilder.GetILGenerator() probably doesn't need a virtual implementation.
    • ModuleBuilder.GetTypeToken(Type) is already public in .NET Framework, but with a different signature, we need to converge (or change the name)
      • Probably applies to the other "new" methods on ModuleBuilder

    namespace System.Reflection.Emit
    {
    -    public sealed partial class AssemblyBuilder : System.Reflection.Assembly
    +    public abstract partial class AssemblyBuilder : System.Reflection.Assembly
        {
    -        internal AssemblyBuilder();
    +        protected AssemblyBuilder();
    -        public System.Reflection.Emit.ModuleBuilder DefineDynamicModule(string name);
    +        public virtual System.Reflection.Emit.ModuleBuilder DefineDynamicModule(string name);
    -        public System.Reflection.Emit.ModuleBuilder? GetDynamicModule(string name);
    +        public virtual System.Reflection.Emit.ModuleBuilder? GetDynamicModule(string name);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
       }
           
    -    public sealed partial class ConstructorBuilder : System.Reflection.ConstructorInfo
    +    public abstract partial class ConstructorBuilder : System.Reflection.ConstructorInfo
        {
    -        internal ConstructorBuilder();
    +        protected ConstructorBuilder();
    -        public bool InitLocals { get; set; }
    +        public virtual bool InitLocals { get; set; }
    -        public System.Reflection.Emit.ParameterBuilder DefineParameter(int iSequence, System.Reflection.ParameterAttributes attributes, string? strParamName);
    +        public virtual System.Reflection.Emit.ParameterBuilder DefineParameter(int iSequence, System.Reflection.ParameterAttributes attributes, string? strParamName);
    -        public System.Reflection.Emit.ILGenerator GetILGenerator();
    +        public virtual ILGenerator GetILGenerator();
    -        public System.Reflection.Emit.ILGenerator GetILGenerator(int streamSize);
    +        public virtual System.Reflection.Emit.ILGenerator GetILGenerator(int streamSize);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetImplementationFlags(System.Reflection.MethodImplAttributes attributes;
    +        public virtual void SetImplementationFlags(System.Reflection.MethodImplAttributes attributes);
        }
        
    -    public sealed partial class EnumBuilder : System.Reflection.TypeInfo
    +    public abstract partial class EnumBuilder : System.Reflection.TypeInfo
        {
    -        internal EnumBuilder();
    +        protected EnumBuilder();
    -        public System.Reflection.Emit.FieldBuilder UnderlyingField { get; }
    +        public virtual System.Reflection.Emit.FieldBuilder UnderlyingField { get ; }
    -        public System.Reflection.TypeInfo CreateTypeInfo();
    +        public virtual System.Reflection.TypeInfo CreateTypeInfo();
    -        public System.Reflection.Emit.FieldBuilder DefineLiteral(string literalName, object? literalValue);
    +        public virtual System.Reflection.Emit.FieldBuilder DefineLiteral(string literalName, object? literalValue);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
        }
        
    -    public sealed partial class EventBuilder
    +    public abstract partial class EventBuilder
        {
    -        internal EventBuilder();
    +        protected EventBuilder();
    -        public void AddOtherMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void AddOtherMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    -        public void SetAddOnMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void SetAddOnMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetRaiseMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void SetRaiseMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    -        public void SetRemoveOnMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void SetRemoveOnMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
        }
        
    -    public sealed partial class FieldBuilder : System.Reflection.FieldInfo
    +    public abstract partial class FieldBuilder : System.Reflection.FieldInfo
        {
    -        internal FieldBuilder();
    +        protected FieldBuilder();
    -        public void SetConstant(object? defaultValue);
    +        public virtual void SetConstant(object? defaultValue);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder)
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetOffset(int iOffset);
    +        public virtual void SetOffset(int iOffset);
        }
        
    -    public sealed partial class GenericTypeParameterBuilder : System.Reflection.TypeInfo
    +    public abstract partial class GenericTypeParameterBuilder : System.Reflection.TypeInfo
        {
    -        internal GenericTypeParameterBuilder();
    +        protected GenericTypeParameterBuilder();
    -         public void SetBaseTypeConstraint([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? baseTypeConstraint);
    +        public virtual void SetBaseTypeConstraint([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? baseTypeConstraint);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetGenericParameterAttributes(System.Reflection.GenericParameterAttributes genericParameterAttributes);
    +        public virtual void SetGenericParameterAttributes(System.Reflection.GenericParameterAttributes genericParameterAttributes);
    -        public void SetInterfaceConstraints(params System.Type[]? interfaceConstraints);
    +        public virtual void SetInterfaceConstraints(params System.Type[]? interfaceConstraints);
        }
        
    -    public sealed partial class MethodBuilder : System.Reflection.MethodInfo
    +    public abstract partial class MethodBuilder : System.Reflection.MethodInfo
        {
    -        internal MethodBuilder();
    +        protected MethodBuilder();
    -        public bool InitLocals { get; set; }
    +        public virtual bool InitLocals { get; set; }
    -        public System.Reflection.Emit.GenericTypeParameterBuilder[] DefineGenericParameters(params string[] names);
    +        public virtual System.Reflection.Emit.GenericTypeParameterBuilder[] DefineGenericParameters(params string[] names);
    -        public System.Reflection.Emit.ParameterBuilder DefineParameter(int position, System.Reflection.ParameterAttributes attributes, string? strParamName);
    +        public virtual System.Reflection.Emit.ParameterBuilder DefineParameter(int position, System.Reflection.ParameterAttributes attributes, string? strParamName);
    -        public System.Reflection.Emit.ILGenerator GetILGenerator();
    +        public virtual System.Reflection.Emit.ILGenerator GetILGenerator();
    -        public System.Reflection.Emit.ILGenerator GetILGenerator(int size);
    +        public virtual System.Reflection.Emit.ILGenerator GetILGenerator(int size);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetImplementationFlags(System.Reflection.MethodImplAttributes attributes);
    +        public virtual void SetImplementationFlags(System.Reflection.MethodImplAttributes attributes);
    -        public void SetParameters(params System.Type[] parameterTypes);
    +        public virtual void SetParameters(params System.Type[] parameterTypes);
    -        public void SetReturnType(System.Type? returnType);
    +        public virtual void SetReturnType(System.Type? returnType);
    -        public void SetSignature(System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
    +        public virtual void SetSignature(System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
        }
        
    -    public partial class ModuleBuilder : System.Reflection.Module
    +    public abstract partial class ModuleBuilder : System.Reflection.Module
        {
    -        internal ModuleBuilder();
    +        protected ModuleBuilder();
    -        public void CreateGlobalFunctions();
    +        public virtual void CreateGlobalFunctions();
    -        public System.Reflection.Emit.EnumBuilder DefineEnum(string name, System.Reflection.TypeAttributes visibility, System.Type underlyingType);
    +        public virtual System.Reflection.Emit.EnumBuilder DefineEnum(string name, System.Reflection.TypeAttributes visibility, System.Type underlyingType);
    -        public System.Reflection.Emit.MethodBuilder DefineGlobalMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? requiredReturnTypeCustomModifiers, System.Type[]? optionalReturnTypeCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? requiredParameterTypeCustomModifiers, System.Type[][]? optionalParameterTypeCustomModifiers);
    +        public virtual System.Reflection.Emit.MethodBuilder DefineGlobalMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? requiredReturnTypeCustomModifiers, System.Type[]? optionalReturnTypeCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? requiredParameterTypeCustomModifiers, System.Type[][]? optionalParameterTypeCustomModifiers);
    -        public System.Reflection.Emit.FieldBuilder DefineInitializedData(string name, byte[] data, System.Reflection.FieldAttributes attributes) ;
    +        public virtual System.Reflection.Emit.FieldBuilder DefineInitializedData(string name, byte[] data, System.Reflection.FieldAttributes attributes);
    -        public System.Reflection.Emit.MethodBuilder DefinePInvokeMethod(string name, string dllName, string entryName, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? parameterTypes, System.Runtime.InteropServices.CallingConvention nativeCallConv, System.Runtime.InteropServices.CharSet nativeCharSet);
    +        public virtual System.Reflection.Emit.MethodBuilder DefinePInvokeMethod(string name, string dllName, string entryName, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? parameterTypes, System.Runtime.InteropServices.CallingConvention nativeCallConv, System.Runtime.InteropServices.CharSet nativeCharSet);
    -        public System.Reflection.Emit.TypeBuilder DefineType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Reflection.Emit.PackingSize packingSize, int typesize);
    +        public virtual System.Reflection.Emit.TypeBuilder DefineType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Reflection.Emit.PackingSize packingSize, int typesize);
    -        public System.Reflection.Emit.TypeBuilder DefineType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Type[]? interfaces);
    +        public virtual System.Reflection.Emit.TypeBuilder DefineType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Type[]? interfaces);
    -        public System.Reflection.Emit.FieldBuilder DefineUninitializedData(string name, int size, System.Reflection.FieldAttributes attributes);
    +        public virtual System.Reflection.Emit.FieldBuilder DefineUninitializedData(string name, int size, System.Reflection.FieldAttributes attributes);
    -        public System.Reflection.MethodInfo GetArrayMethod(System.Type arrayClass, string methodName, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? parameterTypes);
    +        public virtual System.Reflection.MethodInfo GetArrayMethod(System.Type arrayClass, string methodName, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? parameterTypes);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    
    // New public APIs: these are existing internal methods used outside of the ModuleBuilder, so needs to make public 
    +        public virtual int GetTypeToken(Type type);
    +        public virtual int GetMethodToken(MethodInfo method);
    +        public virtual int GetFieldToken(FieldInfo field);
    +        public virtual int GetConstructorToken(ConstructorInfo con);
    +        public virtual int GetSignatureToken(SignatureHelper sigHelper);
    +        public virtual int GetStringConstant(string str);
        }
        
    -    public sealed partial class PropertyBuilder : System.Reflection.PropertyInfo
    +    public abstract partial class PropertyBuilder : System.Reflection.PropertyInfo
        {
    -        internal PropertyBuilder();
    +        protected PropertyBuilder();
    -        public void AddOtherMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void AddOtherMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    -        public void SetConstant(object? defaultValue);
    +        public virtual void SetConstant(object? defaultValue);
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetGetMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void SetGetMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    -        public void SetSetMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
    +        public virtual void SetSetMethod(System.Reflection.Emit.MethodBuilder mdBuilder);
        }
       
    -   public sealed partial class TypeBuilder : System.Reflection.TypeInfo
    +   public abstract partial class TypeBuilder : System.Reflection.TypeInfo
        {
    -        internal TypeBuilder();
    +        protected TypeBuilder();
    -        public System.Reflection.Emit.PackingSize PackingSize { get; }
    +        public virtual System.Reflection.Emit.PackingSize PackingSize { get; }
    -        public int Size { get; }
    +        public virtual int Size { get; }
    -        public void AddInterfaceImplementation([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type interfaceType);
    +        public virtual void AddInterfaceImplementation([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type interfaceType);
    -        public System.Reflection.TypeInfo CreateTypeInfo();
    +        public virtual System.Reflection.TypeInfo CreateTypeInfo();
    -        public System.Reflection.Emit.ConstructorBuilder DefineConstructor(System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type[]? parameterTypes, System.Type[][]? requiredCustomModifiers, System.Type[][]? optionalCustomModifiers);
    +        public virtual System.Reflection.Emit.ConstructorBuilder DefineConstructor(System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type[]? parameterTypes, System.Type[][]? requiredCustomModifiers, System.Type[][]? optionalCustomModifiers);
    -        public System.Reflection.Emit.ConstructorBuilder DefineDefaultConstructor(System.Reflection.MethodAttributes attributes);
    +        public virtual System.Reflection.Emit.ConstructorBuilder DefineDefaultConstructor(System.Reflection.MethodAttributes attributes);
    -        public System.Reflection.Emit.EventBuilder DefineEvent(string name, System.Reflection.EventAttributes attributes, System.Type eventtype);
    +        public virtual System.Reflection.Emit.EventBuilder DefineEvent(string name, System.Reflection.EventAttributes attributes, System.Type eventtype);
    -        public System.Reflection.Emit.FieldBuilder DefineField(string fieldName, System.Type type, System.Type[]? requiredCustomModifiers, System.Type[]? optionalCustomModifiers, System.Reflection.FieldAttributes attributes) { throw null; }
    +        public virtual System.Reflection.Emit.FieldBuilder DefineField(string fieldName, System.Type type, System.Type[]? requiredCustomModifiers, System.Type[]? optionalCustomModifiers, System.Reflection.FieldAttributes attributes);
    -        public System.Reflection.Emit.GenericTypeParameterBuilder[] DefineGenericParameters(params string[] names);
    +        public virtual System.Reflection.Emit.GenericTypeParameterBuilder[] DefineGenericParameters(params string[] names);
    -        public System.Reflection.Emit.FieldBuilder DefineInitializedData(string name, byte[] data, System.Reflection.FieldAttributes attributes);
    +        public virtual System.Reflection.Emit.FieldBuilder DefineInitializedData(string name, byte[] data, System.Reflection.FieldAttributes attributes);
    -        public System.Reflection.Emit.MethodBuilder DefineMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
    +        public virtual System.Reflection.Emit.MethodBuilder DefineMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
    -        public void DefineMethodOverride(System.Reflection.MethodInfo methodInfoBody, System.Reflection.MethodInfo methodInfoDeclaration);
    +        public virtual void DefineMethodOverride(System.Reflection.MethodInfo methodInfoBody, System.Reflection.MethodInfo methodInfoDeclaration);
    -        public System.Reflection.Emit.TypeBuilder DefineNestedType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Reflection.Emit.PackingSize packSize, int typeSize);
    +        public virtual System.Reflection.Emit.TypeBuilder DefineNestedType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Reflection.Emit.PackingSize packSize, int typeSize);
    -        public System.Reflection.Emit.TypeBuilder DefineNestedType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Type[]? interfaces);
    +        public virtual System.Reflection.Emit.TypeBuilder DefineNestedType(string name, System.Reflection.TypeAttributes attr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent, System.Type[]? interfaces);
    -        public System.Reflection.Emit.MethodBuilder DefinePInvokeMethod(string name, string dllName, string entryName, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers, System.Runtime.InteropServices.CallingConvention nativeCallConv, System.Runtime.InteropServices.CharSet nativeCharSet);
    +        public virtual System.Reflection.Emit.MethodBuilder DefinePInvokeMethod(string name, string dllName, string entryName, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers, System.Runtime.InteropServices.CallingConvention nativeCallConv, System.Runtime.InteropServices.CharSet nativeCharSet);
    -        public System.Reflection.Emit.PropertyBuilder DefineProperty(string name, System.Reflection.PropertyAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
    +        public virtual System.Reflection.Emit.PropertyBuilder DefineProperty(string name, System.Reflection.PropertyAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type returnType, System.Type[]? returnTypeRequiredCustomModifiers, System.Type[]? returnTypeOptionalCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? parameterTypeRequiredCustomModifiers, System.Type[][]? parameterTypeOptionalCustomModifiers);
    -        public System.Reflection.Emit.ConstructorBuilder DefineTypeInitializer();
    +        public virtual System.Reflection.Emit.ConstructorBuilder DefineTypeInitializer();
    -        public System.Reflection.Emit.FieldBuilder DefineUninitializedData(string name, int size, System.Reflection.FieldAttributes attributes);
    +        public virtual System.Reflection.Emit.FieldBuilder DefineUninitializedData(string name, int size, System.Reflection.FieldAttributes attributes);
    -        public bool IsCreated();
    +        public virtual bool IsCreated();
    -        public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    +        public virtual void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute);
    -        public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    +        public virtual void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder);
    -        public void SetParent([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent);
    +        public virtual void SetParent([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type? parent);
        }
    }
18. bartonjs

    added

    api-needs-workAPI needs work before it is approved, it is NOT ready for implementationAPI needs work before it is approved, it is NOT ready for implementation

    and removed

    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementationAPI is ready for review, it is NOT ready for implementation

    on Jan 17, 2023
19. teo-tsirpanis commented on Jan 17, 2023

    

    teo-tsirpanis

    on Jan 17, 2023

    Last edited by teo-tsirpanis

    Contributor

    More actions

    We should also give ILGenerator (and Label or maybe not) a similar treatment. Currently its constructors are internal.
20. buyaa-n commented on Jan 23, 2023

    

    buyaa-n

    on Jan 23, 2023

    Last edited by buyaa-n

    ContributorAuthor

    More actions

    • Does everything that is being changed to virtual need to be? For example, the parameterless ConstructorBuilder.GetILGenerator() probably doesn't need a virtual implementation.

    Yep, defaulting the parameter less implementation.

    • ModuleBuilder.GetTypeToken(Type) is already public in .NET Framework, but with a different signature, we need to converge (or change the name)
      • Probably applies to the other "new" methods on ModuleBuilder

    Even though they are doing same thing we don't want to add a wrapper type for tokens and metadata token is int for all other reflection APIs, therefore we renamed the methods into ModuleBuilder.GetMetadataToken(Type) with different overloads.

    • Everything that was proposed as (non-virtual) to abstract should instead be non-virtual to virtual (don't mismatch across the ref and src)
    • Consider instead using the Template Method Pattern, and introducing the virtualness through (e.g. protected abstract [AssemblyBuilder::]DefineDynamicModuleCore(string name))

    Followed the suggestion, new API proposal below:

    namespace System.Reflection.Emit
    {
    -    public sealed partial class AssemblyBuilder : System.Reflection.Assembly
    +    public abstract partial class AssemblyBuilder : System.Reflection.Assembly
        {
    -       internal AssemblyBuilder();
    +       protected AssemblyBuilder();
    +	protected abstract ModuleBuilder DefineDynamicModuleCore(string name);
    +	protected abstract ModuleBuilder? GetDynamicModuleCore(string name);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
        }
    -    public sealed partial class ConstructorBuilder : System.Reflection.ConstructorInfo
    +    public abstract partial class ConstructorBuilder : System.Reflection.ConstructorInfo
        {
    -       internal ConstructorBuilder();
    +       protected ConstructorBuilder();
    +	protected abstract bool InitLocalsCore { get; set; }
    +	protected abstract ParameterBuilder DefineParameterCore(int iSequence, ParameterAttributes attributes, string strParamName);
    +	protected abstract ILGenerator GetILGeneratorCore(int streamSize);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetImplementationFlagsCore(MethodImplAttributes attributes);
        }
    -    public sealed partial class EnumBuilder : System.Reflection.TypeInfo
    +    public abstract partial class EnumBuilder : System.Reflection.TypeInfo
        {
    -       internal EnumBuilder();
    +       protected EnumBuilder();
    +	protected abstract FieldBuilder UnderlyingFieldCore { get; }
    +	protected abstract TypeInfo CreateTypeInfoCore();
    +	protected abstract FieldBuilder DefineLiteralCore(string literalName, object? literalValue);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
        }
    -    public sealed partial class EventBuilder
    +    public abstract class EventBuilder
        {
    -       internal EventBuilder();
    +       protected EventBuilder();
    +	protected abstract void AddOtherMethodCore(MethodBuilder mdBuilder);
    +	protected abstract void SetAddOnMethodCore(MethodBuilder mdBuilder);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetRaiseMethodCore(MethodBuilder mdBuilder);
    +	protected abstract void SetRemoveOnMethodCore(MethodBuilder mdBuilder);
        }
    -    public sealed partial class FieldBuilder : System.Reflection.FieldInfo
    +    public abstract partial class FieldBuilder : System.Reflection.FieldInfo
        {
    -       internal FieldBuilder();
    +       protected FieldBuilder();
    +	protected abstract void SetConstantCore(object? defaultValue);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetOffsetCore(int iOffset);
        }
    -    public sealed partial class GenericTypeParameterBuilder : System.Reflection.TypeInfo
    +    public abstract partial class GenericTypeParameterBuilder : System.Reflection.TypeInfo
        {
    -       internal GenericTypeParameterBuilder();
    +       protected GenericTypeParameterBuilder();
    +	protected abstract void SetBaseTypeConstraintCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? baseTypeConstraint);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetGenericParameterAttributesCore(GenericParameterAttributes genericParameterAttributes);
    +	protected abstract void SetInterfaceConstraintsCore(params Type[]? interfaceConstraints);
        }
    -    public sealed partial class MethodBuilder : System.Reflection.MethodInfo
    +    public abstract partial class MethodBuilder : System.Reflection.MethodInfo
        {
    -       internal MethodBuilder();
    +       protected MethodBuilder();
    +	protected abstract bool InitLocalsCore { get; set; }
    +	protected abstract GenericTypeParameterBuilder[] DefineGenericParametersCore(params string[] names);
    +	protected abstract ParameterBuilder DefineParameterCore(int position, ParameterAttributes attributes, string? strParamName);
    +	protected abstract ILGenerator GetILGeneratorCore(int size);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetImplementationFlagsCore(MethodImplAttributes attributes);
    +	protected abstract void SetSignatureCore(Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers, Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers);
        }
    -    public partial class ModuleBuilder : System.Reflection.Module
    +    public abstract partial class ModuleBuilder : System.Reflection.Module
        {
    -       internal ModuleBuilder();
    +       protected ModuleBuilder();
    +	protected abstract void CreateGlobalFunctionsCore();
    +	protected abstract EnumBuilder DefineEnumCore(string name, TypeAttributes visibility, Type underlyingType);
    +	protected abstract MethodBuilder DefineGlobalMethodCore(string name, MethodAttributes attributes, CallingConventions callingConvention, Type? returnType, Type[]? requiredReturnTypeCustomModifiers, Type[]? optionalReturnTypeCustomModifiers, Type[]? parameterTypes, Type[][]? requiredParameterTypeCustomModifiers, Type[][]? optionalParameterTypeCustomModifiers);
    +	protected abstract FieldBuilder DefineInitializedDataCore(string name, byte[] data, FieldAttributes attributes);
    +	protected abstract MethodBuilder DefinePInvokeMethodCore(string name, string dllName, string entryName, MethodAttributes attributes, CallingConventions callingConvention, Type? returnType, Type[]? parameterTypes, CallingConvention nativeCallConv, CharSet nativeCharSet);
    +	protected abstract TypeBuilder DefineTypeCore(string name, TypeAttributes attr, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent, Type[]? interfaces, PackingSize packingSize, int typesize);
    +	protected abstract FieldBuilder DefineUninitializedDataCore(string name, int size, FieldAttributes attributes);
    +	protected abstract MethodInfo GetArrayMethodCore(Type arrayClass, string methodName, CallingConventions callingConvention, Type? returnType, Type[]? parameterTypes);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    	// New public APIs
    +	public abstract int GetMetadataToken(Type type);
    +       public abstract int GetMetadataToken(FieldInfo field);
    +       public abstract int GetMetadataToken(MethodInfo method);
    +       public abstract int GetMetadataToken(ConstructorInfo constructor);
    +       public abstract int GetMetadataToken(SignatureHelper sigHelper);
    +       public abstract int GetMetadataToken(string str);
        }
    -    public sealed partial class PropertyBuilder : System.Reflection.PropertyInfo
    +    public abstract partial class PropertyBuilder : System.Reflection.PropertyInfo
        {
    -       internal PropertyBuilder();
    +       protected PropertyBuilder();
    +	protected abstract void AddOtherMethodCore(MethodBuilder mdBuilder);
    +	protected abstract void SetConstantCore(object defaultValue);
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetGetMethodCore(MethodBuilder mdBuilder);
    +	protected abstract void SetSetMethodCore(MethodBuilder mdBuilder);
        }
    -   public sealed partial class TypeBuilder : System.Reflection.TypeInfo
    +    public abstract partial class TypeBuilder : System.Reflection.TypeInfo
        {
    -       internal TypeBuilder();
    +       protected TypeBuilder();
    +	protected abstract PackingSize PackingSizeCore { get; }
    +	protected abstract void AddInterfaceImplementationCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type interfaceType);
    +	protected abstract TypeInfo CreateTypeInfoCore();
    +	protected abstract ConstructorBuilder DefineConstructorCore(MethodAttributes attributes, CallingConventions callingConvention, Type[]? parameterTypes, Type[][]? requiredCustomModifiers, Type[][]? optionalCustomModifiers);
    +	protected abstract ConstructorBuilder DefineDefaultConstructorCore(MethodAttributes attributes);
    +	protected abstract EventBuilder DefineEventCore(string name, EventAttributes attributes, Type eventtype);
    +	protected abstract FieldBuilder DefineFieldCore(string fieldName, Type type, Type[]? requiredCustomModifiers, Type[]? optionalCustomModifiers, FieldAttributes attributes);
    +	protected abstract GenericTypeParameterBuilder[] DefineGenericParametersCore(params string[] names);
    +	protected abstract FieldBuilder DefineInitializedDataCore(string name, byte[] data, FieldAttributes attributes);
    +	protected abstract MethodBuilder DefineMethodCore(string name, MethodAttributes attributes, CallingConventions callingConvention, Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers, Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers);
    +	protected abstract void DefineMethodOverrideCore(MethodInfo methodInfoBody, MethodInfo methodInfoDeclaration);
    +	protected abstract TypeBuilder DefineNestedTypeCore(string name, TypeAttributes attr, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent, Type[]? interfaces, PackingSize packSize, int typeSize);
    +	protected abstract MethodBuilder DefinePInvokeMethodCore(string name, string dllName, string entryName, MethodAttributes attributes, CallingConventions callingConvention, Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers, Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers, CallingConvention nativeCallConv, CharSet nativeCharSet);
    +	protected abstract PropertyBuilder DefinePropertyCore(string name, PropertyAttributes attributes, CallingConventions callingConvention, Type returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers, Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers);
    +	protected abstract ConstructorBuilder DefineTypeInitializerCore();
    +	protected abstract FieldBuilder DefineUninitializedDataCore(string name, int size, FieldAttributes attributes);
    +	protected abstract bool IsCreatedCore();
    +	protected abstract void SetCustomAttributeCore(ConstructorInfo con, byte[] binaryAttribute);
    +	protected abstract void SetCustomAttributeCore(CustomAttributeBuilder customBuilder);
    +	protected abstract void SetParentCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent);
        }
    }

    CC @jkotas @steveharter @AaronRobinsonMSFT
21. buyaa-n

    added

    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementationAPI is ready for review, it is NOT ready for implementation

    and removed

    api-needs-workAPI needs work before it is approved, it is NOT ready for implementationAPI needs work before it is approved, it is NOT ready for implementation

    on Jan 23, 2023
22. terrajobst commented on Jan 24, 2023

    

    terrajobst

    on Jan 24, 2023

    Last edited by dotnet-api-review

    Contributor

    More actions

    Video

    Looks good as proposed except for the GetMetdataToken().

    We should have distinct method groups to avoid the (conceptual) ambiguity around what the string-based one does:

    • GetTypeMetadataToken(Type type)
    • GetFieldMetadataToken(FieldInfo field)
    • GetMethodMetadataToken(MethodInfo method)
    • GetMethodMetadataToken(ConstructorInfo constructor)
    • GetSignatureMetadataToken(SignatureHelper signature)
    • GetStringMetadataToken(string stringConstant)
23. terrajobst

    added

    api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implemented

    and removed

    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementationAPI is ready for review, it is NOT ready for implementation

    on Jan 24, 2023
24. buyaa-n

    removed

    blockingMarks issues that we want to fast track in order to unblock other important workMarks issues that we want to fast track in order to unblock other important work

    on Jan 26, 2023
25. buyaa-n

    closed this as completedin #78544on Jan 27, 2023
26. ghost removed

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Jan 27, 2023
27. ghost locked as resolved and limited conversation to collaborators on Feb 27, 2023