Skip to content

Commit

Permalink
Revert "Avoid Attribute.GetCustomAttributes() returning null for open…
Browse files Browse the repository at this point in the history 
… generic type (dotnet#65237)" (dotnet#66508)

This reverts commit 2029495.
  • Loading branch information
@jkotas
jkotas committed Mar 11, 2022
1 parent b60b5e5 commit f99ba2e
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 170 deletions.
16 changes: 10 additions & 6 deletions src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,10 @@ private static AttributeUsageAttribute InternalGetAttributeUsage(Type type)
SR.Format(SR.Format_AttributeUsage, type));
}

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

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

Expand All @@ -472,7 +474,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),
_ => (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit)
_ => (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!,
};
}

Expand Down Expand Up @@ -534,23 +536,25 @@ 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 (Attribute[])element.GetCustomAttributes(attributeType, inherit);
return (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!;
}

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 (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit);
return (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!;
}

public static bool IsDefined(ParameterInfo element, Type attributeType)
Expand Down
64 changes: 23 additions & 41 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs
View file
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 CreateAttributeArrayHelper(caType, 0);
return (caType.IsValueType) ? Array.Empty<object>() : CreateAttributeArrayHelper(caType, 0);

if (type.IsGenericType && !type.IsGenericTypeDefinition)
type = (type.GetGenericTypeDefinition() as RuntimeType)!;
Expand All @@ -919,6 +919,8 @@ 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 @@ -930,7 +932,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp
type = (type.BaseType as RuntimeType)!;
}

object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
for (int i = 0; i < result.Count; i++)
{
typedResult[i] = result[i];
Expand Down Expand Up @@ -961,6 +963,8 @@ 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 @@ -972,7 +976,7 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy
method = method.GetParentDefinition()!;
}

object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
for (int i = 0; i < result.Count; i++)
{
typedResult[i] = result[i];
Expand Down Expand Up @@ -1119,13 +1123,16 @@ 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);

object[] result = CreateAttributeArrayHelper(attributeFilterType, attributes.Count + pcaCount);
bool useObjectArray = attributeFilterType is null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters;
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : attributeFilterType!;

object[] result = CreateAttributeArrayHelper(arrayType, attributes.Count + pcaCount);
for (int i = 0; i < attributes.Count; i++)
{
result[i] = attributes[i];
Expand Down Expand Up @@ -1432,42 +1439,6 @@ 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 @@ -1505,6 +1476,17 @@ 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
6 changes: 3 additions & 3 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs
View file
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
16 changes: 13 additions & 3 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,19 @@ private static Attribute[] Instantiate(IEnumerable<CustomAttributeData> cads, Ty
attributes.Add(instantiatedAttribute);
}
int count = attributes.Count;
Attribute[] result = actualElementType.ContainsGenericParameters
? new Attribute[count]
: (Attribute[])Array.CreateInstance(actualElementType, 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;
}
attributes.CopyTo(result, 0);
return result;
}
Expand Down
8 changes: 0 additions & 8 deletions src/libraries/System.Reflection/tests/ParameterInfoTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,6 @@ 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
100 changes: 4 additions & 96 deletions src/libraries/System.Runtime/tests/System/Attributes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
using System.Runtime.InteropServices;
using Xunit;

[module: Debuggable(true, false)]
[module:Debuggable(true,false)]
namespace System.Tests
{
public class AttributeIsDefinedTests
Expand Down Expand Up @@ -218,71 +218,6 @@ public static void GetCustomAttributes_Interface()
{
Assert.True(typeof(ExampleWithAttribute).GetCustomAttributes(typeof(INameable), inherit: false)[0] is NameableAttribute);
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType()
{
GenericAttributesTestHelper<string>(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute), t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForField()
{
FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance);
GenericAttributesTestHelper<TimeSpan>(t => Attribute.GetCustomAttributes(field, t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForConstructor()
{
ConstructorInfo method = typeof(HasGenericAttribute).GetConstructor(Type.EmptyTypes);
GenericAttributesTestHelper<Guid>(t => Attribute.GetCustomAttributes(method, t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForMethod()
{
MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method));
GenericAttributesTestHelper<long>(t => Attribute.GetCustomAttributes(method, t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForParameter()
{
ParameterInfo parameter = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)).GetParameters()[0];
GenericAttributesTestHelper<ulong>(t => Attribute.GetCustomAttributes(parameter, t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForProperty()
{
PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property));
GenericAttributesTestHelper<List<object>>(t => Attribute.GetCustomAttributes(property, t));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForEvent()
{
EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event));
GenericAttributesTestHelper<DateTime?>(t => Attribute.GetCustomAttributes(@event, t));
}

private static void GenericAttributesTestHelper<TGenericParameter>(Func<Type, Attribute[]> getCustomAttributes)
{
Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>));
Assert.Empty(openGenericAttributes);

Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute<TGenericParameter>));
Assert.Equal(1, closedGenericAttributes.Length);
Assert.Equal(typeof(GenericAttribute<TGenericParameter>[]), closedGenericAttributes.GetType());
}
}

public static class GetCustomAttribute
Expand All @@ -291,7 +226,7 @@ public static class GetCustomAttribute
[Fact]
public static void customAttributeCount()
{
List<CustomAttributeData> customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList();
List<CustomAttributeData> customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList();
// [System.Security.UnverifiableCodeAttribute()]
// [TestAttributes.FooAttribute()]
// [TestAttributes.ComplicatedAttribute((Int32)1, Stuff = 2)]
Expand Down Expand Up @@ -725,7 +660,7 @@ public override object TypeId
}
public class BaseClass
{
public virtual void TestMethod([ArgumentUsage("for test")] string[] strArray, params string[] strList)
public virtual void TestMethod([ArgumentUsage("for test")]string[] strArray, params string[] strList)
{
}
}
Expand Down Expand Up @@ -881,39 +816,12 @@ public interface INameable
string Name { get; }
}

[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
[AttributeUsage (AttributeTargets.All, AllowMultiple = true)]
public class NameableAttribute : Attribute, INameable
{
string INameable.Name => "Nameable";
}

[Nameable]
public class ExampleWithAttribute { }

public class GenericAttribute<T> : Attribute
{
}

[GenericAttribute<string>]
public class HasGenericAttribute
{
[GenericAttribute<TimeSpan>]
internal bool Field;

[GenericAttribute<Guid>]
public HasGenericAttribute() { }

[GenericAttribute<long>]
public void Method([GenericAttribute<ulong>] int parameter)
{
this.Field = true;
this.Event += () => { };
}

[GenericAttribute<List<object>>]
public int Property { get; set; }

[GenericAttribute<DateTime?>]
public event Action Event;
}
}
2 changes: 0 additions & 2 deletions src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
[assembly: MultiAttribute<bool>()]
[assembly: MultiAttribute<bool>(true)]

[module: SingleAttribute<long>()]

[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false)]
public class SingleAttribute<T> : Attribute
{
Expand Down
Loading

0 comments on commit f99ba2e

Please sign in to comment.