Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error handling for generic arguments constrained as Enums #72141

Merged
merged 1 commit into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,12 @@ public bool IsTypeDesc

return (MethodTable*)m_asTAddr;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TypeHandle TypeHandleOf<T>()
{
return new TypeHandle((void*)RuntimeTypeHandle.GetValueInternal(typeof(T).TypeHandle));
}
}

// Helper structs used for tail calls via helper.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,11 @@ private static bool FilterApplyMethodBase(

#region Private\Internal Members

internal unsafe TypeHandle GetNativeTypeHandle()
{
return new TypeHandle((void*)m_handle);
}

internal IntPtr GetUnderlyingNativeHandle()
{
return m_handle;
Expand Down Expand Up @@ -3360,7 +3365,56 @@ public override Guid GUID
[MethodImpl(MethodImplOptions.InternalCall)]
private extern void GetGUID(ref Guid result);

internal bool IsDelegate() => GetBaseType() == typeof(MulticastDelegate);
protected override unsafe bool IsValueTypeImpl()
{
TypeHandle th = GetNativeTypeHandle();

// We need to return true for generic parameters with the ValueType constraint.
if (th.IsTypeDesc)
return IsSubclassOf(typeof(ValueType));

bool isValueType = th.AsMethodTable()->IsValueType;
GC.KeepAlive(this);
return isValueType;
}

public override unsafe bool IsEnum
{
get
{
TypeHandle th = GetNativeTypeHandle();

// We need to return true for generic parameters with the Enum constraint.
if (th.IsTypeDesc)
return IsSubclassOf(typeof(Enum));

bool isEnum = th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf<Enum>().AsMethodTable();
GC.KeepAlive(this);
return isEnum;
}
}

// This returns true for actual enum types only.
internal unsafe bool IsActualEnum
{
get
{
TypeHandle th = GetNativeTypeHandle();

bool isEnum = !th.IsTypeDesc && th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf<Enum>().AsMethodTable();
GC.KeepAlive(this);
return isEnum;
}
}

internal unsafe bool IsDelegate()
{
TypeHandle th = GetNativeTypeHandle();

bool isDelegate = !th.IsTypeDesc && th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf<MulticastDelegate>().AsMethodTable();
GC.KeepAlive(this);
return isDelegate;
}

public override GenericParameterAttributes GenericParameterAttributes
{
Expand Down Expand Up @@ -3756,7 +3810,7 @@ internal bool TryByRefFastPath(ref object arg, ref bool isValueType)

internal static CorElementType GetUnderlyingType(RuntimeType type)
{
if (type.IsEnum)
if (type.IsActualEnum)
{
type = (RuntimeType)Enum.GetUnderlyingType(type);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public ref partial struct TypedReference
// for System.UIntPtr without inspecting the type desc any further. Otherwise, the type handle
// is just wrapping a method table pointer, so return that directly with a reinterpret cast.
MethodTable* pMethodTable = typeHandle.IsTypeDesc
? (MethodTable*)RuntimeTypeHandle.GetValueInternal(typeof(UIntPtr).TypeHandle)
? TypeHandle.TypeHandleOf<UIntPtr>().AsMethodTable()
: typeHandle.AsMethodTable();

Debug.Assert(pMethodTable is not null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ public abstract class RuntimeType : TypeInfo
// For desktop compatibility, do not bounce an incoming integer that's the wrong size.
// Do a value-preserving cast of both it and the enum values and do a 64-bit compare.

if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum);

return Enum.GetEnumName(this, rawValue);
}

public sealed override string[] GetEnumNames()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

string[] ret = Enum.InternalGetNames(this);
Expand All @@ -40,7 +40,7 @@ public sealed override string[] GetEnumNames()

public sealed override Type GetEnumUnderlyingType()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

return Enum.InternalGetUnderlyingType(this);
Expand All @@ -51,7 +51,7 @@ public sealed override bool IsEnumDefined(object value)
if (value == null)
throw new ArgumentNullException(nameof(value));

if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

if (value is string valueAsString)
Expand Down Expand Up @@ -94,7 +94,7 @@ public sealed override bool IsEnumDefined(object value)
[RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues<TEnum> overload instead.")]
public sealed override Array GetEnumValues()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

Array values = Enum.GetEnumInfo(this).ValuesAsUnderlyingType;
Expand All @@ -110,5 +110,8 @@ public sealed override Array GetEnumValues()
Array.Copy(values, result, values.Length);
return result;
}

internal bool IsActualEnum
=> TryGetEEType(out EETypePtr eeType) && eeType.IsEnum;
}
}
4 changes: 2 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Enum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1486,10 +1486,10 @@ private static RuntimeType ValidateRuntimeType(Type enumType)
{
ArgumentNullException.ThrowIfNull(enumType);

if (!enumType.IsEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, nameof(enumType));
if (enumType is not RuntimeType rtType)
throw new ArgumentException(SR.Arg_MustBeType, nameof(enumType));
if (!rtType.IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, nameof(enumType));
#if NATIVEAOT
// Check for the unfortunate "typeof(Outer<>.InnerEnum)" corner case.
// https://github.com/dotnet/runtime/issues/7976
Expand Down
27 changes: 8 additions & 19 deletions src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ public override MemberInfo[] GetDefaultMembers()
{
ArgumentNullException.ThrowIfNull(value);

Type valueType = value.GetType();
RuntimeType valueType = (RuntimeType)value.GetType();

if (!(valueType.IsEnum || IsIntegerType(valueType)))
if (!(valueType.IsActualEnum || IsIntegerType(valueType)))
throw new ArgumentException(SR.Arg_MustBeEnumBaseTypeOrEnum, nameof(value));

ulong ulValue = Enum.ToUInt64(value);
Expand All @@ -109,7 +109,7 @@ public override MemberInfo[] GetDefaultMembers()

public override string[] GetEnumNames()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

string[] ret = Enum.InternalGetNames(this);
Expand All @@ -121,7 +121,7 @@ public override string[] GetEnumNames()
[RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues<TEnum> overload instead.")]
public override Array GetEnumValues()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

// Get all of the values
Expand All @@ -141,7 +141,7 @@ public override Array GetEnumValues()

public override Type GetEnumUnderlyingType()
{
if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

return Enum.InternalGetUnderlyingType(this);
Expand Down Expand Up @@ -202,7 +202,7 @@ protected override TypeCode GetTypeCodeImpl()
typeCode = TypeCode.Decimal;
else if (ReferenceEquals(this, typeof(DateTime)))
typeCode = TypeCode.DateTime;
else if (IsEnum)
else if (IsActualEnum)
typeCode = GetTypeCode(Enum.InternalGetUnderlyingType(this));
else
typeCode = TypeCode.Object;
Expand Down Expand Up @@ -251,14 +251,14 @@ public override bool IsEnumDefined(object value)
{
ArgumentNullException.ThrowIfNull(value);

if (!IsEnum)
if (!IsActualEnum)
throw new ArgumentException(SR.Arg_MustBeEnum, "enumType");

// Check if both of them are of the same type
RuntimeType valueType = (RuntimeType)value.GetType();

// If the value is an Enum then we need to extract the underlying value from it
if (valueType.IsEnum)
if (valueType.IsActualEnum)
{
if (!valueType.IsEquivalentTo(this))
throw new ArgumentException(SR.Format(SR.Arg_EnumAndObjectMustBeSameType, valueType, this));
Expand Down Expand Up @@ -292,17 +292,6 @@ public override bool IsEnumDefined(object value)
}
}

protected override bool IsValueTypeImpl()
{
// We need to return true for generic parameters with the ValueType constraint.
// So we cannot use the faster RuntimeTypeHandle.IsValueType because it returns
// false for all generic parameters.
if (this == typeof(ValueType) || this == typeof(Enum))
return false;

return IsSubclassOf(typeof(ValueType));
}

protected override bool IsByRefImpl() => RuntimeTypeHandle.IsByRef(this);

protected override bool IsPrimitiveImpl() => RuntimeTypeHandle.IsPrimitive(this);
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Runtime/tests/System/EnumTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,21 @@ public void GetValues_EnumTypeNotEnum_ThrowsArgumentException(Type enumType)
AssertExtensions.Throws<ArgumentException>("enumType", () => Enum.GetValues(enumType));
}

private class ClassWithEnumConstraint<T> where T : Enum { }

[Fact]
public void EnumConstraint_ThrowsArgumentException()
{
Type genericArgumentWithEnumConstraint = typeof(ClassWithEnumConstraint<>).GetGenericArguments()[0];
Assert.True(genericArgumentWithEnumConstraint.IsEnum);

Assert.Throws<ArgumentException>(() => Enum.GetUnderlyingType(genericArgumentWithEnumConstraint));
Assert.Throws<ArgumentException>(() => Enum.IsDefined(genericArgumentWithEnumConstraint, 1));
Assert.Throws<ArgumentException>(() => Enum.GetName(genericArgumentWithEnumConstraint, 1));
Assert.Throws<ArgumentException>(() => Enum.GetNames(genericArgumentWithEnumConstraint));
Assert.Throws<ArgumentException>(() => Enum.GetValues(genericArgumentWithEnumConstraint));
}

public static IEnumerable<object[]> ToString_Format_TestData()
{
// Format "D": the decimal equivalent of the value is returned.
Expand Down
15 changes: 15 additions & 0 deletions src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,8 +1262,23 @@ internal bool IsDelegate()
return GetBaseType() == typeof(System.MulticastDelegate);
}

protected override bool IsValueTypeImpl()
{
// We need to return true for generic parameters with the ValueType constraint.
// So we cannot use the faster RuntimeTypeHandle.IsValueType because it returns
// false for all generic parameters.
if (this == typeof(ValueType) || this == typeof(Enum))
return false;

return IsSubclassOf(typeof(ValueType));
}

// Returns true for generic parameters with the Enum constraint.
public override bool IsEnum => GetBaseType() == EnumType;

// Returns true for actual enum types only.
internal bool IsActualEnum => RuntimeTypeHandle.GetBaseType(this) == EnumType;

public override GenericParameterAttributes GenericParameterAttributes
{
get
Expand Down