Skip to content

Commit

Permalink
Avoid Attribute.GetCustomAttributes() returning null for open generic…
Browse files Browse the repository at this point in the history
… type

* Revert "Revert "Avoid Attribute.GetCustomAttributes() returning null for open generic type (#65237)" (#66508)"

This reverts commit f99ba2e.

* Make DynamicMethod.GetCustomAttributes() return well-typed arrays.

This change makes DynamicMethod.GetCustomAttributes() compatible with
Attribute.GetCustomAttributes().
  • Loading branch information
madelson committed Mar 14, 2022
1 parent 4b93e52 commit 917a0b1
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 71 deletions.
16 changes: 6 additions & 10 deletions src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,8 @@ private static AttributeUsageAttribute InternalGetAttributeUsage(Type type)
SR.Format(SR.Format_AttributeUsage, type));
}

private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount)
{
return (Attribute[])Array.CreateInstance(elementType, elementCount);
}
private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount) =>
elementType.ContainsGenericParameters ? new Attribute[elementCount] : (Attribute[])Array.CreateInstance(elementType, elementCount);
#endregion

#endregion
Expand All @@ -459,7 +457,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, Type attribu
{
MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, attributeType, inherit),
MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, attributeType, inherit),
_ => (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!,
_ => (Attribute[])element.GetCustomAttributes(attributeType, inherit)
};
}

Expand All @@ -474,7 +472,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, bool inherit
{
MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, typeof(Attribute), inherit),
MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, typeof(Attribute), inherit),
_ => (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!,
_ => (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit)
};
}

Expand Down Expand Up @@ -536,25 +534,23 @@ public static Attribute[] GetCustomAttributes(ParameterInfo element!!, Type attr
if (element.Member == null)
throw new ArgumentException(SR.Argument_InvalidParameterInfo, nameof(element));


MemberInfo member = element.Member;
if (member.MemberType == MemberTypes.Method && inherit)
return InternalParamGetCustomAttributes(element, attributeType, inherit);

return (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!;
return (Attribute[])element.GetCustomAttributes(attributeType, inherit);
}

public static Attribute[] GetCustomAttributes(ParameterInfo element!!, bool inherit)
{
if (element.Member == null)
throw new ArgumentException(SR.Argument_InvalidParameterInfo, nameof(element));


MemberInfo member = element.Member;
if (member.MemberType == MemberTypes.Method && inherit)
return InternalParamGetCustomAttributes(element, null, inherit);

return (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!;
return (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit);
}

public static bool IsDefined(ParameterInfo element, Type attributeType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,15 +624,18 @@ public override object Invoke(object? obj, BindingFlags invokeAttr, Binder? bind
throw new ArgumentException(SR.Argument_MustBeRuntimeMethodInfo, "this");
}

public override object[] GetCustomAttributes(Type attributeType, bool inherit)
public override object[] GetCustomAttributes(Type attributeType!!, bool inherit)
{
if (attributeType == null)
throw new ArgumentNullException(nameof(attributeType));
if (attributeType.UnderlyingSystemType is not RuntimeType attributeRuntimeType)
throw new ArgumentException(SR.Arg_MustBeType, nameof(attributeType));

if (attributeType.IsAssignableFrom(typeof(MethodImplAttribute)))
return new object[] { new MethodImplAttribute((MethodImplOptions)GetMethodImplementationFlags()) };
else
return Array.Empty<object>();
bool includeMethodImplAttribute = attributeType.IsAssignableFrom(typeof(MethodImplAttribute));
object[] result = CustomAttribute.CreateAttributeArrayHelper(attributeRuntimeType, includeMethodImplAttribute ? 1 : 0);
if (includeMethodImplAttribute)
{
result[0] = new MethodImplAttribute((MethodImplOptions)GetMethodImplementationFlags());
}
return result;
}

public override object[] GetCustomAttributes(bool inherit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp
Debug.Assert(caType is not null);

if (type.GetElementType() is not null)
return (caType.IsValueType) ? Array.Empty<object>() : CreateAttributeArrayHelper(caType, 0);
return CreateAttributeArrayHelper(caType, 0);

if (type.IsGenericType && !type.IsGenericTypeDefinition)
type = (type.GetGenericTypeDefinition() as RuntimeType)!;
Expand All @@ -919,8 +919,6 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp

RuntimeType.ListBuilder<object> result = default;
bool mustBeInheritable = false;
bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters);
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;

for (int i = 0; i < pcas.Count; i++)
result.Add(pcas[i]);
Expand All @@ -932,7 +930,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp
type = (type.BaseType as RuntimeType)!;
}

object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
for (int i = 0; i < result.Count; i++)
{
typedResult[i] = result[i];
Expand Down Expand Up @@ -963,8 +961,6 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy

RuntimeType.ListBuilder<object> result = default;
bool mustBeInheritable = false;
bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters);
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;

for (int i = 0; i < pcas.Count; i++)
result.Add(pcas[i]);
Expand All @@ -976,7 +972,7 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy
method = method.GetParentDefinition()!;
}

object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
for (int i = 0; i < result.Count; i++)
{
typedResult[i] = result[i];
Expand Down Expand Up @@ -1123,16 +1119,13 @@ private static bool IsCustomAttributeDefined(
}

private static object[] GetCustomAttributes(
RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType? attributeFilterType)
RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType attributeFilterType)
{
RuntimeType.ListBuilder<object> attributes = default;

AddCustomAttributes(ref attributes, decoratedModule, decoratedMetadataToken, attributeFilterType, false, default);

bool useObjectArray = attributeFilterType is null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters;
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : attributeFilterType!;

object[] result = CreateAttributeArrayHelper(arrayType, attributes.Count + pcaCount);
object[] result = CreateAttributeArrayHelper(attributeFilterType, attributes.Count + pcaCount);
for (int i = 0; i < attributes.Count; i++)
{
result[i] = attributes[i];
Expand Down Expand Up @@ -1439,6 +1432,42 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA

return attributeUsageAttribute ?? AttributeUsageAttribute.Default;
}

internal static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount)
{
bool useAttributeArray = false;
bool useObjectArray = false;

if (caType == typeof(Attribute))
{
useAttributeArray = true;
}
else if (caType.IsValueType)
{
useObjectArray = true;
}
else if (caType.ContainsGenericParameters)
{
if (caType.IsSubclassOf(typeof(Attribute)))
{
useAttributeArray = true;
}
else
{
useObjectArray = true;
}
}

if (useAttributeArray)
{
return elementCount == 0 ? Array.Empty<Attribute>() : new Attribute[elementCount];
}
if (useObjectArray)
{
return elementCount == 0 ? Array.Empty<object>() : new object[elementCount];
}
return elementCount == 0 ? caType.GetEmptyArray() : (object[])Array.CreateInstance(caType, elementCount);
}
#endregion

#region Private Static FCalls
Expand Down Expand Up @@ -1476,17 +1505,6 @@ private static void GetPropertyOrFieldData(
module, &pBlobStart, (byte*)blobEnd, out name, out isProperty, out type, out value);
blobStart = (IntPtr)pBlobStart;
}

private static object[] CreateAttributeArrayHelper(RuntimeType elementType, int elementCount)
{
// If we have 0 elements, don't allocate a new array
if (elementCount == 0)
{
return elementType.GetEmptyArray();
}

return (object[])Array.CreateInstance(elementType, elementCount);
}
#endregion
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,12 @@ public override object[] GetCustomAttributes(bool inherit)

public override object[] GetCustomAttributes(Type attributeType!!, bool inherit)
{
if (MdToken.IsNullToken(m_tkParamDef))
return Array.Empty<object>();

if (attributeType.UnderlyingSystemType is not RuntimeType attributeRuntimeType)
throw new ArgumentException(SR.Arg_MustBeType, nameof(attributeType));

if (MdToken.IsNullToken(m_tkParamDef))
return CustomAttribute.CreateAttributeArrayHelper(attributeRuntimeType, 0);

return CustomAttribute.GetCustomAttributes(this, attributeRuntimeType);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,9 @@ private static Attribute[] Instantiate(IEnumerable<CustomAttributeData> cads, Ty
attributes.Add(instantiatedAttribute);
}
int count = attributes.Count;
Attribute[] result;
try
{
result = (Attribute[])Array.CreateInstance(actualElementType, count);
}
catch (NotSupportedException) when (actualElementType.ContainsGenericParameters)
{
// This is here for desktop compatibility (using try-catch as control flow to avoid slowing down the mainline case.)
// GetCustomAttributes() normally returns an array of the exact attribute type requested except when
// the requested type is an open type. Its ICustomAttributeProvider counterpart would return an Object[] array but that's
// not possible with this api's return type so it returns null instead.
return null;
}
Attribute[] result = actualElementType.ContainsGenericParameters
? new Attribute[count]
: (Attribute[])Array.CreateInstance(actualElementType, count);
attributes.CopyTo(result, 0);
return result;
}
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Reflection/tests/ParameterInfoTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,14 @@ public void CustomAttributesInheritanceTest(int paramIndex, bool exists, int exp
Assert.Equal(expectedMyAttributeValue, exists ? myAttribute.Value : expectedMyAttributeValue);
}

[Fact]
public static void GetCustomAttributesOnParameterWithNullMetadataTokenReturnsArrayOfCorrectType()
{
var parameterWithNullMetadataToken = typeof(int[]).GetProperty(nameof(Array.Length)).GetMethod.ReturnParameter;
Assert.Equal(typeof(Attribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken).GetType());
Assert.Equal(typeof(MyAttribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken, typeof(MyAttribute)).GetType());
}

[Fact]
public void VerifyGetCustomAttributesData()
{
Expand Down
Loading

0 comments on commit 917a0b1

Please sign in to comment.