Remove validation that failing when setting constants with core assembly type

buyaa-n · buyaa-n · commit b74cd2bf7ede · 2024-08-14T15:07:10.000-07:00
diff --git a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx
@@ -303,4 +303,7 @@
   <data name="InvalidOperation_UnmatchingSymScope" xml:space="preserve">
     <value>Unmatching symbol scope.</value>
   </data>
+  <data name="Argument_MustBeEnum" xml:space="preserve">
+    <value>Type provided must be an Enum.</value>
+  </data>
 </root>
diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/FieldBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/FieldBuilderImpl.cs
@@ -40,78 +40,10 @@ internal FieldBuilderImpl(TypeBuilderImpl typeBuilder, string fieldName, Type ty
         protected override void SetConstantCore(object? defaultValue)
         {
             _typeBuilder.ThrowIfCreated();
-            ValidateDefaultValueType(defaultValue, _fieldType);
             _defaultValue = defaultValue;
             _attributes |= FieldAttributes.HasDefault;
         }
 
-        internal static void ValidateDefaultValueType(object? defaultValue, Type destinationType)
-        {
-            if (defaultValue == null)
-            {
-                // nullable value types can hold null value.
-                if (destinationType.IsValueType && !(destinationType.IsGenericType && destinationType.GetGenericTypeDefinition() == typeof(Nullable<>)))
-                {
-                    throw new ArgumentException(SR.Argument_ConstantNull);
-                }
-            }
-            else
-            {
-                Type sourceType = defaultValue.GetType();
-                // We should allow setting a constant value on a ByRef parameter
-                if (destinationType.IsByRef)
-                {
-                    destinationType = destinationType.GetElementType()!;
-                }
-
-                // Convert nullable types to their underlying type.
-                destinationType = Nullable.GetUnderlyingType(destinationType) ?? destinationType;
-
-                if (destinationType.IsEnum)
-                {
-                    Type underlyingType;
-                    if (destinationType is EnumBuilderImpl enumBldr)
-                    {
-                        underlyingType = enumBldr.GetEnumUnderlyingType();
-
-                        if (sourceType != enumBldr._typeBuilder.UnderlyingSystemType &&
-                            sourceType != underlyingType &&
-                            // If the source type is an enum, should not throw when the underlying types match
-                            sourceType.IsEnum &&
-                            sourceType.GetEnumUnderlyingType() != underlyingType)
-                        {
-                            throw new ArgumentException(SR.Argument_ConstantDoesntMatch);
-                        }
-                    }
-                    else if (destinationType is TypeBuilderImpl typeBldr)
-                    {
-                        underlyingType = typeBldr.UnderlyingSystemType;
-
-                        if (underlyingType == null || (sourceType != typeBldr.UnderlyingSystemType && sourceType != underlyingType))
-                        {
-                            throw new ArgumentException(SR.Argument_ConstantDoesntMatch);
-                        }
-                    }
-                    else
-                    {
-                        underlyingType = Enum.GetUnderlyingType(destinationType);
-
-                        if (sourceType != destinationType && sourceType != underlyingType)
-                        {
-                            throw new ArgumentException(SR.Argument_ConstantDoesntMatch);
-                        }
-                    }
-                }
-                else
-                {
-                    if (!destinationType.IsAssignableFrom(sourceType))
-                    {
-                        throw new ArgumentException(SR.Argument_ConstantDoesntMatch);
-                    }
-                }
-            }
-        }
-
         internal void SetData(byte[] data)
         {
             _rvaData = data;
diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ParameterBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ParameterBuilderImpl.cs
@@ -32,8 +32,6 @@ public ParameterBuilderImpl(MethodBuilderImpl methodBuilder, int sequence, Param
 
         public override void SetConstant(object? defaultValue)
         {
-            Type parameterType = _position == 0 ? _methodBuilder.ReturnType : _methodBuilder.ParameterTypes![_position - 1];
-            FieldBuilderImpl.ValidateDefaultValueType(defaultValue, parameterType);
             _defaultValue = defaultValue;
             _attributes |= ParameterAttributes.HasDefault;
         }
diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/PropertyBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/PropertyBuilderImpl.cs
@@ -57,7 +57,6 @@ protected override void AddOtherMethodCore(MethodBuilder mdBuilder)
         protected override void SetConstantCore(object? defaultValue)
         {
             _containingType.ThrowIfCreated();
-            FieldBuilderImpl.ValidateDefaultValueType(defaultValue, _propertyType);
             _defaultValue = defaultValue;
         }
 
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
@@ -616,23 +616,22 @@ public override Type GetGenericTypeDefinition()
         public override string? Namespace => _namespace;
         public override Assembly Assembly => _module.Assembly;
         public override Module Module => _module;
-        public override Type UnderlyingSystemType
+        public override Type UnderlyingSystemType => this;
+
+        public override Type GetEnumUnderlyingType()
         {
-            get
+            if (IsEnum)
             {
-                if (IsEnum)
+                if (_enumUnderlyingType == null)
                 {
-                    if (_enumUnderlyingType == null)
-                    {
-                        throw new InvalidOperationException(SR.InvalidOperation_NoUnderlyingTypeOnEnum);
-                    }
-
-                    return _enumUnderlyingType;
-                }
-                else
-                {
-                    return this;
+                    throw new InvalidOperationException(SR.InvalidOperation_NoUnderlyingTypeOnEnum);
                 }
+
+                return _enumUnderlyingType;
+            }
+            else
+            {
+                throw new ArgumentException(SR.Argument_MustBeEnum);
             }
         }
         public override bool IsSZArray => false;
diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveEnumBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveEnumBuilderTests.cs
@@ -37,7 +37,7 @@ public static IEnumerable<object[]> DefineLiteral_TestData()
             yield return new object[] { typeof(uint), (uint)1 };
 
             yield return new object[] { typeof(int), 0 };
-            yield return new object[] { typeof(int), 1 };
+            yield return new object[] { typeof(int), Test.Second };
 
             yield return new object[] { typeof(ulong), (ulong)0 };
             yield return new object[] { typeof(ulong), (ulong)1 };
diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySavePropertyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySavePropertyBuilderTests.cs
@@ -163,6 +163,24 @@ public void SetConstantVariousValues(Type returnType, object defaultValue)
             Assert.Equal(defaultValue, property.GetConstantValue());
         }
 
+        [Theory]
+        [MemberData(nameof(SetConstant_TestData))]
+        public void SetConstantVariousValuesMlcCoreAssembly(Type returnType, object defaultValue)
+        {
+            using (MetadataLoadContext mlc = new MetadataLoadContext(new CoreMetadataAssemblyResolver()))
+            {
+                PersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyDynamicAssembly"), mlc.CoreAssembly);
+                ModuleBuilder mb = ab.DefineDynamicModule("My Module");
+                Type returnTypeFromCore = returnType != typeof(PropertyBuilderTest11.Colors) ? mlc.CoreAssembly.GetType(returnType.FullName, true) : returnType;
+                TypeBuilder type = mb.DefineType("MyType", TypeAttributes.Public);
+
+                PropertyBuilder property = type.DefineProperty("TestProperty", PropertyAttributes.HasDefault, returnTypeFromCore, null);
+                property.SetConstant(defaultValue);
+
+                Assert.Equal(defaultValue, property.GetConstantValue());
+            }
+        }
+
         [Fact]
         public void SetCustomAttribute_ConstructorInfo_ByteArray_NullConstructorInfo_ThrowsArgumentNullException()
         {
@@ -194,7 +212,6 @@ public void Set_WhenTypeAlreadyCreated_ThrowsInvalidOperationException()
             MethodAttributes getMethodAttributes = MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.HideBySig;
             MethodBuilder method = type.DefineMethod("TestMethod", getMethodAttributes, typeof(int), null);
             method.GetILGenerator().Emit(OpCodes.Ret);
-            AssertExtensions.Throws<ArgumentException>(() => property.SetConstant((decimal)10));
             CustomAttributeBuilder customAttrBuilder = new CustomAttributeBuilder(typeof(IntPropertyAttribute).GetConstructor([typeof(int)]), [10]);
             type.CreateType();
 
@@ -204,18 +221,5 @@ public void Set_WhenTypeAlreadyCreated_ThrowsInvalidOperationException()
             Assert.Throws<InvalidOperationException>(() => property.SetConstant(1));
             Assert.Throws<InvalidOperationException>(() => property.SetCustomAttribute(customAttrBuilder));
         }
-
-        [Fact]
-        public void SetConstant_ValidationThrows()
-        {
-            AssemblySaveTools.PopulateAssemblyBuilderAndTypeBuilder(out TypeBuilder type);
-            FieldBuilder field = type.DefineField("TestField", typeof(int), FieldAttributes.Private);
-            PropertyBuilder property = type.DefineProperty("TestProperty", PropertyAttributes.HasDefault, typeof(int), null);
-
-            AssertExtensions.Throws<ArgumentException>(() => property.SetConstant((decimal)10));
-            AssertExtensions.Throws<ArgumentException>(() => property.SetConstant(null));
-            type.CreateType();
-            Assert.Throws<InvalidOperationException>(() => property.SetConstant(1));
-        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -32,8 +32,6 @@ public ParameterBuilderImpl(MethodBuilderImpl methodBuilder, int sequence, Param`
`32`	`32`
`33`	`33`	`public override void SetConstant(object? defaultValue)`
`34`	`34`	`{`
`35`		`- Type parameterType = _position == 0 ? _methodBuilder.ReturnType : _methodBuilder.ParameterTypes![_position - 1];`
`36`		`- FieldBuilderImpl.ValidateDefaultValueType(defaultValue, parameterType);`
`37`	`35`	`_defaultValue = defaultValue;`
`38`	`36`	`_attributes \|= ParameterAttributes.HasDefault;`
`39`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,6 @@ protected override void AddOtherMethodCore(MethodBuilder mdBuilder)`
`57`	`57`	`protected override void SetConstantCore(object? defaultValue)`
`58`	`58`	`{`
`59`	`59`	`_containingType.ThrowIfCreated();`
`60`		`- FieldBuilderImpl.ValidateDefaultValueType(defaultValue, _propertyType);`
`61`	`60`	`_defaultValue = defaultValue;`
`62`	`61`	`}`
`63`	`62`