Optimize delegate creation from effectively 'static' local functions

[controlflow]

Version Used:

4.4.0

Steps to Reproduce:

Compile the following code:

var func1 = StaticMethodsOptimization.StaticMethod;
var func2 = LocalFunction; // ALLOCATES
var func3 = StaticLocalFunction;
var func4 = () => { };
var func5 = static () => { };

func1();
func2();
func3();
func4();
func5();

void LocalFunction() { } // no closure
static void StaticLocalFunction() { }

public static class StaticMethodsOptimization {
  public static void StaticMethod() { }
}

Expected Behavior:

All delegate instances cached.

Actual Behavior:

var func2 = LocalFunction; line allocates new delegate instance on each evaluation despite LocalFunction local function being effectively static.
This feels suboptimal and non-symmetric with the behavior of lambda expression.

[jcouv update:] relates to recent work on caching the delegate for static methods #58288

[jcouv]

FYI @pawchen
The handling for static local functions is currently in LocalRewriter (handling of MethodGroup conversions using DelegateCacheRewriter), but the analysis and handling of closures is in ClosureConversion.RewriteLambdaConversion which comes later.
We may need to either delay the handling method groups conversions or bring closure analysis earlier.

[pawchen]

Interesting, if we delay the handling of method group conversions, we can probably benefit from the closure rewritten result for simple cases like:

var a = F;
void F() { }

In this case void F() { } is generated as something like internal static void <<Main>$>g__F|0_0() { }, then we can then rewrite var a = F to use cache. But if we put var a = F inside a local function:

void M()
{
    void L()
    {
        var a = F;
        void F() { }
    }
}

Now void L() is generated like:

    [CompilerGenerated]
    internal static void <<Main>$>g__L|0_1()
    {
        Action action = new Action(<<Main>$>g__F|0_2);
    }

We then have to rewrite the generated methods, too.

If we can extract the closure analysis, we do not need to call rewrite for the generated methods. In fact, we've already done some work in the diagnostic pass to ensure static local functions does not capture. Sounds cleaner if we can reuse the work.

Edit: After checking the diagnostic pass, I think I was wrong. ClosureConversion.Analysis might be the place we are looking.

[pawchen]

After checking the code last night, I might have an idea, yes, we could delay method groups conversions that targets local functions in LocalRewriter.Rewrite, then handle them in ClosureConversion.Rewrite, hopefully. Let me see if I could propose some change this weekend.

[pawchen]

Created PR, please see #69801