Cache the delegate for static method group conversions. (#58288)

Considering the complexities currently we have in this PR, we leave the following interesting ideas/improvements for future considerations:
* Improve handling for scenarios where the enclosing method is generic and the target method is a local function
* Adding type scoped generic containers grouped by the arity
* Take the branching op out of loops
* Use fields instead of containers
* Adding type scoped generic containers grouped by the arity
* Pulling the cache into the containing type
* Module scoped cache

src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs

@@ -71,6 +71,8 @@ internal MethodWithBody(MethodSymbol method, BoundStatement body, ImportChain? i
 
         public SynthesizedClosureEnvironment? StaticLambdaFrame;
 
+        public DelegateCacheContainer? ConcreteDelegateCacheContainer;
+
         /// <summary>
         /// A graph of method->method references for this(...) constructor initializers.
         /// Used to detect and report initializer cycles.

src/Compilers/CSharp/Portable/Errors/MessageID.cs

@@ -241,6 +241,8 @@ internal enum MessageID
         IDS_FeatureNewLinesInInterpolations = MessageBase + 12813,
         IDS_FeatureListPattern = MessageBase + 12814,
         IDS_ParameterNullChecking = MessageBase + 12815,
+
+        IDS_FeatureCacheStaticMethodGroupConversion = MessageBase + 12816,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -354,6 +356,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
                 case MessageID.IDS_FeatureGenericAttributes: // semantic check
                 case MessageID.IDS_FeatureNewLinesInInterpolations: // semantic check
                 case MessageID.IDS_FeatureListPattern: // semantic check
+                case MessageID.IDS_FeatureCacheStaticMethodGroupConversion: // lowering check
                 case MessageID.IDS_ParameterNullChecking: // syntax check
                     return LanguageVersion.Preview;
 

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/DelegateCacheContainer.cs

@@ -0,0 +1,99 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using Roslyn.Utilities;
+
+namespace Microsoft.CodeAnalysis.CSharp.Symbols;
+
+/// <summary>
+/// This type is synthesized to hold the cached delegates that target static method groups.
+/// </summary>
+internal sealed class DelegateCacheContainer : SynthesizedContainer
+{
+    private readonly Symbol _containingSymbol;
+    private readonly NamedTypeSymbol? _constructedContainer;
+    private readonly Dictionary<(TypeSymbol, MethodSymbol), FieldSymbol> _delegateFields = new(CLRSignatureComparer.Instance);
+
+    /// <summary>Creates a type-scope concrete delegate cache container.</summary>
+    internal DelegateCacheContainer(TypeSymbol containingType, int generationOrdinal)
+        : base(GeneratedNames.DelegateCacheContainerType(generationOrdinal), containingMethod: null)
+    {
+        Debug.Assert(containingType.IsDefinition);
+
+        _containingSymbol = containingType;
+    }
+
+    /// <summary>Creates a method-scope generic delegate cache container.</summary>
+    internal DelegateCacheContainer(MethodSymbol ownerMethod, int topLevelMethodOrdinal, int ownerUniqueId, int generationOrdinal)
+        : base(GeneratedNames.DelegateCacheContainerType(generationOrdinal, ownerMethod.Name, topLevelMethodOrdinal, ownerUniqueId), ownerMethod)
+    {
+        Debug.Assert(ownerMethod.IsDefinition);
+        Debug.Assert(ownerMethod.Arity > 0);
+
+        _containingSymbol = ownerMethod.ContainingType;
+        _constructedContainer = Construct(ConstructedFromTypeParameters);
+    }
+
+    public override Symbol ContainingSymbol => _containingSymbol;
+
+    public override bool AreLocalsZeroed => throw ExceptionUtilities.Unreachable;
+
+    public override TypeKind TypeKind => TypeKind.Class;
+
+    public override bool IsStatic => true;
+
+    internal override bool IsRecord => false;
+
+    internal override bool IsRecordStruct => false;
+
+    internal override bool HasPossibleWellKnownCloneMethod() => false;
+
+    internal FieldSymbol GetOrAddCacheField(SyntheticBoundNodeFactory factory, TypeSymbol delegateType, MethodSymbol targetMethod)
+    {
+        Debug.Assert(delegateType.IsDelegateType());
+
+        if (_delegateFields.TryGetValue((delegateType, targetMethod), out var field))
+        {
+            return field;
+        }
+
+        var fieldType = TypeParameters.IsEmpty ? delegateType : TypeMap.SubstituteType(delegateType).Type;
+        var fieldName = GeneratedNames.DelegateCacheContainerFieldName(_delegateFields.Count, targetMethod.Name);
+
+        field = new SynthesizedFieldSymbol(this, fieldType, fieldName, isPublic: true, isStatic: true);
+        factory.AddField(this, field);
+
+        if (!TypeParameters.IsEmpty)
+        {
+            Debug.Assert(_constructedContainer is { });
+
+            field = field.AsMember(_constructedContainer);
+        }
+
+        _delegateFields.Add((delegateType, targetMethod), field);
+
+        return field;
+    }
+
+    private sealed class CLRSignatureComparer : IEqualityComparer<(TypeSymbol delegateType, MethodSymbol targetMethod)>
+    {
+        public static readonly CLRSignatureComparer Instance = new();
+
+        public bool Equals((TypeSymbol delegateType, MethodSymbol targetMethod) x, (TypeSymbol delegateType, MethodSymbol targetMethod) y)
+        {
+            var symbolComparer = SymbolEqualityComparer.CLRSignature;
+
+            return symbolComparer.Equals(x.delegateType, y.delegateType) && symbolComparer.Equals(x.targetMethod, y.targetMethod);
+        }
+
+        public int GetHashCode((TypeSymbol delegateType, MethodSymbol targetMethod) conversion)
+        {
+            var symbolComparer = SymbolEqualityComparer.CLRSignature;
+
+            return Hash.Combine(symbolComparer.GetHashCode(conversion.delegateType), symbolComparer.GetHashCode(conversion.targetMethod));
+        }
+    }
+}

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/DelegateCacheRewriter.cs

@@ -0,0 +1,221 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.PooledObjects;
+using Roslyn.Utilities;
+
+namespace Microsoft.CodeAnalysis.CSharp;
+
+/// <summary>
+/// This type helps rewrite the delegate creations that target static method groups to use a cached instance of delegate.
+/// </summary>
+internal sealed class DelegateCacheRewriter
+{
+    private readonly SyntheticBoundNodeFactory _factory;
+    private readonly int _topLevelMethodOrdinal;
+
+    private Dictionary<MethodSymbol, DelegateCacheContainer>? _genericCacheContainers;
+
+    internal DelegateCacheRewriter(SyntheticBoundNodeFactory factory, int topLevelMethodOrdinal)
+    {
+        Debug.Assert(factory.TopLevelMethod is { });
+
+        _factory = factory;
+        _topLevelMethodOrdinal = topLevelMethodOrdinal;
+    }
+
+    internal static bool CanRewrite(BoundDelegateCreationExpression boundDelegateCreation)
+    {
+        var targetMethod = boundDelegateCreation.MethodOpt;
+
+        Debug.Assert(targetMethod is { });
+
+        return targetMethod.IsStatic && !boundDelegateCreation.IsExtensionMethod;
+    }
+
+    internal BoundExpression Rewrite(BoundDelegateCreationExpression boundDelegateCreation)
+    {
+        var targetMethod = boundDelegateCreation.MethodOpt;
+        var delegateType = boundDelegateCreation.Type;
+
+        Debug.Assert(targetMethod is { });
+
+        var oldSyntax = _factory.Syntax;
+        _factory.Syntax = boundDelegateCreation.Syntax;
+
+        var cacheContainer = GetOrAddCacheContainer(delegateType, targetMethod);
+        var cacheField = cacheContainer.GetOrAddCacheField(_factory, delegateType, targetMethod);
+
+        var boundCacheField = _factory.Field(receiver: null, cacheField);
+        var rewrittenNode = _factory.Coalesce(boundCacheField, _factory.AssignmentExpression(boundCacheField, boundDelegateCreation));
+
+        _factory.Syntax = oldSyntax;
+
+        return rewrittenNode;
+    }
+
+    private DelegateCacheContainer GetOrAddCacheContainer(TypeSymbol delegateType, MethodSymbol targetMethod)
+    {
+        Debug.Assert(_factory.ModuleBuilderOpt is { });
+        Debug.Assert(_factory.CurrentFunction is { });
+
+        var generation = _factory.ModuleBuilderOpt.CurrentGenerationOrdinal;
+
+        DelegateCacheContainer? container;
+
+        // We don't need to synthesize a container for each and every function.
+        //
+        // For example:
+        //   void LF1<T>()
+        //   {
+        //       void LF2<G>()
+        //       {
+        //           void LF3()
+        //           {
+        //               Func<T> d = SomeMethod<T>;
+        //               static void LF4 () { Func<T> d = SomeMethod<T>; }
+        //           }
+        //           
+        //           void LF5()
+        //           {
+        //               Func<T> d = SomeMethod<T>;
+        //           }
+        //       }
+        //   }
+        //
+        // In the above case, only one cached delegate is necessary, and it could be assigned to the container 'owned' by LF1.
+
+        if (!TryGetOwnerFunction(_factory.CurrentFunction, delegateType, targetMethod, out var ownerFunction))
+        {
+            var typeCompilationState = _factory.CompilationState;
+            container = typeCompilationState.ConcreteDelegateCacheContainer;
+
+            if (container is { })
+            {
+                return container;
+            }
+
+            container = new DelegateCacheContainer(typeCompilationState.Type, generation);
+            typeCompilationState.ConcreteDelegateCacheContainer = container;
+        }
+        else
+        {
+            var containers = _genericCacheContainers ??= new Dictionary<MethodSymbol, DelegateCacheContainer>(ReferenceEqualityComparer.Instance);
+
+            if (containers.TryGetValue(ownerFunction, out container))
+            {
+                return container;
+            }
+
+            container = new DelegateCacheContainer(ownerFunction, _topLevelMethodOrdinal, containers.Count, generation);
+            containers.Add(ownerFunction, container);
+        }
+
+        _factory.AddNestedType(container);
+
+        return container;
+    }
+
+    private static bool TryGetOwnerFunction(MethodSymbol currentFunction, TypeSymbol delegateType, MethodSymbol targetMethod, [NotNullWhen(true)] out MethodSymbol? ownerFunction)
+    {
+        if (targetMethod.MethodKind == MethodKind.LocalFunction)
+        {
+            // Local functions can use type parameters from their enclosing methods!
+            //
+            // For example:
+            //   void Test<T>()
+            //   {
+            //       var t = Target<int>;
+            //       static object Target<V>() => default(T);
+            //   }
+            //
+            // Therefore, without too much analysis, we select the closest generic enclosing function as the cache container owner.
+
+            for (Symbol? enclosingSymbol = currentFunction; enclosingSymbol is MethodSymbol enclosingMethod; enclosingSymbol = enclosingSymbol.ContainingSymbol)
+            {
+                if (enclosingMethod.Arity > 0)
+                {
+                    ownerFunction = enclosingMethod;
+                    return true;
+                }
+            }
+
+            ownerFunction = null;
+            return false;
+        }
+
+        // @AlekseyTs: It is Ok to create delegates for other method kinds as well.
+        // @jcouv: We'd likely want to pay attention to this code if this happens.
+        // What we really cared above was,
+        // - "Are there any type parameters from the target method that we cannot discover simply from it's signature?"
+        // As of C# 10, we only observe local functions could potentially answer yes, so we used that.
+        // If this is hit, feel free to change but please also add tests.
+        Debug.Assert(targetMethod.MethodKind == MethodKind.Ordinary);
+
+        var usedTypeParameters = PooledHashSet<TypeParameterSymbol>.GetInstance();
+        try
+        {
+            FindTypeParameters(delegateType, usedTypeParameters);
+            FindTypeParameters(targetMethod, usedTypeParameters);
+
+            for (Symbol? enclosingSymbol = currentFunction; enclosingSymbol is MethodSymbol enclosingMethod; enclosingSymbol = enclosingSymbol.ContainingSymbol)
+            {
+                if (usedTypeParametersContains(usedTypeParameters, enclosingMethod.TypeParameters))
+                {
+                    ownerFunction = enclosingMethod;
+                    return true;
+                }
+            }
+
+            ownerFunction = null;
+            return false;
+        }
+        finally
+        {
+            usedTypeParameters.Free();
+        }
+
+        static bool usedTypeParametersContains(HashSet<TypeParameterSymbol> used, ImmutableArray<TypeParameterSymbol> typeParameters)
+        {
+            foreach (var typeParameter in typeParameters)
+            {
+                if (used.Contains(typeParameter))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+    }
+
+    private static void FindTypeParameters(TypeSymbol type, HashSet<TypeParameterSymbol> result)
+        => type.VisitType(s_typeParameterSymbolCollector, result, visitCustomModifiers: true);
+
+    private static void FindTypeParameters(MethodSymbol method, HashSet<TypeParameterSymbol> result)
+    {
+        FindTypeParameters(method.ContainingType, result);
+
+        foreach (var typeArgument in method.TypeArgumentsWithAnnotations)
+        {
+            typeArgument.VisitType(type: null, typeWithAnnotationsPredicate: null, s_typeParameterSymbolCollector, result, visitCustomModifiers: true);
+        }
+    }
+
+    private static readonly Func<TypeSymbol, HashSet<TypeParameterSymbol>, bool, bool> s_typeParameterSymbolCollector = (typeSymbol, result, _) =>
+    {
+        if (typeSymbol is TypeParameterSymbol typeParameter)
+        {
+            result.Add(typeParameter);
+        }
+
+        return false;
+    };
+}

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs

@@ -27,6 +27,8 @@ internal sealed partial class LocalRewriter : BoundTreeRewriterWithStackGuard
         private LoweredDynamicOperationFactory _dynamicFactory;
         private bool _sawLambdas;
         private int _availableLocalFunctionOrdinal;
+        private readonly int _topLevelMethodOrdinal;
+        private DelegateCacheRewriter? _lazyDelegateCacheRewriter;
         private bool _inExpressionLambda;
 
         private bool _sawAwait;
@@ -57,6 +59,7 @@ private LocalRewriter(
             _dynamicFactory = new LoweredDynamicOperationFactory(factory, containingMethodOrdinal);
             _previousSubmissionFields = previousSubmissionFields;
             _allowOmissionOfConditionalCalls = allowOmissionOfConditionalCalls;
+            _topLevelMethodOrdinal = containingMethodOrdinal;
             _diagnostics = diagnostics;
 
             Debug.Assert(instrumenter != null);

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs

@@ -425,9 +425,26 @@ private BoundExpression MakeConversionNodeCore(
                         var receiver = (!method.RequiresInstanceReceiver && !oldNodeOpt.IsExtensionMethod && !method.IsAbstract) ? _factory.Type(method.ContainingType) : mg.ReceiverOpt;
                         Debug.Assert(receiver is { });
                         _factory.Syntax = oldSyntax;
-                        return new BoundDelegateCreationExpression(syntax, argument: receiver, methodOpt: method,
-                                                                   isExtensionMethod: oldNodeOpt.IsExtensionMethod, wasTargetTyped: false, type: rewrittenType);
+
+                        var boundDelegateCreation = new BoundDelegateCreationExpression(syntax, argument: receiver, methodOpt: method,
+                                                                                        isExtensionMethod: oldNodeOpt.IsExtensionMethod, wasTargetTyped: false, type: rewrittenType);
+
+                        Debug.Assert(_factory.TopLevelMethod is { });
+
+                        if (_factory.Compilation.LanguageVersion >= MessageID.IDS_FeatureCacheStaticMethodGroupConversion.RequiredVersion()
+                            && !_inExpressionLambda // The tree structure / meaning for expression trees should remain untouched.
+                            && _factory.TopLevelMethod.MethodKind != MethodKind.StaticConstructor // Avoid caching twice if people do it manually.
+                            && DelegateCacheRewriter.CanRewrite(boundDelegateCreation))
+                        {
+                            var rewriter = _lazyDelegateCacheRewriter ??= new DelegateCacheRewriter(_factory, _topLevelMethodOrdinal);
+                            return rewriter.Rewrite(boundDelegateCreation);
+                        }
+                        else
+                        {
+                            return boundDelegateCreation;
+                        }
                     }
+
                 default:
                     break;
             }