[Analyzer Proposal]: Warn on FixedAddressValueType/ThreadStatic on instance fields.

[MichalPetryka]

Background and motivation

The runtime ignores FixedAddressValueType/ThreadStatic if the field isn't static, this might lead to unexpected GC holes/logic bugs if the programmer doesn't know this.

API Proposal

Warn on FixedAddressValueType/ThreadStatic if the field isn't static.

API Usage

class C
{
    [FixedAddressValueType]
    public Int128 S; // Warns
    [ThreadStatic]
    public int I; // Warns
}

Alternative Designs

See #129664 for discussion about this.

Risks

No response

[dotnet-policy-service]

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

[EgorBo]

I don't mind having analyzers like that, but is it worth it? it seems to be a low-level feature that implies an advanced expertise. Also, seems like the attribute is fairly rare https://github.com/search?q=FixedAddressValueType+language%3AC%23&type=code&l=C%23

[jkotas]

Related/duplicate #131093

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

[tannergooding]

For both this and #131093 I'm in general agreement with Egor that I'm not convinced its worth having.

I'm not against it and if this isn't something that we feel we want to actively push users away from instead, I'd be willing to take it to API review.

However, I'd like to hear what others on the team think first.

[jkotas]

I think about this as yet another analyzer that detects a specific dead code pattern. We have compiler warnings and analyzers that warn you about unreachable code, unused arguments, unused expression results, .... . If the code does nothing, it is typically a bug or an inefficiency.

I may be ok with a single "dead attribute" analyzer that warns whenever a BCL-provided attribute is used on an element where the attribute has no effect. We may want to start by producing full list to see whether there is enough meat.

An example beyond what's was mentioned so far:

[SuppressGCTransition] // This does nothing
static int Sum(int a, int b) => a + b;