Skip to content

Commit 8dea545

Browse files
jkotasjkotas
authored
Revert "Avoid Attribute.GetCustomAttributes() returning null for open generic type (#65237)"
This reverts commit 2029495.
1 parent d008c6f commit 8dea545

File tree

8 files changed

+56
-170
lines changed

8 files changed

+56
-170
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,10 @@ private static AttributeUsageAttribute InternalGetAttributeUsage(Type type)
434434
SR.Format(SR.Format_AttributeUsage, type));
435435
}
436436

437-
private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount) =>
438-
elementType.ContainsGenericParameters ? new Attribute[elementCount] : (Attribute[])Array.CreateInstance(elementType, elementCount);
437+
private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount)
438+
{
439+
return (Attribute[])Array.CreateInstance(elementType, elementCount);
440+
}
439441
#endregion
440442

441443
#endregion
@@ -457,7 +459,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, Type attribu
457459
{
458460
MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, attributeType, inherit),
459461
MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, attributeType, inherit),
460-
_ => (Attribute[])element.GetCustomAttributes(attributeType, inherit)
462+
_ => (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!,
461463
};
462464
}
463465

@@ -472,7 +474,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, bool inherit
472474
{
473475
MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, typeof(Attribute), inherit),
474476
MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, typeof(Attribute), inherit),
475-
_ => (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit)
477+
_ => (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!,
476478
};
477479
}
478480

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

539+
537540
MemberInfo member = element.Member;
538541
if (member.MemberType == MemberTypes.Method && inherit)
539542
return InternalParamGetCustomAttributes(element, attributeType, inherit);
540543

541-
return (Attribute[])element.GetCustomAttributes(attributeType, inherit);
544+
return (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!;
542545
}
543546

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

552+
549553
MemberInfo member = element.Member;
550554
if (member.MemberType == MemberTypes.Method && inherit)
551555
return InternalParamGetCustomAttributes(element, null, inherit);
552556

553-
return (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit);
557+
return (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!;
554558
}
555559

556560
public static bool IsDefined(ParameterInfo element, Type attributeType)

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp
899899
Debug.Assert(caType is not null);
900900

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

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

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

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

933-
object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
935+
object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
934936
for (int i = 0; i < result.Count; i++)
935937
{
936938
typedResult[i] = result[i];
@@ -961,6 +963,8 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy
961963

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

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

975-
object[] typedResult = CreateAttributeArrayHelper(caType, result.Count);
979+
object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
976980
for (int i = 0; i < result.Count; i++)
977981
{
978982
typedResult[i] = result[i];
@@ -1119,13 +1123,16 @@ private static bool IsCustomAttributeDefined(
11191123
}
11201124

11211125
private static object[] GetCustomAttributes(
1122-
RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType attributeFilterType)
1126+
RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType? attributeFilterType)
11231127
{
11241128
RuntimeType.ListBuilder<object> attributes = default;
11251129

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

1128-
object[] result = CreateAttributeArrayHelper(attributeFilterType, attributes.Count + pcaCount);
1132+
bool useObjectArray = attributeFilterType is null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters;
1133+
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : attributeFilterType!;
1134+
1135+
object[] result = CreateAttributeArrayHelper(arrayType, attributes.Count + pcaCount);
11291136
for (int i = 0; i < attributes.Count; i++)
11301137
{
11311138
result[i] = attributes[i];
@@ -1432,42 +1439,6 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA
14321439

14331440
return attributeUsageAttribute ?? AttributeUsageAttribute.Default;
14341441
}
1435-
1436-
internal static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount)
1437-
{
1438-
bool useAttributeArray = false;
1439-
bool useObjectArray = false;
1440-
1441-
if (caType == typeof(Attribute))
1442-
{
1443-
useAttributeArray = true;
1444-
}
1445-
else if (caType.IsValueType)
1446-
{
1447-
useObjectArray = true;
1448-
}
1449-
else if (caType.ContainsGenericParameters)
1450-
{
1451-
if (caType.IsSubclassOf(typeof(Attribute)))
1452-
{
1453-
useAttributeArray = true;
1454-
}
1455-
else
1456-
{
1457-
useObjectArray = true;
1458-
}
1459-
}
1460-
1461-
if (useAttributeArray)
1462-
{
1463-
return elementCount == 0 ? Array.Empty<Attribute>() : new Attribute[elementCount];
1464-
}
1465-
if (useObjectArray)
1466-
{
1467-
return elementCount == 0 ? Array.Empty<object>() : new object[elementCount];
1468-
}
1469-
return elementCount == 0 ? caType.GetEmptyArray() : (object[])Array.CreateInstance(caType, elementCount);
1470-
}
14711442
#endregion
14721443

14731444
#region Private Static FCalls
@@ -1505,6 +1476,17 @@ private static void GetPropertyOrFieldData(
15051476
module, &pBlobStart, (byte*)blobEnd, out name, out isProperty, out type, out value);
15061477
blobStart = (IntPtr)pBlobStart;
15071478
}
1479+
1480+
private static object[] CreateAttributeArrayHelper(RuntimeType elementType, int elementCount)
1481+
{
1482+
// If we have 0 elements, don't allocate a new array
1483+
if (elementCount == 0)
1484+
{
1485+
return elementType.GetEmptyArray();
1486+
}
1487+
1488+
return (object[])Array.CreateInstance(elementType, elementCount);
1489+
}
15081490
#endregion
15091491
}
15101492

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,12 +509,12 @@ public override object[] GetCustomAttributes(bool inherit)
509509

510510
public override object[] GetCustomAttributes(Type attributeType!!, bool inherit)
511511
{
512+
if (MdToken.IsNullToken(m_tkParamDef))
513+
return Array.Empty<object>();
514+
512515
if (attributeType.UnderlyingSystemType is not RuntimeType attributeRuntimeType)
513516
throw new ArgumentException(SR.Arg_MustBeType, nameof(attributeType));
514517

515-
if (MdToken.IsNullToken(m_tkParamDef))
516-
return CustomAttribute.CreateAttributeArrayHelper(attributeRuntimeType, 0);
517-
518518
return CustomAttribute.GetCustomAttributes(this, attributeRuntimeType);
519519
}
520520

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,19 @@ private static Attribute[] Instantiate(IEnumerable<CustomAttributeData> cads, Ty
141141
attributes.Add(instantiatedAttribute);
142142
}
143143
int count = attributes.Count;
144-
Attribute[] result = actualElementType.ContainsGenericParameters
145-
? new Attribute[count]
146-
: (Attribute[])Array.CreateInstance(actualElementType, count);
144+
Attribute[] result;
145+
try
146+
{
147+
result = (Attribute[])Array.CreateInstance(actualElementType, count);
148+
}
149+
catch (NotSupportedException) when (actualElementType.ContainsGenericParameters)
150+
{
151+
// This is here for desktop compatibility (using try-catch as control flow to avoid slowing down the mainline case.)
152+
// GetCustomAttributes() normally returns an array of the exact attribute type requested except when
153+
// the requested type is an open type. Its ICustomAttributeProvider counterpart would return an Object[] array but that's
154+
// not possible with this api's return type so it returns null instead.
155+
return null;
156+
}
147157
attributes.CopyTo(result, 0);
148158
return result;
149159
}

‎src/libraries/System.Reflection/tests/ParameterInfoTests.cs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,6 @@ public void CustomAttributesInheritanceTest(int paramIndex, bool exists, int exp
245245
Assert.Equal(expectedMyAttributeValue, exists ? myAttribute.Value : expectedMyAttributeValue);
246246
}
247247

248-
[Fact]
249-
public static void GetCustomAttributesOnParameterWithNullMetadataTokenReturnsArrayOfCorrectType()
250-
{
251-
var parameterWithNullMetadataToken = typeof(int[]).GetProperty(nameof(Array.Length)).GetMethod.ReturnParameter;
252-
Assert.Equal(typeof(Attribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken).GetType());
253-
Assert.Equal(typeof(MyAttribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken, typeof(MyAttribute)).GetType());
254-
}
255-
256248
[Fact]
257249
public void VerifyGetCustomAttributesData()
258250
{

‎src/libraries/System.Runtime/tests/System/Attributes.cs

Lines changed: 4 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
using System.Runtime.InteropServices;
2121
using Xunit;
2222

23-
[module: Debuggable(true, false)]
23+
[module:Debuggable(true,false)]
2424
namespace System.Tests
2525
{
2626
public class AttributeIsDefinedTests
@@ -218,71 +218,6 @@ public static void GetCustomAttributes_Interface()
218218
{
219219
Assert.True(typeof(ExampleWithAttribute).GetCustomAttributes(typeof(INameable), inherit: false)[0] is NameableAttribute);
220220
}
221-
222-
[Fact]
223-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
224-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType()
225-
{
226-
GenericAttributesTestHelper<string>(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute), t));
227-
}
228-
229-
[Fact]
230-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
231-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForField()
232-
{
233-
FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance);
234-
GenericAttributesTestHelper<TimeSpan>(t => Attribute.GetCustomAttributes(field, t));
235-
}
236-
237-
[Fact]
238-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
239-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForConstructor()
240-
{
241-
ConstructorInfo method = typeof(HasGenericAttribute).GetConstructor(Type.EmptyTypes);
242-
GenericAttributesTestHelper<Guid>(t => Attribute.GetCustomAttributes(method, t));
243-
}
244-
245-
[Fact]
246-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
247-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForMethod()
248-
{
249-
MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method));
250-
GenericAttributesTestHelper<long>(t => Attribute.GetCustomAttributes(method, t));
251-
}
252-
253-
[Fact]
254-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
255-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForParameter()
256-
{
257-
ParameterInfo parameter = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)).GetParameters()[0];
258-
GenericAttributesTestHelper<ulong>(t => Attribute.GetCustomAttributes(parameter, t));
259-
}
260-
261-
[Fact]
262-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
263-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForProperty()
264-
{
265-
PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property));
266-
GenericAttributesTestHelper<List<object>>(t => Attribute.GetCustomAttributes(property, t));
267-
}
268-
269-
[Fact]
270-
[ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)]
271-
public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForEvent()
272-
{
273-
EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event));
274-
GenericAttributesTestHelper<DateTime?>(t => Attribute.GetCustomAttributes(@event, t));
275-
}
276-
277-
private static void GenericAttributesTestHelper<TGenericParameter>(Func<Type, Attribute[]> getCustomAttributes)
278-
{
279-
Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>));
280-
Assert.Empty(openGenericAttributes);
281-
282-
Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute<TGenericParameter>));
283-
Assert.Equal(1, closedGenericAttributes.Length);
284-
Assert.Equal(typeof(GenericAttribute<TGenericParameter>[]), closedGenericAttributes.GetType());
285-
}
286221
}
287222

288223
public static class GetCustomAttribute
@@ -291,7 +226,7 @@ public static class GetCustomAttribute
291226
[Fact]
292227
public static void customAttributeCount()
293228
{
294-
List<CustomAttributeData> customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList();
229+
List<CustomAttributeData> customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList();
295230
// [System.Security.UnverifiableCodeAttribute()]
296231
// [TestAttributes.FooAttribute()]
297232
// [TestAttributes.ComplicatedAttribute((Int32)1, Stuff = 2)]
@@ -725,7 +660,7 @@ public override object TypeId
725660
}
726661
public class BaseClass
727662
{
728-
public virtual void TestMethod([ArgumentUsage("for test")] string[] strArray, params string[] strList)
663+
public virtual void TestMethod([ArgumentUsage("for test")]string[] strArray, params string[] strList)
729664
{
730665
}
731666
}
@@ -881,39 +816,12 @@ public interface INameable
881816
string Name { get; }
882817
}
883818

884-
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
819+
[AttributeUsage (AttributeTargets.All, AllowMultiple = true)]
885820
public class NameableAttribute : Attribute, INameable
886821
{
887822
string INameable.Name => "Nameable";
888823
}
889824

890825
[Nameable]
891826
public class ExampleWithAttribute { }
892-
893-
public class GenericAttribute<T> : Attribute
894-
{
895-
}
896-
897-
[GenericAttribute<string>]
898-
public class HasGenericAttribute
899-
{
900-
[GenericAttribute<TimeSpan>]
901-
internal bool Field;
902-
903-
[GenericAttribute<Guid>]
904-
public HasGenericAttribute() { }
905-
906-
[GenericAttribute<long>]
907-
public void Method([GenericAttribute<ulong>] int parameter)
908-
{
909-
this.Field = true;
910-
this.Event += () => { };
911-
}
912-
913-
[GenericAttribute<List<object>>]
914-
public int Property { get; set; }
915-
916-
[GenericAttribute<DateTime?>]
917-
public event Action Event;
918-
}
919827
}

‎src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
[assembly: MultiAttribute<bool>()]
1818
[assembly: MultiAttribute<bool>(true)]
1919

20-
[module: SingleAttribute<long>()]
21-
2220
[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false)]
2321
public class SingleAttribute<T> : Attribute
2422
{

0 commit comments

Comments
 (0)