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]: An attribute to indicate boxed value type is not expected for certain API #91191

Open
huoyaoyuan opened this issue Aug 28, 2023 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics
Milestone

Comments

@huoyaoyuan
Copy link
Member

huoyaoyuan commented Aug 28, 2023

Background and motivation

There are APIs that accepts object and operates with object identity, thus boxed structs will not work.

Examples including:

  • GC.KeepAlive (For keeping the collectible type alive, will KeepAlive(value.GetType()) be better?)
  • Monitor.Enter

ArgumentNullException.ThrowIfNull (#85154) is similar, but the motivation is different, and will have different meaning with nullable value type.

With such an attribute, we can have one analyzer for all these APIs, without recognizing them one by one, and also works for user code. Can it be reviewed together?

API Proposal

namespace System.Diagnostics.CodeAnalysis;

[AttributeUsage(AttributeTargets.Parameter, Inherited = true)]
public class ObjectIdentyExpectedAttribute : Attribute
{
}

The analyzer will warn if the declared type of the parameter is value type.

API Usage

// Moitor.Enter annotated with ObjectIdentyExpected

lock (myStruct) // analyzer warns here
{

}

Alternative Designs

Should it be named as what's expected, or what's unexpected?

Should it be inherited?

Should we include a custom diagnostic id for analyzer? Users are unlikely to suppress such warnings though.

Risks

No response

@huoyaoyuan huoyaoyuan added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 28, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 28, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 28, 2023
@teo-tsirpanis
Copy link
Contributor

It is valid to pass a struct to GC.KeepAlive if you want to keep its object fields alive, and I heard that the JIT optimizes it to not box.

@jakobbotsch
Copy link
Member

and I heard that the JIT optimizes it to not box.

Correct -- #54412.

@ghost
Copy link

ghost commented Aug 28, 2023

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 APIs that accepts object and operates with object identity, thus boxed structs will not work.

Examples including:

  • GC.KeepAlive (For keeping the collectible type alive, will KeepAlive(value.GetType()) be better?)
  • Monitor.Enter

ArgumentNullException.ThrowIfNull (#85154) is similar, but the motivation is different, and will have different meaning with nullable value type.

With such an attribute, we can have one analyzer for all these APIs, without recognizing them one by one, and also works for user code. Can it be reviewed together?

API Proposal

namespace System.Diagnostics.CodeAnalysis;

[AttributeUsage(AttributeTargets.Parameter, Inherited = true)]
public class ObjectIdentyExpectedAttribute : Attribute
{
}

API Usage

// Moitor.Enter annotated with ObjectIdentyExpected

lock (myStruct) // analyzer warns here
{

}

Alternative Designs

Should it be named as what's expected, or what's unexpected?

Should it be inherited?

Should we include a custom diagnostic id for analyzer? Users are unlikely to suppress such warnings though.

Risks

No response

Author: huoyaoyuan
Assignees: -
Labels:

api-suggestion, area-System.Diagnostics, untriaged, needs-area-label

Milestone: -

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 28, 2023
@tommcdon tommcdon added this to the 9.0.0 milestone Aug 31, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 31, 2023
@stephentoub stephentoub modified the milestones: 9.0.0, 10.0.0 Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics
Projects
None yet
Development

No branches or pull requests

7 participants