[mono] Allow overriding GetCustomAttributesData routines

[uweigand]

• Have CustomAttributeData.GetCustomAttributes call target.GetCustomAttributesData
  to allow derived classes to override the latter (like in CoreCLR)

System.Text.Json.SourceGeneration.Tests are currently failing when using a Mono-based dotnet runtime. This is because the source-generation logic overrides GetCustomAttributesData in various derived classes, and expects this to be called from the generic CustomAttributeData.GetCustomAttributes routine. This works in CoreCLR, because there CustomAttributeData.GetCustomAttributes calls target.GetCustomAttributesData which calls back to CustomAttributeData.GetCustomAttributesInternal, which does the actual work. However, the current Mono code has instead target.GetCustomAttributesData calling to CustomAttributeData.GetCustomAttributes, which does the actual work.

Fix this by introducing GetCustomAttributesInternal routines and re-arranging the call sequence to match CoreCLR.

[lambdageek]

@uweigand maybe we can pull the common bit of CustomAttributeData into a new file under src/libraries and share it between the two runtimes?

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs

Lines 15 to 49 in bd9d4e0

	public class CustomAttributeData
	{
	#region Public Static Members
	public static IList<CustomAttributeData> GetCustomAttributes(MemberInfo target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(Module target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(Assembly target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(ParameterInfo target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	#endregion

[uweigand]

@uweigand maybe we can pull the common bit of CustomAttributeData into a new file under src/libraries and share it between the two runtimes?

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs

Lines 15 to 49 in bd9d4e0

	public class CustomAttributeData
	{
	#region Public Static Members
	public static IList<CustomAttributeData> GetCustomAttributes(MemberInfo target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(Module target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(Assembly target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	public static IList<CustomAttributeData> GetCustomAttributes(ParameterInfo target)
	{
	if (target is null)
	throw new ArgumentNullException(nameof(target));
	return target.GetCustomAttributesData();
	}
	#endregion

I guess so ... but is it worthwhile doing this now, just for these trivial bits? I think in the end most of the custom attribute logic should be shareable between Mono and CoreCLR, but that will need more cleanup.

[lambdageek]

I guess so ... but is it worthwhile doing this now, just for these trivial bits?

Yea that's exactly my question. 😄

I guess one reason to do it on a small relatively trivial PR is that it would be pretty easy to make sense of the diff, compared to something deeper.

But it's up to you. I certainly don't have a problem with the PR as is.

I think in the end most of the custom attribute logic should be shareable between Mono and CoreCLR, but that will need more cleanup.

Yea I think compared to elsewhere in Reflection the differences here are more incidental, so we should be able to converge.

[uweigand]

I'd prefer to go with the PR as-is for now. I can look into what can be merged here a bit later (probably best after .NET 6 has branched).

[uweigand]

New version that moves common bits to the shared location. This includes moving ToString, GetHashCode, Equals, and AttributeType, which either were already identical or could trivially be made identical. Note that this obsoletes #55728 as well as the new code already allows overriding the accessor routines.

[marek-safar]

New version that moves common bits to the shared location.

Thanks! You'll need to update coreclr implementation too to avoid code duplication.

[ghost]

Hello @marek-safar!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.