Add class as a target for RequiresUnreferencedCodeAttribute

[tlakollo]

Background and Motivation

As explained in dotnet/linker#1742 some types are considered not trim compatible leading to mark all its member methods with RequiresUnreferencedCode. This could lead to hundreds of members annotated with RequiresUnreferencedCode. The proposal is to only annotate the type with RequiresUnreferencedCode, then the linker will treat this as if we annotated constructors and static methods with RequiresUnreferencedCode.

Proposed API

 namespace System.Diagnostics.CodeAnalysis
 {
     [AttributeUsage(
+        AttributeTargets.Class |
         AttributeTargets.Constructor |
         AttributeTargets.Method,
         Inherited = false)]
     public sealed class RequiresUnreferencedCodeAttribute : Attribute
     {
     }
 }

Usage Examples

[RequiresUnreferencedCode ("Class MyType has been marked as dangerous for trimming", Url = "https://helpurl/MyType")]
public class MyType {
  public static void M1 () {}
  public static void M2 () {}
}

public class OtherType {
  public void Main () {
    M1();     -> This creates a warning IL2026
    M2();     -> This creates a warning IL2026
  }
}

Alternative Designs

Apply to other AttributeTargets like struct and interface. As per @MichalStrehovsky comments, Structs are problematic because they can be default-instantiated. We would like this to be considered unsafe (and warn) only if a constructor or a static method on the type is called to limit the amount of noise (and additional suppressions needed) when a type is annotated as such.
If we allow this on structs, we would have to warn on instance methods too, plus it would introduce problems when unsafe instance methods get called indirectly through interfaces (the tool would not be able to see that at all).

Since this enhancement is meant to help people to annotate fewer methods seems to be a good option to start just by enabling it in classes, but this issue is meant to discuss which targets we want to enable.

Risks

Related PR

Related to #50064

[ghost]

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @tannergooding, @sbomer
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

As explained in dotnet/linker#1742 some types are considered not trim compatible leading to mark all its member methods with RequiresUnreferencedCode. This could lead to hundreds of members annotated with RequiresUnreferencedCode. The proposal is to only annotate the type with RequiresUnreferencedCode, then the linker will treat this as if we annotated constructors and static methods with RequiresUnreferencedCode.

Proposed API

- [System.AttributeUsageAttribute(System.AttributeTargets.Constructor | System.AttributeTargets.Method, Inherited=false)]
+ [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Constructor | System.AttributeTargets.Method, Inherited=false)]
#if SYSTEM_PRIVATE_CORELIB
    public
#else
    internal
#endif
        sealed class RequiresUnreferencedCodeAttribute : Attribute

Usage Examples

[RequiresUnreferencedCode ("Class MyType has been marked as dangerous for trimming", Url = "https://helpurl/MyType")]
public class MyType {
  public static void M1 () {}
  public static void M2 () {}
}

public class OtherType {
  public void Main () {
    M1();     -> This creates a warning IL2026
    M2();     -> This creates a warning IL2026
  }
}

Alternative Designs

Apply to other AttributeTargets like struct and interface. As per @MichalStrehovsky comments, Structs are problematic because they can be default-instantiated. We would like this to be considered unsafe (and warn) only if a constructor or a static method on the type is called to limit the amount of noise (and additional suppressions needed) when a type is annotated as such.
If we allow this on structs, we would have to warn on instance methods too, plus it would introduce problems when unsafe instance methods get called indirectly through interfaces (the tool would not be able to see that at all).

Since this enhancement is meant to help people to annotate fewer methods seems to be a good option to start just by enabling it in classes, but this issue is meant to discuss which targets we want to enable.

Risks

Related to #50064

Author:	tlakollo
Assignees:	tlakollo
Labels:	api-suggestion, linkable-framework
Milestone:	-

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @vitek-karas, @agocke, @CoffeeFlux, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

As explained in dotnet/linker#1742 some types are considered not trim compatible leading to mark all its member methods with RequiresUnreferencedCode. This could lead to hundreds of members annotated with RequiresUnreferencedCode. The proposal is to only annotate the type with RequiresUnreferencedCode, then the linker will treat this as if we annotated constructors and static methods with RequiresUnreferencedCode.

Proposed API

- [System.AttributeUsageAttribute(System.AttributeTargets.Constructor | System.AttributeTargets.Method, Inherited=false)]
+ [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Constructor | System.AttributeTargets.Method, Inherited=false)]
#if SYSTEM_PRIVATE_CORELIB
    public
#else
    internal
#endif
        sealed class RequiresUnreferencedCodeAttribute : Attribute

Usage Examples

[RequiresUnreferencedCode ("Class MyType has been marked as dangerous for trimming", Url = "https://helpurl/MyType")]
public class MyType {
  public static void M1 () {}
  public static void M2 () {}
}

public class OtherType {
  public void Main () {
    M1();     -> This creates a warning IL2026
    M2();     -> This creates a warning IL2026
  }
}

Alternative Designs

Apply to other AttributeTargets like struct and interface. As per @MichalStrehovsky comments, Structs are problematic because they can be default-instantiated. We would like this to be considered unsafe (and warn) only if a constructor or a static method on the type is called to limit the amount of noise (and additional suppressions needed) when a type is annotated as such.
If we allow this on structs, we would have to warn on instance methods too, plus it would introduce problems when unsafe instance methods get called indirectly through interfaces (the tool would not be able to see that at all).

Since this enhancement is meant to help people to annotate fewer methods seems to be a good option to start just by enabling it in classes, but this issue is meant to discuss which targets we want to enable.

Risks

Related PR

Related to #50064

Author:	tlakollo
Assignees:	tlakollo
Labels:	api-suggestion, area-AssemblyLoader-coreclr, linkable-framework, untriaged
Milestone:	-

[bartonjs]

Video

• Looks good as proposed
• We discussed the Struct and Interface targets, and felt that while they'll eventually show up we shouldn't add the usage until we have an implementation backing them up.

 namespace System.Diagnostics.CodeAnalysis
 {
     [AttributeUsage(
+        AttributeTargets.Class |
         AttributeTargets.Constructor |
         AttributeTargets.Method,
         Inherited = false)]
     public sealed class RequiresUnreferencedCodeAttribute : Attribute
     {
     }
 }

[tlakollo]

Closing via #50064