Discussion: allow attributes on local functions

[jcouv]

Some use-cases:

• DllImports
• put MethodImplOptions to force inlining on some local functions. Instead, RavenDB folks had to expand them to regular private methods to get the inlining they wanted.
• Null/NotNull attributes from ReSharper

Note: this needs to be tested with nullability attributes.

Relates to dotnet/roslyn#19372

[HaloFour]

DllImports

This will require some minor changes to the C# spec as currently it doesn't seem that the compiler can handle extern local functions?

I'm also going to assume that this does not include attempting to also apply attributes on the parameters generated to support enclosing the local scope? Any concerns regarding whether the compiler generates a static method with ref parameters to enclose the scope vs. a closure "display" class and an instance method and how that might affect scenarios in which someone would want to annotate these methods?

[agocke]

The main work to be done here is validating that the proposed syntax is unambiguous. Otherwise, it should be pretty straightforward to thread this through the compiler.

[stephentoub]

put MethodImplOptions to force inlining on some local functions

I've also had a need for the opposite, to prevent a function from being inlined in order to help a caller of it get inlined (and with minimal assembly), e.g.

Data Get()
{
    return _data ?? Initialize();

    [MethodImpl(MethodImplOptions.NoInlining)]
    Data Initialize()
    {
        ... // expensive code to initialize _data;
        return _data;
    }
}

[steevcoco]

... Just a "bump" as @stephentoub commented: I would also like to apply `MethodImpl' to local functions.

It might be nifty if it was also possible with lambdas ... (AggressiveInlining.)

[Rabadash8820]

Any updates here? I am interested in the performance benefits of AggressiveInlining small local functions for video game code. This feature was lauded in a Unite 2017 presentation, and I would really like to combine faster inline code with more modular local functions.

[yaakov-h]

I find it funny that you want AggressiveInlining, when today I wanted this exact feature for NoInlining 😆

[jnm2]

I wanted NoInlining recently too :D
Was dancing around because jitting the helper method is no good until the calling method has executed AssemblyResolve +=.

[Happypig375]

Just hit this one today. I wanted to use [System.Diagnostics.Conditional("DEBUG")] on a local method. Had to use a regular private method instead.

[bartdesmet]

Another case is for EnumeratorCancellation on the CancellationToken parameter of a local function implementing an async iterator, which is common when implementing query operators.

public static IAsyncEnumerable<T> Where<T>(this IAsyncEnumerable<T> source, Func<T, bool> predicate)
{
    if (source == null)
        throw new ArgumentNullException(nameof(source));
    if (predicate == null)
        throw new ArgumentNullException(nameof(predicate));

    return Core();

    async IAsyncEnumerable<T> Core([EnumeratorCancellation] CancellationToken token = default)
    {
        await foreach (var item in source.WithCancellation(token))
        {
            if (predicate(item))
            {
                yield return item;
            }
        }
    }
}

[Happypig375]

This has been implemented. I am currently using dotnet SDK 3.1.301 and I can put the Conditional attribute on local static methods (static is required or else error). I can also put NotNullWhen on parameters. This has been implemented.

[333fred]

@Happypig375 yes, it has been implemented. We keep the issue open until we have a spec for the feature, however.

[stephentoub]

static is required or else error

yes, it has been implemented

The design is that attributes are only allowed on local functions if they're static?

[stephentoub]

static is required or else error

Nevermind, I just tried it and it doesn't appear that static is required. Phew :)

[Happypig375]

static is required or else error

yes, it has been implemented

The design is that attributes are only allowed on local functions if they're static?

It is only limited to Conditional.

[julealgon]

Wait... has this now been implemented for all attributes? I just tested and it seems to allow any attribute on the local function itself, not only [Conditional].

[jcouv]

@julealgon That's correct. Local functions support all attributes in C# 9, not just [Conditional].

[julealgon]

I have a separate use case for adding attributes to the local function itself for use inside test methods... wanted to try something like using one local function to provide data to another local function, kinda like a "local" datasource.