Skip to content

Commit

Permalink
Fix invalid suppression in System.Attribute
Browse files Browse the repository at this point in the history
The suppression on System.Attribute is wrong. Ran into this in dotnet#70201 where `EditorBrowsableAttributeTests.EqualsTest` is failing because we didn't generate any reflection metadata for the field.

There's a difference between "a field is kept" and "a field is accessible from reflection without issues". The issue is more pronounced in Native AOT (where the field can be completely erased from reflection). The implementation of Attribute.Equals didn't see any fields on the attribute type because the static analysis didn't deem any of them necessary.

This fixes the suppression and replaces it with a more correct DAM on type.
  • Loading branch information
MichalStrehovsky committed Jun 14, 2022
1 parent 367193d commit 84a3ffa
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
5 changes: 1 addition & 4 deletions src/libraries/System.Private.CoreLib/src/System/Attribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ namespace System
[AttributeUsage(AttributeTargets.All, Inherited = true, AllowMultiple = false)]
[Serializable]
[System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields | DynamicallyAccessedMemberTypes.PublicFields)]
public abstract partial class Attribute
{
protected Attribute() { }

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Unused fields don't make a difference for equality")]
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj == null)
Expand Down Expand Up @@ -48,8 +47,6 @@ public override bool Equals([NotNullWhen(true)] object? obj)
return true;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Unused fields don't make a difference for hashcode quality")]
public override int GetHashCode()
{
Type type = GetType();
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Runtime/ref/System.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ public AssemblyLoadEventArgs(System.Reflection.Assembly loadedAssembly) { }
public delegate void AssemblyLoadEventHandler(object? sender, System.AssemblyLoadEventArgs args);
public delegate void AsyncCallback(System.IAsyncResult ar);
[System.AttributeUsageAttribute(System.AttributeTargets.All, Inherited=true, AllowMultiple=false)]
[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicFields | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicFields)]
public abstract partial class Attribute
{
protected Attribute() { }
Expand Down

0 comments on commit 84a3ffa

Please sign in to comment.