Abstract out ParameterBuilder and implement it (#85446)

* Save parameters into assembly and test it * Add MarshalAs pseduo attribute handling and other refactoring * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> --------- Co-authored-by: Jan Kotas <[email protected]>
dotnet · Apr 28, 2023 · dd95ed8 · dd95ed8
1 parent da75e6d
commit dd95ed8
Show file tree

Hide file tree

Showing 23 changed files with 804 additions and 662 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj
@@ -163,7 +163,6 @@
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\DynamicMethod.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\ILGenerator.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\LocalBuilder.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\ParameterBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeAssemblyBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeConstructorBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeEnumBuilder.cs" />
@@ -172,6 +171,7 @@
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeGenericTypeParameterBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeMethodBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeModuleBuilder.cs" />
+    <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeParameterBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimePropertyBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\RuntimeTypeBuilder.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\SignatureHelper.cs" />

diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeMethodBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeMethodBuilder.cs
@@ -646,7 +646,7 @@ protected override ParameterBuilder DefineParameterCore(int position, ParameterA
                 throw new ArgumentOutOfRangeException(SR.ArgumentOutOfRange_ParamSequence);
 
             attributes &= ~ParameterAttributes.ReservedMask;
-            return new ParameterBuilder(this, position, attributes, strParamName);
+            return new RuntimeParameterBuilder(this, position, attributes, strParamName);
         }
 
         protected override void SetImplementationFlagsCore(MethodImplAttributes attributes)

diff --git a/...ystem/Reflection/Emit/ParameterBuilder.cs → ...eflection/Emit/RuntimeParameterBuilder.cs b/...ystem/Reflection/Emit/ParameterBuilder.cs → ...eflection/Emit/RuntimeParameterBuilder.cs
@@ -5,10 +5,10 @@
 
 namespace System.Reflection.Emit
 {
-    public class ParameterBuilder
+    internal sealed class RuntimeParameterBuilder : ParameterBuilder
     {
         // Set the default value of the parameter
-        public virtual void SetConstant(object? defaultValue)
+        public override void SetConstant(object? defaultValue)
         {
             RuntimeTypeBuilder.SetConstantValue(
                 _methodBuilder.GetModuleBuilder(),
@@ -17,28 +17,16 @@ public virtual void SetConstant(object? defaultValue)
                 defaultValue);
         }
 
-        // Use this function if client decides to form the custom attribute blob themselves
-        public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute)
+        protected override void SetCustomAttributeCore(ConstructorInfo con, ReadOnlySpan<byte> binaryAttribute)
         {
-            ArgumentNullException.ThrowIfNull(con);
-            ArgumentNullException.ThrowIfNull(binaryAttribute);
-
             RuntimeTypeBuilder.DefineCustomAttribute(
                 _methodBuilder.GetModuleBuilder(),
                 _token,
                 ((RuntimeModuleBuilder)_methodBuilder.GetModule()).GetMethodMetadataToken(con),
                 binaryAttribute);
         }
 
-        // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder
-        public void SetCustomAttribute(CustomAttributeBuilder customBuilder)
-        {
-            ArgumentNullException.ThrowIfNull(customBuilder);
-
-            customBuilder.CreateCustomAttribute((RuntimeModuleBuilder)(_methodBuilder.GetModule()), _token);
-        }
-
-        internal ParameterBuilder(
+        internal RuntimeParameterBuilder(
             RuntimeMethodBuilder methodBuilder,
             int sequence,
             ParameterAttributes attributes,
@@ -62,17 +50,11 @@ internal int GetToken()
             return _token;
         }
 
-        public virtual string? Name => _name;
-
-        public virtual int Position => _position;
-
-        public virtual int Attributes => (int)_attributes;
-
-        public bool IsIn => (_attributes & ParameterAttributes.In) != 0;
+        public override string? Name => _name;
 
-        public bool IsOut => (_attributes & ParameterAttributes.Out) != 0;
+        public override int Position => _position;
 
-        public bool IsOptional => (_attributes & ParameterAttributes.Optional) != 0;
+        public override int Attributes => (int)_attributes;
 
         private readonly string? _name;
         private readonly int _position;

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
@@ -145,7 +145,6 @@
     <Compile Include="System\Reflection\Emit\DynamicMethod.cs" />
     <Compile Include="System\Reflection\Emit\ILGenerator.cs" />
     <Compile Include="System\Reflection\Emit\LocalBuilder.cs" />
-    <Compile Include="System\Reflection\Emit\ParameterBuilder.cs" />
     <Compile Include="System\Reflection\Emit\ReflectionEmitThrower.cs" />
     <Compile Include="System\Reflection\Emit\SignatureHelper.cs" />
     <Compile Include="System\Reflection\EnumInfo.cs" />

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs
diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
@@ -646,6 +646,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\OpCodeType.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\OperandType.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\PackingSize.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\ParameterBuilder.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\PEFileKinds.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\PropertyBuilder.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Reflection\Emit\StackBehaviour.cs" />

diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs
@@ -0,0 +1,35 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace System.Reflection.Emit
+{
+    public abstract partial class ParameterBuilder
+    {
+        protected ParameterBuilder() { }
+        public virtual int Attributes => throw new NotImplementedException();
+        public bool IsIn => ((ParameterAttributes)Attributes & ParameterAttributes.In) != 0;
+        public bool IsOptional => ((ParameterAttributes)Attributes & ParameterAttributes.Optional) != 0;
+        public bool IsOut => ((ParameterAttributes)Attributes & ParameterAttributes.Out) != 0;
+        public virtual string? Name => throw new NotImplementedException();
+        public virtual int Position => throw new NotImplementedException();
+        public virtual void SetConstant(object? defaultValue) => throw new NotImplementedException();
+        public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute)
+        {
+            ArgumentNullException.ThrowIfNull(con);
+            ArgumentNullException.ThrowIfNull(binaryAttribute);
+
+            SetCustomAttributeCore(con, binaryAttribute);
+        }
+        protected abstract void SetCustomAttributeCore(ConstructorInfo con, ReadOnlySpan<byte> binaryAttribute);
+        public void SetCustomAttribute(CustomAttributeBuilder customBuilder)
+        {
+            ArgumentNullException.ThrowIfNull(customBuilder);
+
+            SetCustomAttributeCore(customBuilder.Ctor, customBuilder.Data);
+        }
+    }
+}
diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs b/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs
@@ -72,9 +72,9 @@ internal LocalBuilder() { }
         public override int LocalIndex { get { throw null; } }
         public override System.Type LocalType { get { throw null; } }
     }
-    public partial class ParameterBuilder
+    public abstract partial class ParameterBuilder
     {
-        internal ParameterBuilder() { }
+        protected ParameterBuilder() { }
         public virtual int Attributes { get { throw null; } }
         public bool IsIn { get { throw null; } }
         public bool IsOptional { get { throw null; } }
@@ -84,6 +84,7 @@ internal ParameterBuilder() { }
         public virtual void SetConstant(object? defaultValue) { }
         public void SetCustomAttribute(System.Reflection.ConstructorInfo con, byte[] binaryAttribute) { }
         public void SetCustomAttribute(System.Reflection.Emit.CustomAttributeBuilder customBuilder) { }
+        protected abstract void SetCustomAttributeCore(System.Reflection.ConstructorInfo con, System.ReadOnlySpan<byte> binaryAttribute);
     }
     public sealed partial class SignatureHelper
     {

diff --git a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx
@@ -126,9 +126,6 @@
   <data name="InvalidOperation_BadInterfaceNotAbstract" xml:space="preserve">
     <value>Interface must be declared abstract.</value>
   </data>
-  <data name="NotSupported_Signature" xml:space="preserve">
-    <value>The signature {0} is not supported.</value>
-  </data>
   <data name="InvalidOperation_AModuleRequired" xml:space="preserve">
     <value>Assembly needs at least one module defined.</value>
   </data>
@@ -141,11 +138,11 @@
   <data name="NotSupported_DynamicModule" xml:space="preserve">
     <value>The invoked member is not supported in a dynamic module.</value>
   </data>
-  <data name="Argument_InvalidTypeArgument" xml:space="preserve">
+  <data name="Argument_InvalidTypeCodeForTypeArgument" xml:space="preserve">
     <value>The type code may not be used as a type argument of a custom attribute .</value>
   </data>
-  <data name="InvalidOperation_EmptyFieldForCustomAttribute" xml:space="preserve">
-    <value>Custom attribute '{0}' doesn't contain a field named '{1}'.</value>
+  <data name="NotSupported_UnmanagedTypeOnlyForFields" xml:space="preserve">
+    <value> 'UnmanagedType.{0}' named parameter is only valid for fields.</value>
   </data>
   <data name="Argument_InvalidCustomAttributeLength" xml:space="preserve">
     <value>Custom attribute '{0}' data length is only '{1}'.</value>
@@ -162,4 +159,16 @@
   <data name="Argument_DllNameCannotBeEmpty" xml:space="preserve">
     <value>DllName cannot be empty.</value>
   </data>
+  <data name="ArgumentOutOfRange_ParamSequence" xml:space="preserve">
+    <value>The specified parameter index is not in range.</value>
+  </data>
+  <data name="Argument_InvalidArgumentForAttribute" xml:space="preserve">
+    <value>Invalid constructor argument {0} provided for MarshalAs atttribute.</value>
+  </data>
+  <data name="Argument_InvalidParameterForUnmanagedType" xml:space="preserve">
+    <value>Named parameter {0} is not valid for UnmanagedType.{1} type.</value>
+  </data>
+  <data name="Argument_SizeConstMustBeSpecified" xml:space="preserve">
+    <value>SizeConst parameter must be specified for UnmanagedType.ByValTStr type.</value>
+  </data>
 </root>
diff --git a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj
@@ -10,6 +10,8 @@
     <Compile Include="System\Reflection\Emit\FieldBuilderImpl.cs" />
     <Compile Include="System\Reflection\Emit\MethodBuilderImpl.cs" />
     <Compile Include="System\Reflection\Emit\ModuleBuilderImpl.cs" />
+    <Compile Include="System\Reflection\Emit\ParameterBuilderImpl.cs" />
+    <Compile Include="System\Reflection\Emit\PseudoCustomAttributesData.cs" />
     <Compile Include="System\Reflection\Emit\TypeBuilderImpl.cs" />
     <Compile Include="System\Reflection\Emit\SignatureHelper.cs" />
   </ItemGroup>

diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/CustomAttributeWrapper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/CustomAttributeWrapper.cs
@@ -2,8 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Buffers.Binary;
-using System.Diagnostics.CodeAnalysis;
+using System.Diagnostics;
 using System.Reflection.Metadata;
+using System.Reflection.Metadata.Ecma335;
+using System.Runtime.InteropServices;
 
 namespace System.Reflection.Emit
 {
@@ -171,7 +173,7 @@ private static Type ElementTypeToType(PrimitiveSerializationTypeCode elementType
                 PrimitiveSerializationTypeCode.Single => typeof(float),
                 PrimitiveSerializationTypeCode.Double => typeof(double),
                 PrimitiveSerializationTypeCode.String => typeof(string),
-                _ => throw new ArgumentException(SR.Argument_InvalidTypeArgument, "binaryAttribute"),
+                _ => throw new ArgumentException(SR.Argument_InvalidTypeCodeForTypeArgument, "binaryAttribute"),
             };
     }
 }