Add DynamicallyAccessedMembersAttribute

[MichalStrehovsky]

This is in support of dotnet/linker#988 - see the spec there for motivation.

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(
        AttributeTargets.Field |
        AttributeTargets.ReturnValue |
        AttributeTargets.GenericParameter |
        AttributeTargets.Parameter |
        AttributeTargets.Property)]
    public sealed class DynamicallyAccessedMembersAttribute : Attribute
    {
        public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberKinds memberKinds)
        {
            MemberKinds = memberKinds;
        }

        public DynamicallyAccessedMemberKinds MemberKinds { get; }
    }

    [Flags]
    public enum DynamicallyAccessedMemberKinds
    {
        DefaultConstructor  = 0b00000000_00000001,
        PublicConstructors  = 0b00000000_00000011,
        Constructors        = 0b00000000_00000111,
        PublicMethods       = 0b00000000_00001000,
        Methods             = 0b00000000_00011000,
        PublicFields        = 0b00000000_00100000,
        Fields              = 0b00000000_01100000,
        PublicNestedTypes   = 0b00000000_10000000,
        NestedTypes         = 0b00000001_10000000,
        PublicProperties    = 0b00000010_00000000,
        Properties          = 0b00000110_00000000,
        PublicEvents        = 0b00001000_00000000,
        Events              = 0b00011000_00000000,
    }
}

[vitek-karas]

Looks good - I would add an overload with a message. Being able to customize the UX when this is reported as potential issue by the linker will be very valuable I think.

[vitek-karas]

/cc @eerhardt

[vitek-karas]

/cc @marek-safar

[terrajobst]

Video

• What about MetadataLoadContext?
  • Will need to make use of supressions, there is a separate attribute for that
• It feels the defaults are flipped
  • We should have PublicMethods and NonPublicMethods to make it clear what's being returned
  • The semantics should match BindingFlags
• Are attributes kept?
  • Depends, but there is a discussion on trimming those too
• Should the enum have an "All" member?
• We should have a None member with value 0
• We should make sure the attribute can only be applied once is not inherited
• The linker tooling should warn if the attribute is applied to things that aren't System.Type or System.String, the linker should warn

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(
        AttributeTargets.Field |
        AttributeTargets.ReturnValue |
        AttributeTargets.GenericParameter |
        AttributeTargets.Parameter |
        AttributeTargets.Property)]
    public sealed class DynamicallyAccessedMembersAttribute : Attribute
    {
        public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberKinds memberKinds)
        {
            MemberKinds = memberKinds;
        }

        public DynamicallyAccessedMemberKinds MemberKinds { get; }
    }

    [Flags]
    public enum DynamicallyAccessedMemberKinds
    {
        None                         = 0b00000000_00000000,
        DefaultConstructor           = 0b00000000_00000001,
        PublicConstructors           = 0b00000000_00000011,
        NonPublicConstructors        = 0b00000000_00000100,
        PublicMethods                = 0b00000000_00001000,
        NonPublicMethods             = 0b00000000_00010000,
        PublicFields                 = 0b00000000_00100000,
        NonPublicFields              = 0b00000000_01000000,
        PublicNestedTypes            = 0b00000000_10000000,
        NonPublicNestedTypes         = 0b00000001_00000000,
        PublicProperties             = 0b00000010_00000000,
        NonPublicProperties          = 0b00000100_00000000,
        PublicEvents                 = 0b00001000_00000000,
        NonPublicEvents              = 0b00010000_00000000,
    }
}

[eerhardt]

@terrajobst - I made some edits to your pasted API from the review.

1. Updated the namespace
2. Updated the names of the enum to be Public and NonPublic.
3. Update the values of the enum to match the names.

[eerhardt]

One tiny naming question.

We already have System.Reflection.MemberTypes. Would it make sense to name this enum DynamicallyAccessedMemberTypes instead of DynamicallyAccessedMemberKinds to align with the existing enum?

@terrajobst @stephentoub @bartonjs @GrabYourPitchforks @MichalStrehovsky @vitek-karas ?

[stephentoub]

Would it make sense to name this enum DynamicallyAccessedMemberTypes instead of DynamicallyAccessedMemberKinds to align with the existing enum?

Sounds fine to me.