Skip to content

Refactor runtime-specific logic out of CustomAttributeData #55870

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.

Sign up for GitHub

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

Merged
jkotas merged 12 commits into from
Jul 17, 2021
Merged

Refactor runtime-specific logic out of CustomAttributeData #55870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c1bb326
Refactor runtime-specific logic out of CustomAttributeData
jkotas Jul 17, 2021
6d6274e
Fix linker build break
jkotas Jul 17, 2021
00aa646
Simplify CanonicalizeValue
jkotas Jul 17, 2021
10890e0
Use pattern matching
jkotas Jul 17, 2021
24ba08c
Update src/libraries/System.Private.CoreLib/src/System/Reflection/Cus…
jkotas Jul 17, 2021
9a3841d
Feedback
jkotas Jul 17, 2021
7825aab
Merge remote-tracking branch 'origin/customattributedata' into custom…
jkotas Jul 17, 2021
26f880a
Fix bad merge
jkotas Jul 17, 2021
2275e34
Revert incorrect change
jkotas Jul 17, 2021
0bcb0f1
Is this hitting codegen bug?
jkotas Jul 17, 2021
dcd0e38
Fix Mono ICALL order
jkotas Jul 17, 2021
e9b68dc
Revert incorrect change
jkotas Jul 17, 2021
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
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@
<Compile Include="$(BclSourcesRoot)\System\Reflection\AssemblyName.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Associates.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\ConstructorInfo.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\CustomAttribute.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\AssemblyBuilder.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\AssemblyBuilderData.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\ConstructorBuilder.cs" />
Expand Down Expand Up @@ -194,6 +193,7 @@
<Compile Include="$(BclSourcesRoot)\System\Reflection\RtFieldInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeAssembly.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeConstructorInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeCustomAttributeData.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeEventInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeExceptionHandlingClause.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeFieldInfo.cs" />
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}

internal static RuntimeAssembly InternalLoad(string assemblyName, ref StackCrawlMark stackMark, AssemblyLoadContext? assemblyLoadContext = null)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
63 changes: 24 additions & 39 deletions .../src/System/Reflection/CustomAttribute.cs → .../Reflection/RuntimeCustomAttributeData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace System.Reflection
{
public partial class CustomAttributeData
internal sealed class RuntimeCustomAttributeData : CustomAttributeData
{
#region Internal Static Members
internal static IList<CustomAttributeData> GetCustomAttributesInternal(RuntimeType target)
Expand Down Expand Up @@ -103,7 +103,7 @@ private static IList<CustomAttributeData> GetCombinedList(IList<CustomAttributeD
customAttributes.CopyTo(pca, pseudoAttributes.Count);
for (int i = 0; i < pseudoAttributes.Count; i++)
{
pca[i] = new CustomAttributeData(pseudoAttributes[i]);
pca[i] = new RuntimeCustomAttributeData(pseudoAttributes[i]);
}

return Array.AsReadOnly(pca);
Expand Down Expand Up @@ -181,15 +181,15 @@ private static CustomAttributeEncoding TypeToCustomAttributeEncoding(RuntimeType
}
private static CustomAttributeType InitCustomAttributeType(RuntimeType parameterType)
{
CustomAttributeEncoding encodedType = CustomAttributeData.TypeToCustomAttributeEncoding(parameterType);
CustomAttributeEncoding encodedType = TypeToCustomAttributeEncoding(parameterType);
CustomAttributeEncoding encodedArrayType = CustomAttributeEncoding.Undefined;
CustomAttributeEncoding encodedEnumType = CustomAttributeEncoding.Undefined;
string? enumName = null;

if (encodedType == CustomAttributeEncoding.Array)
{
parameterType = (RuntimeType)parameterType.GetElementType();
encodedArrayType = CustomAttributeData.TypeToCustomAttributeEncoding(parameterType);
encodedArrayType = TypeToCustomAttributeEncoding(parameterType);
}

if (encodedType == CustomAttributeEncoding.Enum || encodedArrayType == CustomAttributeEncoding.Enum)
Expand All @@ -210,7 +210,7 @@ private static IList<CustomAttributeData> GetCustomAttributes(RuntimeModule modu

CustomAttributeData[] customAttributes = new CustomAttributeData[records.Length];
for (int i = 0; i < records.Length; i++)
customAttributes[i] = new CustomAttributeData(module, records[i].tkCtor, in records[i].blob);
customAttributes[i] = new RuntimeCustomAttributeData(module, records[i].tkCtor, in records[i].blob);

return Array.AsReadOnly(customAttributes);
}
Expand Down Expand Up @@ -262,17 +262,13 @@ internal static CustomAttributeTypedArgument Filter(IList<CustomAttributeData> a
private IList<CustomAttributeNamedArgument> m_namedArgs = null!;

#region Constructor
protected CustomAttributeData()
{
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Property setters and fields which are accessed by any attribute instantiation which is present in the code linker has analyzed." +
"As such enumerating all fields and properties may return different results after trimming" +
"but all those which are needed to actually have data will be there.")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
Justification = "We're getting a MethodBase of a constructor that we found in the metadata. The attribute constructor won't be trimmed.")]
private CustomAttributeData(RuntimeModule scope, MetadataToken caCtorToken, in ConstArray blob)
private RuntimeCustomAttributeData(RuntimeModule scope, MetadataToken caCtorToken, in ConstArray blob)
{
m_scope = scope;
m_ctor = (RuntimeConstructorInfo)RuntimeType.GetMethodBase(scope, caCtorToken)!;
Expand Down Expand Up @@ -301,7 +297,7 @@ private CustomAttributeData(RuntimeModule scope, MetadataToken caCtorToken, in C
#endregion

#region Pseudo Custom Attribute Constructor
internal CustomAttributeData(Attribute attribute)
internal RuntimeCustomAttributeData(Attribute attribute)
{
if (attribute is DllImportAttribute dllImportAttribute)
Init(dllImportAttribute);
Expand Down Expand Up @@ -415,9 +411,9 @@ private void Init(object pca)
#endregion

#region Public Members
public virtual ConstructorInfo Constructor => m_ctor;
public override ConstructorInfo Constructor => m_ctor;

public virtual IList<CustomAttributeTypedArgument> ConstructorArguments
public override IList<CustomAttributeTypedArgument> ConstructorArguments
{
get
{
Expand All @@ -439,7 +435,7 @@ public virtual IList<CustomAttributeTypedArgument> ConstructorArguments
}
}

public virtual IList<CustomAttributeNamedArgument> NamedArguments
public override IList<CustomAttributeNamedArgument> NamedArguments
{
get
{
Expand Down Expand Up @@ -557,17 +553,6 @@ private static RuntimeType ResolveType(RuntimeModule scope, string typeName)
}
#endregion

private static object CanonicalizeValue(object value)
{
Debug.Assert(value is not null);

if (value.GetType().IsEnum)
{
return ((Enum)value).GetValue();
}
return value;
}

internal CustomAttributeTypedArgument(RuntimeModule scope, CustomAttributeEncodedArgument encodedArg)
{
CustomAttributeEncoding encodedType = encodedArg.CustomAttributeType.EncodedType;
Expand All @@ -577,22 +562,22 @@ internal CustomAttributeTypedArgument(RuntimeModule scope, CustomAttributeEncode

if (encodedType == CustomAttributeEncoding.Enum)
{
m_argumentType = ResolveType(scope, encodedArg.CustomAttributeType.EnumName!);
m_value = EncodedValueToRawValue(encodedArg.PrimitiveValue, encodedArg.CustomAttributeType.EncodedEnumType);
_argumentType = ResolveType(scope, encodedArg.CustomAttributeType.EnumName!);
_value = EncodedValueToRawValue(encodedArg.PrimitiveValue, encodedArg.CustomAttributeType.EncodedEnumType);
}
else if (encodedType == CustomAttributeEncoding.String)
{
m_argumentType = typeof(string);
m_value = encodedArg.StringValue;
_argumentType = typeof(string);
_value = encodedArg.StringValue;
}
else if (encodedType == CustomAttributeEncoding.Type)
{
m_argumentType = typeof(Type);
_argumentType = typeof(Type);

m_value = null;
_value = null;

if (encodedArg.StringValue is not null)
m_value = ResolveType(scope, encodedArg.StringValue);
_value = ResolveType(scope, encodedArg.StringValue);
}
else if (encodedType == CustomAttributeEncoding.Array)
{
Expand All @@ -608,25 +593,25 @@ internal CustomAttributeTypedArgument(RuntimeModule scope, CustomAttributeEncode
elementType = CustomAttributeEncodingToType(encodedType);
}

m_argumentType = elementType.MakeArrayType();
_argumentType = elementType.MakeArrayType();

if (encodedArg.ArrayValue is null)
{
m_value = null;
_value = null;
}
else
{
CustomAttributeTypedArgument[] arrayValue = new CustomAttributeTypedArgument[encodedArg.ArrayValue.Length];
for (int i = 0; i < arrayValue.Length; i++)
arrayValue[i] = new CustomAttributeTypedArgument(scope, encodedArg.ArrayValue[i]);

m_value = Array.AsReadOnly(arrayValue);
_value = Array.AsReadOnly(arrayValue);
}
}
else
{
m_argumentType = CustomAttributeEncodingToType(encodedType);
m_value = EncodedValueToRawValue(encodedArg.PrimitiveValue, encodedType);
_argumentType = CustomAttributeEncodingToType(encodedType);
_value = EncodedValueToRawValue(encodedArg.PrimitiveValue, encodedType);
}
}
}
Expand Down Expand Up @@ -1168,7 +1153,7 @@ private static void AddCustomAttributes(
// The derivedAttributes list must be passed by value so that it is not modified with the discovered attributes
RuntimeType.ListBuilder<object> derivedAttributes)
{
CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedMetadataToken);
CustomAttributeRecord[] car = RuntimeCustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedMetadataToken);

if (attributeFilterType is null && car.Length == 0)
{
Expand Down Expand Up @@ -1435,7 +1420,7 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA
{
RuntimeModule decoratedModule = decoratedAttribute.GetRuntimeModule();
MetadataImport scope = decoratedModule.MetadataImport;
CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedAttribute.MetadataToken);
CustomAttributeRecord[] car = RuntimeCustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedAttribute.MetadataToken);

AttributeUsageAttribute? attributeUsageAttribute = null;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeEventInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ public override bool HasDefaultValue
if (raw)
{
CustomAttributeTypedArgument value =
CustomAttributeData.Filter(
CustomAttributeData.GetCustomAttributes(this), typeof(DateTimeConstantAttribute), 0);
RuntimeCustomAttributeData.Filter(
RuntimeCustomAttributeData.GetCustomAttributes(this), typeof(DateTimeConstantAttribute), 0);

if (value.ArgumentType != null)
return new DateTime((long)value.Value!);
Expand Down Expand Up @@ -541,7 +541,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public override bool IsDefined(Type attributeType, bool inherit)

public override IList<CustomAttributeData> GetCustomAttributesData()
{
return CustomAttributeData.GetCustomAttributesInternal(this);
return RuntimeCustomAttributeData.GetCustomAttributesInternal(this);
}
#endregion

Expand Down
26 changes: 19 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/CustomAttributeData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Text;

namespace System.Reflection
{
public partial class CustomAttributeData
public class CustomAttributeData
{
#region Public Static Members
public static IList<CustomAttributeData> GetCustomAttributes(MemberInfo target)
Expand Down Expand Up @@ -42,6 +43,10 @@ public static IList<CustomAttributeData> GetCustomAttributes(ParameterInfo targe
}
#endregion

protected CustomAttributeData()
{
}

#region Object Override
public override string ToString()
{
Expand All @@ -53,19 +58,21 @@ public override string ToString()

bool first = true;

int count = ConstructorArguments.Count;
for (int i = 0; i < count; i++)
IList<CustomAttributeTypedArgument> constructorArguments = ConstructorArguments;
Copy link
Member Author

@jkotas jkotas Jul 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache in locals to avoid redundant interface calls.

int constructorArgumentsCount = constructorArguments.Count;
for (int i = 0; i < constructorArgumentsCount; i++)
{
if (!first) vsb.Append(", ");
vsb.Append(ConstructorArguments[i].ToString());
vsb.Append(constructorArguments[i].ToString());
first = false;
}

count = NamedArguments.Count;
for (int i = 0; i < count; i++)
IList<CustomAttributeNamedArgument> namedArguments = NamedArguments;
int namedArgumentsCount = namedArguments.Count;
for (int i = 0; i < namedArgumentsCount; i++)
{
if (!first) vsb.Append(", ");
vsb.Append(NamedArguments[i].ToString());
vsb.Append(namedArguments[i].ToString());
first = false;
}

Expand All @@ -79,6 +86,11 @@ public override string ToString()

#region Public Members
public virtual Type AttributeType => Constructor.DeclaringType!;

// Expected to be overriden
public virtual ConstructorInfo Constructor => null!;
Copy link
Member Author

@jkotas jkotas Jul 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the existing Mono behavior. The existing CoreCLR behavior is to throw NullReferenceException in some of these.

public virtual IList<CustomAttributeTypedArgument> ConstructorArguments => null!;
public virtual IList<CustomAttributeNamedArgument> NamedArguments => null!;
#endregion
}
}
Loading