Merge pull request #45473 from 333fred/func-ptr-bugs

Function pointer bug fixes
dotnet · Jun 27, 2020 · fa69a8c · fa69a8c
2 parents 076c90f + 71d1c6f
commit fa69a8c
Show file tree

Hide file tree

Showing 6 changed files with 384 additions and 9 deletions.
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
@@ -1671,6 +1671,7 @@ private BoundFunctionPointerInvocation BindFunctionPointerInvocation(SyntaxNode
 
             if (!overloadResolutionResult.Succeeded)
             {
+                ImmutableArray<FunctionPointerMethodSymbol> methods = methodsBuilder.ToImmutableAndFree();
                 overloadResolutionResult.ReportDiagnostics(
                     binder: this,
                     node.Location,
@@ -1680,15 +1681,15 @@ private BoundFunctionPointerInvocation BindFunctionPointerInvocation(SyntaxNode
                     boundExpression,
                     boundExpression.Syntax,
                     analyzedArguments,
-                    methodsBuilder.ToImmutableAndFree(),
+                    methods,
                     typeContainingConstructor: null,
                     delegateTypeBeingInvoked: null,
                     returnRefKind: funcPtr.Signature.RefKind);
 
                 return new BoundFunctionPointerInvocation(
                     node,
                     boundExpression,
-                    analyzedArguments.Arguments.SelectAsArray((expr, args) => args.binder.BindToNaturalType(expr, args.diagnostics), (binder: this, diagnostics)),
+                    BuildArgumentsForErrorRecovery(analyzedArguments, StaticCast<MethodSymbol>.From(methods)),
                     analyzedArguments.RefKinds.ToImmutableOrNull(),
                     LookupResultKind.OverloadResolutionFailure,
                     funcPtr.Signature.ReturnType,

diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
@@ -292,7 +292,7 @@ private void PerformMemberOverloadResolution<TMember>(
             if (isFunctionPointerResolution)
             {
                 RemoveCallingConventionMismatches(results, callingConvention);
-                RemoveStaticInstanceMismatches(results, requireStatic: true);
+                RemoveMethodsNotDeclaredStatic(results);
             }
 
             // NB: As in dev12, we do this AFTER removing less derived members.
@@ -387,6 +387,22 @@ private static void RemoveStaticInstanceMismatches<TMember>(ArrayBuilder<MemberR
             }
         }
 
+        private static void RemoveMethodsNotDeclaredStatic<TMember>(ArrayBuilder<MemberResolutionResult<TMember>> results) where TMember : Symbol
+        {
+            // RemoveStaticInstanceMistmatches allows methods that do not need a reciever but are not declared static,
+            // such as a local function that is not declared static. This eliminates methods that are not actually
+            // declared as static
+            for (int f = 0; f < results.Count; f++)
+            {
+                var result = results[f];
+                TMember member = result.Member;
+                if (result.Result.IsValid && !member.IsStatic)
+                {
+                    results[f] = new MemberResolutionResult<TMember>(member, result.LeastOverriddenMember, MemberAnalysisResult.StaticInstanceMismatch());
+                }
+            }
+        }
+
         private void RemoveConstraintViolations<TMember>(ArrayBuilder<MemberResolutionResult<TMember>> results) where TMember : Symbol
         {
             // When the feature 'ImprovedOverloadCandidates' is enabled, we do not include methods for which the type arguments

diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
@@ -324,7 +324,7 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
                     break;
 
                 case BoundKind.FunctionPointerLoad:
-                    EmitLoadFunction((BoundFunctionPointerLoad)expression);
+                    EmitLoadFunction((BoundFunctionPointerLoad)expression, used);
                     break;
 
                 default:
@@ -3466,11 +3466,15 @@ private void EmitCallCleanup(SyntaxNode syntax, UseKind useKind, MethodSymbol me
             }
         }
 
-        private void EmitLoadFunction(BoundFunctionPointerLoad load)
+        private void EmitLoadFunction(BoundFunctionPointerLoad load, bool used)
         {
             Debug.Assert(load.Type is { TypeKind: TypeKind.FunctionPointer });
-            _builder.EmitOpCode(ILOpCode.Ldftn);
-            EmitSymbolToken(load.TargetMethod, load.Syntax, optArgList: null);
+
+            if (used)
+            {
+                _builder.EmitOpCode(ILOpCode.Ldftn);
+                EmitSymbolToken(load.TargetMethod, load.Syntax, optArgList: null);
+            }
         }
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.cs b/src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.cs
@@ -1303,6 +1303,33 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE
             return base.VisitDelegateCreationExpression(node);
         }
 
+        public override BoundNode VisitFunctionPointerLoad(BoundFunctionPointerLoad node)
+        {
+            if (node.TargetMethod.MethodKind == MethodKind.LocalFunction)
+            {
+                Debug.Assert(node.TargetMethod is { RequiresInstanceReceiver: false, IsStatic: true });
+                ImmutableArray<BoundExpression> arguments = default;
+                ImmutableArray<RefKind> argRefKinds = default;
+
+                RemapLocalFunction(
+                    node.Syntax,
+                    node.TargetMethod,
+                    out BoundExpression receiver,
+                    out MethodSymbol remappedMethod,
+                    ref arguments,
+                    ref argRefKinds);
+
+                Debug.Assert(arguments.IsDefault &&
+                             argRefKinds.IsDefault &&
+                             receiver.Kind == BoundKind.TypeExpression &&
+                             remappedMethod is { RequiresInstanceReceiver: false, IsStatic: true });
+
+                return node.Update(remappedMethod, node.Type);
+            }
+
+            return base.VisitFunctionPointerLoad(node);
+        }
+
         public override BoundNode VisitConversion(BoundConversion conversion)
         {
             // a conversion with a method should have been rewritten, e.g. to an invocation

diff --git a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs
@@ -506,6 +506,11 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE
             return node.Update(rewrittenArgument, method, node.IsExtensionMethod, type);
         }
 
+        public override BoundNode VisitFunctionPointerLoad(BoundFunctionPointerLoad node)
+        {
+            return node.Update(VisitMethodSymbol(node.TargetMethod), VisitType(node.Type));
+        }
+
         public override BoundNode VisitLoweredConditionalAccess(BoundLoweredConditionalAccess node)
         {
             BoundExpression receiver = (BoundExpression)this.Visit(node.Receiver);