Skip to content
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.

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

[API Proposal]: Enhance RequiresDynamicCode Attribute to apply to class #67368

Closed
LakshanF opened this issue Mar 30, 2022 · 4 comments
Closed
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics
Milestone

Comments

@LakshanF
Copy link
Member

LakshanF commented Mar 30, 2022

Background and motivation

There are scenarios where it makes sense to apply this attribute to a class as discussed in this PR. In the particular case, the internal class is around runtime code generation and applying at the class level results in a better user experience as opposed to annotating at member level.

The trimmer equivalent attribute, RequiresUnreferencedCode, already allows the attribute to be applied at class level.

API Proposal

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

API Usage

[RequiresDynamicCode("The native code for Regex compilation might not be available at runtime.")]
internal abstract class RegexCompiler

Alternative Designs

None and leave the RequiresDynamicCodeAttribute class as is, which would require working around the issue

Risks

Requires Analyzers build for this attribute and NativeAot compiler to adjust to the enhanced scope.

@LakshanF LakshanF added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 30, 2022
@dotnet-issue-labeler
Copy link

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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 30, 2022
@tlakollo tlakollo added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Mar 30, 2022
@tlakollo
Copy link
Contributor

I added the api-ready-for-review I don't think we will discuss anything interesting in this PR. I think is only worth discussing this in the API review meeting.

@ghost
Copy link

ghost commented Mar 31, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

There are scenarios where it makes sense to apply this attribute to a class as discussed in this PR. In the particular case, the internal class is around runtime code generation and applying at the class level results in a better user experience as opposed to annotating at member level.

The trimmer equivalent attribute, RequiresUnreferencedCode, already allows the attribute to be applied at class level.

API Proposal

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

API Usage

[RequiresDynamicCode("The native code for Regex compilation might not be available at runtime.")]
internal abstract class RegexCompiler

Alternative Designs

None and leave the RequiresDynamicCodeAttribute class as is, which would require working around the issue

Risks

Requires Analyzers build for this attribute and NativeAot compiler to adjust to the enhanced scope.

Author: LakshanF
Assignees: -
Labels:

api-suggestion, area-System.Diagnostics, api-ready-for-review

Milestone: -

@LakshanF LakshanF added the blocked Issue/PR is blocked on something - see comments label Mar 31, 2022
@tlakollo tlakollo added blocking Marks issues that we want to fast track in order to unblock other important work and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation blocked Issue/PR is blocked on something - see comments labels Mar 31, 2022
@tommcdon tommcdon added this to the 7.0.0 milestone Apr 3, 2022
@bartonjs
Copy link
Member

bartonjs commented Apr 8, 2022

Video

Looks good as proposed

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

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics
Projects
None yet
Development

No branches or pull requests

5 participants