[API/Analyzer Proposal]: An attribute+analyzer to warn about closure parameters

[colejohnson66]

Background and motivation

Many times, it is desirable to want a static delegate, not a closure to be passed in. Currently, there is no way to warn against this. Sure, you can put static before the lambda definition to prevent a closure, but that is up to the caller to do so, not the callee.

For example, when writing a source generator, especially an incremental one, many of the delegate parameters should be static, but there is no warning if you accidentally create a closure.

Some analyzers do exist to warn about any closure allocation, such as the heap allocation viewer plugin for JetBrains Rider, but those are indiscriminate in their warnings - any closure is flagged. While useful in many cases, that is not always desired.

API Proposal

As far as the API is concerned, a single attribute would be added to the BCL:

namespace System;

[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false)]
public class DisallowClosuresAttribute : Attribute;

The logic would be implemented by the analyzer.

API Usage

Example usage in the BCL:

namespace System;

public class String
{
    public static string Create<TState>(
        int length,
        TState state,
        [DisallowClosures] SpanAction<char, TState> action); // note the new attribute
}

User experience:

namespace UserCode;

public static class Program
{
    public static void Main()
    {
        // create a string of `x` length filled with `c`
        int x = int.Parse(Console.ReadLine());
        char c = Console.ReadKey().KeyChar;

        // Oops! Accidental closure creation; Warning generated
        string s = string.Create(x, c, (state, span) => span.Fill(c));
        Console.WriteLine(s);
    }
}

Alternative Designs

Alternatively, a generic analyzer that detects methods with a "state" parameter (and delegates taking said state type) could be implemented, but that could have false positives and negatives.

Risks

If applied to the BCL, users passing closures could receive warnings they were not getting before.

[stephentoub]

Example usage in the BCL

I'm not understanding in what situation we'd ever apply it. It's up to the caller how they want to invoke the method; from the method's perspective, it's not a problem if they want to allocate a new delegate, if they want to manually allocate an object the delegate points to, if they want to let the compiler do it for them with a closure, etc.

[colejohnson66]

The idea is that some methods take a state parameter so as to avoid creating closures if possible. If the method can annotate the parameter as "avoid closures", then the author would be warned that a closure is being created. A bit similar to the "capture of variable modified in outer scope" warnings that already exist.

[ghost]

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

Issue Details

---

Background and motivation

Many times, it is desirable to want a static delegate, not a closure to be passed in. Currently, there is no way to warn against this. Sure, you can put static before the lambda definition to prevent a closure, but that is up to the caller to do so, not the callee.

For example, when writing a source generator, especially an incremental one, many of the delegate parameters should be static, but there is no warning if you accidentally create a closure.

Some analyzers do exist to warn about any closure allocation, such as the heap allocation viewer plugin for JetBrains Rider, but those are indiscriminate in their warnings - any closure is flagged. While useful in many cases, that is not always desired.

API Proposal

As far as the API is concerned, a single attribute would be added to the BCL:

namespace System;

[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false)]
public class DisallowClosuresAttribute : Attribute;

The logic would be implemented by the analyzer.

API Usage

Example usage in the BCL:

namespace System;

public class String
{
    public static string Create<TState>(
        int length,
        TState state,
        [DisallowClosures] SpanAction<char, TState> action); // note the new attribute
}

User experience:

namespace UserCode;

public static class Program
{
    public static void Main()
    {
        // create a string of `x` length filled with `c`
        int x = int.Parse(Console.ReadLine());
        char c = Console.ReadKey().KeyChar;

        // Oops! Accidental closure creation; Warning generated
        string s = string.Create(x, c, (state, span) => span.Fill(c));
        Console.WriteLine(s);
    }
}

Alternative Designs

Alternatively, a generic analyzer that detects methods with a "state" parameter (and delegates taking said state type) could be implemented, but that could have false positives and negatives.

Risks

If applied to the BCL, users passing closures could receive warnings they were not getting before.

Author:	colejohnson66
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged, needs-area-label
Milestone:	-

[akarpov89]

@colejohnson66 FYI there's such attribute in the JetBrains.Annotations package, it's called RequireStaticDelegateAttribute, needless to say that ReSharper and Rider spot violations of this annotation.