From cc2d6e860a2bd5cb4488415f0a2c7d46ecad6d53 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Thu, 23 Feb 2017 12:31:58 -0800 Subject: [PATCH] Eliminate bound method groups during local lowering (#17227) Fixes #14882 Fixes #13840 Fixes #13915 --- .../CSharp/Portable/BoundTree/BoundNodes.xml | 13 ++- .../CSharp/Portable/CodeGen/EmitConversion.cs | 9 +- .../CSharp/Portable/CodeGen/EmitExpression.cs | 8 +- .../AsyncRewriter/AwaitExpressionSpiller.cs | 4 +- .../ExpressionLambdaRewriter.cs | 11 ++- .../LambdaRewriter/LambdaRewriter.Analysis.cs | 32 +------ ...Rewriter.LocalFunctionReferenceRewriter.cs | 25 ----- .../Lowering/LambdaRewriter/LambdaRewriter.cs | 15 +-- .../LocalRewriter/LocalRewriter_Conversion.cs | 12 +++ ...ocalRewriter_DelegateCreationExpression.cs | 11 +++ .../Lowering/MethodToClassRewriter.cs | 92 ++----------------- .../Rewriters/CapturedVariableRewriter.cs | 22 +---- 12 files changed, 60 insertions(+), 194 deletions(-) diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index d99de71579d..fc9deea0d67 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -1389,11 +1389,22 @@ - + + diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitConversion.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitConversion.cs index 4b0fbf60412..e54991034c2 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitConversion.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitConversion.cs @@ -14,8 +14,7 @@ private void EmitConversionExpression(BoundConversion conversion, bool used) switch (conversion.ConversionKind) { case ConversionKind.MethodGroup: - EmitMethodGroupConversion(conversion, used); - return; + throw ExceptionUtilities.UnexpectedValue(conversion.ConversionKind); case ConversionKind.NullToPointer: // The null pointer is represented as 0u. _builder.EmitIntConstant(0); @@ -313,11 +312,5 @@ private MethodSymbol DelegateConstructor(SyntaxNode syntax, TypeSymbol delegateT _diagnostics.Add(ErrorCode.ERR_BadDelegateConstructor, syntax.Location, delegateType); return null; } - - private void EmitMethodGroupConversion(BoundConversion conversion, bool used) - { - var group = (BoundMethodGroup)conversion.Operand; - EmitDelegateCreation(conversion, group.InstanceOpt, conversion.IsExtensionMethod, conversion.SymbolOpt, conversion.Type, used); - } } } diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index 04d493d051f..72b6eef8ed4 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -633,8 +633,8 @@ private void EmitDupExpression(BoundDup expression, bool used) private void EmitDelegateCreationExpression(BoundDelegateCreationExpression expression, bool used) { - var mg = expression.Argument as BoundMethodGroup; - var receiver = mg != null ? mg.ReceiverOpt : expression.Argument; + Debug.Assert(expression.Argument?.Kind != BoundKind.MethodGroup); + var receiver = expression.Argument; var meth = expression.MethodOpt ?? receiver.Type.DelegateInvokeMethod(); Debug.Assert((object)meth != null); EmitDelegateCreation(expression, receiver, expression.IsExtensionMethod, meth, expression.Type, used); @@ -1269,7 +1269,7 @@ private bool CanUseCallOnRefTypeReceiver(BoundExpression receiver) case ConversionKind.MethodGroup: case ConversionKind.AnonymousFunction: - return true; + throw ExceptionUtilities.UnexpectedValue(conversion.ConversionKind); case ConversionKind.ExplicitReference: case ConversionKind.ImplicitReference: @@ -3010,9 +3010,9 @@ private TypeSymbol StackMergeType(BoundExpression expr) var conversion = (BoundConversion)expr; var conversionKind = conversion.ConversionKind; if (conversionKind.IsImplicitConversion() && - conversionKind != ConversionKind.MethodGroup && conversionKind != ConversionKind.NullLiteral) { + Debug.Assert(conversionKind != ConversionKind.MethodGroup); return StackMergeType(conversion.Operand); } break; diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs index 5a7cea1228c..f04a6dc2692 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs @@ -856,9 +856,7 @@ public override BoundNode VisitConversion(BoundConversion node) public override BoundNode VisitMethodGroup(BoundMethodGroup node) { - BoundSpillSequenceBuilder builder = null; - var receiver = VisitExpression(ref builder, node.ReceiverOpt); - return UpdateExpression(builder, node.Update(node.TypeArgumentsOpt, node.Name, node.Methods, node.LookupSymbolOpt, node.LookupError, node.Flags, receiver, node.ResultKind)); + throw ExceptionUtilities.Unreachable; } public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationExpression node) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs index 995ab95aaf7..1461a0a1103 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs @@ -688,10 +688,15 @@ private BoundExpression DelegateCreation(BoundExpression receiver, MethodSymbol private BoundExpression VisitDelegateCreationExpression(BoundDelegateCreationExpression node) { - var mg = node.Argument as BoundMethodGroup; - if (mg != null) + if (node.Argument.Kind == BoundKind.MethodGroup) { - return DelegateCreation(mg.ReceiverOpt, node.MethodOpt, node.Type, node.MethodOpt.IsStatic && !mg.SearchExtensionMethods); + throw ExceptionUtilities.UnexpectedValue(BoundKind.MethodGroup); + } + + if ((object)node.MethodOpt != null) + { + bool staticMember = node.MethodOpt.IsStatic && !node.IsExtensionMethod; + return DelegateCreation(node.Argument, node.MethodOpt, node.Type, staticMember); } var d = node.Argument.Type as NamedTypeSymbol; diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index 7cb3de477d6..2e3965882f6 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -557,37 +557,7 @@ private BoundNode VisitSyntaxWithReceiver(SyntaxNode syntax, BoundNode receiver) public override BoundNode VisitMethodGroup(BoundMethodGroup node) { - // We only get here in error cases, as normally the enclosing node is a method group conversion - // whose visit (below) doesn't call this. So we don't know which method is to be selected, and - // therefore don't know if the receiver is used. Assume if the receiver was provided, it is used. - var receiverOpt = node.ReceiverOpt; - if (receiverOpt != null) - { - return VisitSyntaxWithReceiver(node.Syntax, receiverOpt); - } - return null; - } - - public override BoundNode VisitConversion(BoundConversion node) - { - if (node.ConversionKind == ConversionKind.MethodGroup) - { - if (node.SymbolOpt?.MethodKind == MethodKind.LocalFunction) - { - // Use OriginalDefinition to strip generic type parameters - ReferenceVariable(node.Syntax, node.SymbolOpt.OriginalDefinition); - MethodsConvertedToDelegates.Add(node.SymbolOpt.OriginalDefinition); - } - if (node.IsExtensionMethod || ((object)node.SymbolOpt != null && !node.SymbolOpt.IsStatic)) - { - return VisitSyntaxWithReceiver(node.Syntax, ((BoundMethodGroup)node.Operand).ReceiverOpt); - } - return null; - } - else - { - return base.VisitConversion(node); - } + throw ExceptionUtilities.Unreachable; } public override BoundNode VisitPropertyAccess(BoundPropertyAccess node) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs index ad77288869f..5c033fe33a9 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs @@ -88,31 +88,6 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE return base.VisitDelegateCreationExpression(node); } - - public override BoundNode VisitConversion(BoundConversion conversion) - { - if (conversion.ConversionKind == ConversionKind.MethodGroup && - conversion.SymbolOpt?.MethodKind == MethodKind.LocalFunction) - { - BoundExpression receiver; - MethodSymbol method; - var arguments = default(ImmutableArray); - _lambdaRewriter.RemapLocalFunction( - conversion.Syntax, - conversion.SymbolOpt, - out receiver, - out method, - ref arguments); - - return new BoundDelegateCreationExpression( - conversion.Syntax, - receiver, - method, - isExtensionMethod: false, - type: conversion.Type); - } - return base.VisitConversion(conversion); - } } /// diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 81c2d7be351..233c4b00b8e 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -1174,6 +1174,7 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE public override BoundNode VisitConversion(BoundConversion conversion) { + Debug.Assert(conversion.ConversionKind != ConversionKind.MethodGroup); if (conversion.ConversionKind == ConversionKind.AnonymousFunction) { var result = (BoundExpression)RewriteLambdaConversion((BoundLambda)conversion.Operand); @@ -1194,20 +1195,6 @@ public override BoundNode VisitConversion(BoundConversion conversion) return result; } - if (conversion.ConversionKind == ConversionKind.MethodGroup && - conversion.SymbolOpt?.MethodKind == MethodKind.LocalFunction) - { - var rewritten = conversion.Update( - conversion.Operand, - conversion.Conversion, - conversion.IsBaseConversion, - conversion.Checked, - conversion.ExplicitCastInCode, - conversion.ConstantValueOpt, - this.VisitType(conversion.Type)); - - return PartiallyLowerLocalFunctionReference(rewritten); - } return base.VisitConversion(conversion); } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs index ac486e67106..6e9ae372936 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs @@ -332,6 +332,18 @@ private static bool IsFloatPointExpressionOfUnknownPrecision(BoundExpression rew explicitCastInCode: explicitCastInCode, rewrittenType: (NamedTypeSymbol)rewrittenType); + case ConversionKind.MethodGroup: + { + // we eliminate the method group conversion entirely from the bound nodes following local lowering + var mg = (BoundMethodGroup)rewrittenOperand; + var method = oldNode.SymbolOpt; + Debug.Assert((object)method != null); + var oldSyntax = _factory.Syntax; + _factory.Syntax = (mg.ReceiverOpt ?? mg).Syntax; + var receiver = (method.IsStatic && !oldNode.IsExtensionMethod) ? _factory.Type(method.ContainingType) : VisitExpression(mg.ReceiverOpt); + _factory.Syntax = oldSyntax; + return new BoundDelegateCreationExpression(syntax, argument: receiver, methodOpt: method, isExtensionMethod: oldNode.IsExtensionMethod, type: rewrittenType); + } default: break; } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DelegateCreationExpression.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DelegateCreationExpression.cs index a8dea49e5d7..6da8ad1a1fb 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DelegateCreationExpression.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DelegateCreationExpression.cs @@ -21,6 +21,17 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE return new BoundDelegateCreationExpression(node.Syntax, loweredReceiver, methodOpt: null, isExtensionMethod: false, type: node.Type); } + if (node.Argument.Kind == BoundKind.MethodGroup) + { + var mg = (BoundMethodGroup)node.Argument; + var method = node.MethodOpt; + var oldSyntax = _factory.Syntax; + _factory.Syntax = (mg.ReceiverOpt ?? mg).Syntax; + var receiver = (method.IsStatic && !node.IsExtensionMethod) ? _factory.Type(method.ContainingType) : VisitExpression(mg.ReceiverOpt); + _factory.Syntax = oldSyntax; + return node.Update(receiver, method, node.IsExtensionMethod, node.Type); + } + return base.VisitDelegateCreationExpression(node); } } diff --git a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs index 5e5a8dd3508..4b8e2d4becf 100644 --- a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs @@ -201,8 +201,9 @@ private Conversion RewriteConversion(Conversion conversion) { case ConversionKind.ExplicitUserDefined: case ConversionKind.ImplicitUserDefined: - case ConversionKind.MethodGroup: return new Conversion(conversion.Kind, VisitMethodSymbol(conversion.Method), conversion.IsExtensionMethod); + case ConversionKind.MethodGroup: + throw ExceptionUtilities.UnexpectedValue(conversion.Kind); default: return conversion; } @@ -476,36 +477,17 @@ public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpre return rewritten; } - public override BoundNode VisitConversion(BoundConversion conversion) - { - if (conversion.ConversionKind == ConversionKind.MethodGroup && (object)conversion.SymbolOpt != null) - { - return RewriteMethodGroupConversion(conversion); - } - else - { - return base.VisitConversion(conversion); - } - } - public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationExpression node) { BoundExpression originalArgument = node.Argument; BoundExpression rewrittenArgument = (BoundExpression)this.Visit(originalArgument); MethodSymbol method = node.MethodOpt; - if (originalArgument.Kind == BoundKind.MethodGroup && rewrittenArgument.Kind == BoundKind.MethodGroup) - { - // if the original argument was a method group AND the receiver was BoundKind.BaseReference - // and the visited argument is still a method group with receiver overridden, change the - // method to point to the wrapper method - var originalReceiver = ((BoundMethodGroup)originalArgument).ReceiverOpt; - var newReceiver = ((BoundMethodGroup)rewrittenArgument).ReceiverOpt; - if (BaseReferenceInReceiverWasRewritten(originalReceiver, newReceiver) && method.IsMetadataVirtual()) - { - method = GetMethodWrapperForBaseNonVirtualCall(method, originalArgument.Syntax); - // NOTE: we substitute the method in rewritten bound delegate - // creation node, but leave the method group unchanged - } + + // if the original receiver was BoundKind.BaseReference (i.e. from a method group) + // and the receiver is overridden, change the method to point to a wrapper method + if (BaseReferenceInReceiverWasRewritten(originalArgument, rewrittenArgument) && method.IsMetadataVirtual()) + { + method = GetMethodWrapperForBaseNonVirtualCall(method, originalArgument.Syntax); } method = VisitMethodSymbol(method); TypeSymbol type = this.VisitType(node.Type); @@ -657,64 +639,6 @@ private static bool BaseReferenceInReceiverWasRewritten(BoundExpression original rewrittenReceiver != null && rewrittenReceiver.Kind != BoundKind.BaseReference; } - private BoundNode RewriteMethodGroupConversion(BoundConversion conversion) - { - // in a method group conversion, we may need to rewrite the selected method - BoundMethodGroup operand = (BoundMethodGroup)conversion.Operand; - BoundExpression originalReceiverOpt = operand.ReceiverOpt; - BoundExpression receiverOpt; - - if (originalReceiverOpt == null) - { - receiverOpt = null; - } - else if (!conversion.IsExtensionMethod && conversion.SymbolOpt.IsStatic) - { - receiverOpt = new BoundTypeExpression(originalReceiverOpt.Syntax, null, VisitType(originalReceiverOpt.Type)); - } - else - { - receiverOpt = (BoundExpression)Visit(originalReceiverOpt); - } - - TypeSymbol type = this.VisitType(conversion.Type); - - MethodSymbol method = conversion.SymbolOpt; - - // if the original receiver was a base access and is was rewritten, - // change the method to point to the wrapper method - if (BaseReferenceInReceiverWasRewritten(originalReceiverOpt, receiverOpt) && method.IsMetadataVirtual()) - { - method = GetMethodWrapperForBaseNonVirtualCall(method, conversion.Syntax); - } - - method = VisitMethodSymbol(method); - operand = operand.Update( - TypeMap.SubstituteTypesWithoutModifiers(operand.TypeArgumentsOpt), - method.Name, - operand.Methods, - operand.LookupSymbolOpt, - operand.LookupError, - operand.Flags, - receiverOpt, - operand.ResultKind); - - var conversionInfo = conversion.Conversion; - if (conversionInfo.Method != (object)method) - { - conversionInfo = conversionInfo.SetConversionMethod(method); - } - - return conversion.Update( - operand, - conversionInfo, - isBaseConversion: conversion.IsBaseConversion, - @checked: conversion.Checked, - explicitCastInCode: conversion.ExplicitCastInCode, - constantValueOpt: conversion.ConstantValueOpt, - type: type); - } - /// /// A wrapper method that is created for non-virtually calling a base-class /// virtual method from other classes (like those created for lambdas...). diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/CapturedVariableRewriter.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/CapturedVariableRewriter.cs index 4af77da7882..5b7c5391e86 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/CapturedVariableRewriter.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/CapturedVariableRewriter.cs @@ -70,27 +70,7 @@ public override BoundNode VisitParameter(BoundParameter node) public override BoundNode VisitMethodGroup(BoundMethodGroup node) { - if ((node.Flags & BoundMethodGroupFlags.HasImplicitReceiver) == BoundMethodGroupFlags.HasImplicitReceiver && - (object)_targetMethodThisParameter == null) - { - // This can happen in static contexts. - // NOTE: LocalRewriter has already been run, so the receiver has already been replaced with an - // appropriate type expression, if this is a static context. - // NOTE: Don't go through VisitThisReference, because it'll produce a diagnostic. - return node.Update( - node.TypeArgumentsOpt, - node.Name, - node.Methods, - node.LookupSymbolOpt, - node.LookupError, - node.Flags, - receiverOpt: null, - resultKind: node.ResultKind); - } - else - { - return base.VisitMethodGroup(node); - } + throw ExceptionUtilities.Unreachable; } public override BoundNode VisitThisReference(BoundThisReference node) -- GitLab