From f2ab18e4826f49a834cb05c733e8c27d5f1c7340 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Wed, 23 Feb 2022 22:41:26 -0800 Subject: [PATCH 1/4] Avoid binding lambdas unnecessarily in some error scenarios --- .../Binder/Binder.QueryUnboundLambdaState.cs | 2 +- .../Portable/Binder/Binder_Invocation.cs | 20 +- .../Portable/BoundTree/UnboundLambda.cs | 34 ++- .../Test/Semantic/Semantics/LambdaTests.cs | 30 +++ .../Semantics/OverloadResolutionPerfTests.cs | 226 ++++++++++++++++++ 5 files changed, 303 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.QueryUnboundLambdaState.cs b/src/Compilers/CSharp/Portable/Binder/Binder.QueryUnboundLambdaState.cs index 4f5f4e34a4531..6be41b32ffad5 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.QueryUnboundLambdaState.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.QueryUnboundLambdaState.cs @@ -78,7 +78,7 @@ protected override BoundBlock CreateBlockFromLambdaExpressionBody(Binder lambdaB throw ExceptionUtilities.Unreachable; } - protected override BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics) + protected override BoundBlock BindLambdaBodyCore(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics) { return _bodyFactory(lambdaSymbol, lambdaBodyBinder, diagnostics); } diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs index 05a1304c219ee..d6ab38f6ae710 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs @@ -975,6 +975,11 @@ private BoundCall BindInvocationExpressionContinued( switch (argument) { case UnboundLambda unboundLambda: + // BindForErrorRecovery() will seal the binding cache for the lambda, and + // will bind the lambda without a delegate type if not previously bound. + // Note that we're calling BindForErrorRecovery() if there are any errors in arguments, + // not necessarily an error for this argument. It's not clear if that's intentional but it can + // improve perf at the expense of Intellisense: see LambdaTests.TestLambdaWithError20(). var boundWithErrors = unboundLambda.BindForErrorRecovery(); diagnostics.AddRange(boundWithErrors.Diagnostics); break; @@ -1663,14 +1668,17 @@ private ImmutableArray BuildArgumentsForErrorRecovery(AnalyzedA { // bind the argument against each applicable parameter var unboundArgument = (UnboundLambda)argument; - foreach (var parameterList in parameterListList) + if (!unboundArgument.HasBoundForErrorRecovery) { - var parameterType = GetCorrespondingParameterType(analyzedArguments, i, parameterList); - if (parameterType?.Kind == SymbolKind.NamedType && - (object)parameterType.GetDelegateType() != null) + foreach (var parameterList in parameterListList) { - // Just assume we're not in an expression tree for the purposes of error recovery. - var discarded = unboundArgument.Bind((NamedTypeSymbol)parameterType, isExpressionTree: false); + var parameterType = GetCorrespondingParameterType(analyzedArguments, i, parameterList); + if (parameterType?.Kind == SymbolKind.NamedType && + (object)parameterType.GetDelegateType() != null) + { + // Just assume we're not in an expression tree for the purposes of error recovery. + var discarded = unboundArgument.Bind((NamedTypeSymbol)parameterType, isExpressionTree: false); + } } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs b/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs index fce66b2e44abf..e4c18e9be1d54 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs @@ -431,6 +431,8 @@ internal UnboundLambda WithNoCache() public BoundLambda Bind(NamedTypeSymbol delegateType, bool isExpressionTree) => SuppressIfNeeded(Data.Bind(delegateType, isExpressionTree)); + public bool HasBoundForErrorRecovery => Data.HasBoundForErrorRecovery; + public BoundLambda BindForErrorRecovery() => SuppressIfNeeded(Data.BindForErrorRecovery()); @@ -462,6 +464,19 @@ public TypeWithAnnotations InferReturnType(ConversionsBase conversions, NamedTyp public bool ParameterIsNullChecked(int index) { return Data.ParameterIsNullChecked(index); } } + /// + /// Lambda binding state, recorded during testing only. + /// + internal sealed class LambdaBindingData + { + /// + /// Number of lambdas bound. + /// + internal int LambdaBindingCount; + + internal int UnboundLambdaStateCount; + } + internal abstract class UnboundLambdaState { private UnboundLambda _unboundLambda = null!; // we would prefer this readonly, but we have an initialization cycle. @@ -484,6 +499,11 @@ public UnboundLambdaState(Binder binder, bool includeCache) Debug.Assert(binder != null); Debug.Assert(binder.ContainingMemberOrLambda != null); + if (binder.Compilation.TestOnlyCompilationData is LambdaBindingData data) + { + Interlocked.Increment(ref data.UnboundLambdaStateCount); + } + if (includeCache) { _bindingCache = ImmutableDictionary<(NamedTypeSymbol Type, bool IsExpressionLambda), BoundLambda>.Empty.WithComparers(BindingCacheComparer.Instance); @@ -530,7 +550,15 @@ internal UnboundLambdaState WithCaching(bool includeCache) public abstract Location ParameterLocation(int index); public abstract TypeWithAnnotations ParameterTypeWithAnnotations(int index); public abstract RefKind RefKind(int index); - protected abstract BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics); + protected BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics) + { + if (lambdaSymbol.DeclaringCompilation?.TestOnlyCompilationData is LambdaBindingData data) + { + Interlocked.Increment(ref data.LambdaBindingCount); + } + return BindLambdaBodyCore(lambdaSymbol, lambdaBodyBinder, diagnostics); + } + protected abstract BoundBlock BindLambdaBodyCore(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics); /// /// Return the bound expression if the lambda has an expression body and can be reused easily. @@ -1029,6 +1057,8 @@ public virtual Binder ParameterBinder(LambdaSymbol lambdaSymbol, Binder binder) return new WithLambdaParametersBinder(lambdaSymbol, binder); } + public bool HasBoundForErrorRecovery => _errorBinding is { }; + // UNDONE: [MattWar] // UNDONE: Here we enable the consumer of an unbound lambda that could not be // UNDONE: successfully converted to a best bound lambda to do error recovery @@ -1461,7 +1491,7 @@ protected override BoundBlock CreateBlockFromLambdaExpressionBody(Binder lambdaB return lambdaBodyBinder.CreateBlockFromExpression((ExpressionSyntax)this.Body, expression, diagnostics); } - protected override BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics) + protected override BoundBlock BindLambdaBodyCore(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics) { if (this.IsExpressionLambda) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs index 7fe132336e9b2..2b49c989f6c80 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs @@ -2554,6 +2554,36 @@ static void Mc() { } } } + [Fact] + public void TestLambdaWithError20() + { + var source = +@"using System; +using System.Linq.Expressions; +class Program +{ + static void Main() + { + M(string.Empty, y, (x, y) => x.ToString()); + } + static void M(T t, U u, Expression> action) { } +} +"; + var compilation = CreateCompilationWithMscorlib40AndSystemCore(source); + var tree = compilation.SyntaxTrees[0]; + var sm = compilation.GetSemanticModel(tree); + var lambda = tree.GetRoot().DescendantNodes().OfType().Single(); + var reference = lambda.Body.DescendantNodesAndSelf().OfType().First(); + Assert.Equal("x", reference.ToString()); + var typeInfo = sm.GetTypeInfo(reference); + // The inferred parameter type is unknown, unlike similar tests in TestLambdaWithError19, + // because BindInvocationExpressionContinued is calling unboundLambda.BindForErrorRecovery(), + // which binds the lambda without a delegate type rather than attempting to infer a delegate + // type from the inapplicable method, even though the argument error is from 'y' rather + // than from the lambda argument. + Assert.Equal(TypeKind.Error, typeInfo.Type.TypeKind); + } + // See MaxParameterListsForErrorRecovery. [Fact] public void BuildArgumentsForErrorRecovery_ManyOverloads() diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 32e6578244253..798f8de310844 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -786,5 +786,231 @@ public void DefiniteAssignment_ManySwitchCasesAndLabels() // int tmp2; // unused Diagnostic(ErrorCode.WRN_UnreferencedVar, "tmp2").WithArguments("tmp2").WithLocation(9, 13)); } + + [Fact] + public void NestedLambdasAndOverloads_NoTypes_NoErrors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1(string s) + { + F(s, s0 => { + F(s, s1 => { + F(s, s2 => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((136, 186), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify(); + } + + [Fact] + [WorkItem(1153265, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1153265")] + public void NestedLambdasAndOverloads_NoTypes_Errors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1() + { + F(s, s0 => { + F(s, s1 => { + F(s, s2 => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((84, 126), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify( + // (7,11): error CS0103: The name 's' does not exist in the current context + // F(s, s0 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(7, 11), + // (8,11): error CS0103: The name 's' does not exist in the current context + // F(s, s1 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(8, 11), + // (9,11): error CS0103: The name 's' does not exist in the current context + // F(s, s2 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + } + + [Fact] + public void NestedLambdasAndOverloads_ParameterTypes_NoErrors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1(string s) + { + F(s, (string s0) => { + F(s, (string s1) => { + F(s, (string s2) => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((14, 28), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify(); + } + + [Fact] + public void NestedLambdasAndOverloads_ParameterTypes_Errors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1() + { + F(s, (string s0) => { + F(s, (string s1) => { + F(s, (string s2) => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((14, 28), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify( + // (7,11): error CS0103: The name 's' does not exist in the current context + // F(s, s0 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(7, 11), + // (8,11): error CS0103: The name 's' does not exist in the current context + // F(s, s1 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(8, 11), + // (9,11): error CS0103: The name 's' does not exist in the current context + // F(s, s2 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + } + + [Fact] + public void NestedLambdasAndOverloads_ParameterAndReturnTypes_NoErrors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1(string s) + { + F(s, void (string s0) => { + F(s, void (string s1) => { + F(s, void (string s2) => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((3, 9), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify(); + } + + [Fact] + public void NestedLambdasAndOverloads_ParameterAndReturnTypes_Errors() + { + var source = +@"using System; +class Program +{ + static void F0(string s0, string s1, string s2) { } + static void F1() + { + F(s, void (string s0) => { + F(s, void (string s1) => { + F(s, void (string s2) => { + F0(s0, s1, s2); + }); + }); + }); + } + static void F(string s, Action a) { } + static void F(string s, Action a) { } + static void F(string s, Func f) { } + static void F(object o, Action a) { } + static void F(object o, Action a) { } + static void F(object o, Func f) { } +}"; + var comp = CreateCompilation(source); + var data = new LambdaBindingData(); + comp.TestOnlyCompilationData = data; + var diagnostics = comp.GetDiagnostics(); + Assert.Equal((3, 9), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + diagnostics.Verify( + // (7,11): error CS0103: The name 's' does not exist in the current context + // F(s, s0 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(7, 11), + // (8,11): error CS0103: The name 's' does not exist in the current context + // F(s, s1 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(8, 11), + // (9,11): error CS0103: The name 's' does not exist in the current context + // F(s, s2 => { + Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + } } } From 20e233c22bc61bed8986aea48bc603b83f40696e Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Sat, 26 Feb 2022 15:18:09 -0800 Subject: [PATCH 2/4] Track number of overloads considered in nested lambdas --- .../CSharp/Portable/Binder/Binder.cs | 14 +++++++ .../Portable/Binder/Binder_Invocation.cs | 39 +++++++++++++++---- .../Portable/Binder/BuckStopsHereBinder.cs | 4 ++ .../Semantics/OverloadResolutionPerfTests.cs | 2 +- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.cs b/src/Compilers/CSharp/Portable/Binder/Binder.cs index 232eaa7b8118a..0afcac8c29650 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.cs @@ -230,6 +230,20 @@ internal virtual bool IsLabelsScopeBinder /// internal virtual bool IsNestedFunctionBinder => false; + internal const int MaxMethodOverloadCount = 16; + + /// + /// A rough estimate of the method overload count binding nested lambdas. + /// + internal virtual int MethodOverloadCount + { + get + { + Debug.Assert(Next is object); + return Next.MethodOverloadCount; + } + } + /// /// The member containing the binding context. Note that for the purposes of the compiler, /// a lambda expression is considered a "member" of its enclosing method, field, or lambda. diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs index d6ab38f6ae710..dc4ea7f924cbf 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs @@ -180,12 +180,29 @@ private BoundExpression BindInvocationExpression( BoundExpression boundExpression = BindMethodGroup(node.Expression, invoked: true, indexed: false, diagnostics: diagnostics); boundExpression = CheckValue(boundExpression, BindValueKind.RValueOrMethodGroup, diagnostics); string name = boundExpression.Kind == BoundKind.MethodGroup ? GetName(node.Expression) : null; - BindArgumentsAndNames(node.ArgumentList, diagnostics, analyzedArguments, allowArglist: true); - result = BindInvocationExpression(node, node.Expression, name, boundExpression, analyzedArguments, diagnostics); + + // If the method group contains more than one method and if any of the + // arguments are unbound lambda expressions, then track the number of + // methods to avoid binding nested lambda expressions unnecessarily. + Binder binder = this; + if (boundExpression is BoundMethodGroup { Methods: { Length: int methodGroupLength } } && + methodGroupLength > 1 && + node.ArgumentList.Arguments.Any(arg => isLambdaExpression(arg.Expression))) + { + binder = new MethodGroupBinder(this, methodGroupLength); + } + binder.BindArgumentsAndNames(node.ArgumentList, diagnostics, analyzedArguments, allowArglist: true); + result = binder.BindInvocationExpression(node, node.Expression, name, boundExpression, analyzedArguments, diagnostics); } analyzedArguments.Free(); return result; + + static bool isLambdaExpression(SyntaxNode syntax) + { + // PROTOTYPE: Test all cases. + return syntax.Kind() is SyntaxKind.SimpleLambdaExpression or SyntaxKind.ParenthesizedLambdaExpression or SyntaxKind.AnonymousMethodExpression; + } } private BoundExpression BindArgListOperator(InvocationExpressionSyntax node, BindingDiagnosticBag diagnostics, AnalyzedArguments analyzedArguments) @@ -562,6 +579,17 @@ private static bool HasApplicableConditionalMethod(OverloadResolutionResult 0); + MethodOverloadCount = methodGroupLength * next.MethodOverloadCount; + } + + internal override int MethodOverloadCount { get; } + } + private BoundExpression BindMethodGroupInvocation( SyntaxNode syntax, SyntaxNode expression, @@ -975,11 +1003,6 @@ private BoundCall BindInvocationExpressionContinued( switch (argument) { case UnboundLambda unboundLambda: - // BindForErrorRecovery() will seal the binding cache for the lambda, and - // will bind the lambda without a delegate type if not previously bound. - // Note that we're calling BindForErrorRecovery() if there are any errors in arguments, - // not necessarily an error for this argument. It's not clear if that's intentional but it can - // improve perf at the expense of Intellisense: see LambdaTests.TestLambdaWithError20(). var boundWithErrors = unboundLambda.BindForErrorRecovery(); diagnostics.AddRange(boundWithErrors.Diagnostics); break; @@ -1668,7 +1691,7 @@ private ImmutableArray BuildArgumentsForErrorRecovery(AnalyzedA { // bind the argument against each applicable parameter var unboundArgument = (UnboundLambda)argument; - if (!unboundArgument.HasBoundForErrorRecovery) + if (MethodOverloadCount < MaxMethodOverloadCount && !unboundArgument.HasBoundForErrorRecovery) { foreach (var parameterList in parameterListList) { diff --git a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs index 5c78fd6bb40a3..682748ac24613 100644 --- a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs @@ -153,6 +153,10 @@ internal override Symbol? ContainingMemberOrLambda } } + // PROTOTYPE: Seems like an odd name for the property if the + // default value (when there are no overloads) is 1. + internal override int MethodOverloadCount => 1; + internal override bool AreNullableAnnotationsGloballyEnabled() { return GetGlobalAnnotationState(); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 798f8de310844..4ce763c1de5be 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -816,7 +816,7 @@ static void F(object o, Func f) { } var data = new LambdaBindingData(); comp.TestOnlyCompilationData = data; var diagnostics = comp.GetDiagnostics(); - Assert.Equal((136, 186), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + Assert.Equal((84, 126), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); diagnostics.Verify(); } From e22dee365eefc6d8430ee95b6fe2103eeeaf4744 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Sun, 27 Feb 2022 10:07:50 -0800 Subject: [PATCH 3/4] Avoid binding with each parameter type within nested lambdas --- .../CSharp/Portable/Binder/Binder.cs | 14 ----- .../Portable/Binder/Binder_Invocation.cs | 51 ++++++++----------- .../Portable/Binder/BuckStopsHereBinder.cs | 4 -- .../Semantics/OverloadResolutionPerfTests.cs | 2 +- 4 files changed, 21 insertions(+), 50 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.cs b/src/Compilers/CSharp/Portable/Binder/Binder.cs index 0afcac8c29650..232eaa7b8118a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.cs @@ -230,20 +230,6 @@ internal virtual bool IsLabelsScopeBinder /// internal virtual bool IsNestedFunctionBinder => false; - internal const int MaxMethodOverloadCount = 16; - - /// - /// A rough estimate of the method overload count binding nested lambdas. - /// - internal virtual int MethodOverloadCount - { - get - { - Debug.Assert(Next is object); - return Next.MethodOverloadCount; - } - } - /// /// The member containing the binding context. Note that for the purposes of the compiler, /// a lambda expression is considered a "member" of its enclosing method, field, or lambda. diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs index dc4ea7f924cbf..9f6c3c9ccc281 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs @@ -180,29 +180,12 @@ private BoundExpression BindInvocationExpression( BoundExpression boundExpression = BindMethodGroup(node.Expression, invoked: true, indexed: false, diagnostics: diagnostics); boundExpression = CheckValue(boundExpression, BindValueKind.RValueOrMethodGroup, diagnostics); string name = boundExpression.Kind == BoundKind.MethodGroup ? GetName(node.Expression) : null; - - // If the method group contains more than one method and if any of the - // arguments are unbound lambda expressions, then track the number of - // methods to avoid binding nested lambda expressions unnecessarily. - Binder binder = this; - if (boundExpression is BoundMethodGroup { Methods: { Length: int methodGroupLength } } && - methodGroupLength > 1 && - node.ArgumentList.Arguments.Any(arg => isLambdaExpression(arg.Expression))) - { - binder = new MethodGroupBinder(this, methodGroupLength); - } - binder.BindArgumentsAndNames(node.ArgumentList, diagnostics, analyzedArguments, allowArglist: true); - result = binder.BindInvocationExpression(node, node.Expression, name, boundExpression, analyzedArguments, diagnostics); + BindArgumentsAndNames(node.ArgumentList, diagnostics, analyzedArguments, allowArglist: true); + result = BindInvocationExpression(node, node.Expression, name, boundExpression, analyzedArguments, diagnostics); } analyzedArguments.Free(); return result; - - static bool isLambdaExpression(SyntaxNode syntax) - { - // PROTOTYPE: Test all cases. - return syntax.Kind() is SyntaxKind.SimpleLambdaExpression or SyntaxKind.ParenthesizedLambdaExpression or SyntaxKind.AnonymousMethodExpression; - } } private BoundExpression BindArgListOperator(InvocationExpressionSyntax node, BindingDiagnosticBag diagnostics, AnalyzedArguments analyzedArguments) @@ -579,17 +562,6 @@ private static bool HasApplicableConditionalMethod(OverloadResolutionResult 0); - MethodOverloadCount = methodGroupLength * next.MethodOverloadCount; - } - - internal override int MethodOverloadCount { get; } - } - private BoundExpression BindMethodGroupInvocation( SyntaxNode syntax, SyntaxNode expression, @@ -1676,6 +1648,22 @@ private ImmutableArray BuildArgumentsForErrorRecovery(AnalyzedA return result; } + private const int MaxLambdaExpressionDepthForErrorRecovery = 2; + + private static int NestedLambdaExpressionDepth(SyntaxNode syntax) + { + int n = 0; + while (syntax is { }) + { + if (syntax.Kind() is SyntaxKind.SimpleLambdaExpression or SyntaxKind.ParenthesizedLambdaExpression or SyntaxKind.AnonymousMethodExpression) + { + n++; + } + syntax = syntax.Parent; + } + return n; + } + private ImmutableArray BuildArgumentsForErrorRecovery(AnalyzedArguments analyzedArguments, IEnumerable> parameterListList) { var discardedDiagnostics = DiagnosticBag.GetInstance(); @@ -1691,7 +1679,8 @@ private ImmutableArray BuildArgumentsForErrorRecovery(AnalyzedA { // bind the argument against each applicable parameter var unboundArgument = (UnboundLambda)argument; - if (MethodOverloadCount < MaxMethodOverloadCount && !unboundArgument.HasBoundForErrorRecovery) + if (NestedLambdaExpressionDepth(unboundArgument.Syntax) <= MaxLambdaExpressionDepthForErrorRecovery && + !unboundArgument.HasBoundForErrorRecovery) { foreach (var parameterList in parameterListList) { diff --git a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs index 682748ac24613..5c78fd6bb40a3 100644 --- a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs @@ -153,10 +153,6 @@ internal override Symbol? ContainingMemberOrLambda } } - // PROTOTYPE: Seems like an odd name for the property if the - // default value (when there are no overloads) is 1. - internal override int MethodOverloadCount => 1; - internal override bool AreNullableAnnotationsGloballyEnabled() { return GetGlobalAnnotationState(); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 4ce763c1de5be..358fd39d8a98b 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -816,7 +816,7 @@ static void F(object o, Func f) { } var data = new LambdaBindingData(); comp.TestOnlyCompilationData = data; var diagnostics = comp.GetDiagnostics(); - Assert.Equal((84, 126), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); + Assert.Equal((104, 154), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); diagnostics.Verify(); } From ea944d19259bae749df17984be2c81372ffc9b8b Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Mon, 24 Jul 2023 12:29:36 -0700 Subject: [PATCH 4/4] Check IOperation --- .../Semantics/OverloadResolutionPerfTests.cs | 132 +++++++++++++++++- 1 file changed, 126 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index f53f3ee9410f7..729477d6c745d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -1001,7 +1001,7 @@ static void F1(string s) F(s, s0 => { F(s, s1 => { F(s, s2 => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1019,6 +1019,26 @@ static void F(object o, Func f) { } var diagnostics = comp.GetDiagnostics(); Assert.Equal((104, 154), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); diagnostics.Verify(); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } [Fact] @@ -1035,7 +1055,7 @@ static void F1() F(s, s0 => { F(s, s1 => { F(s, s2 => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1062,6 +1082,26 @@ static void F(object o, Func f) { } // (9,11): error CS0103: The name 's' does not exist in the current context // F(s, s2 => { Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } [Fact] @@ -1077,7 +1117,7 @@ static void F1(string s) F(s, (string s0) => { F(s, (string s1) => { F(s, (string s2) => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1095,6 +1135,26 @@ static void F(object o, Func f) { } var diagnostics = comp.GetDiagnostics(); Assert.Equal((14, 28), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); diagnostics.Verify(); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } [Fact] @@ -1110,7 +1170,7 @@ static void F1() F(s, (string s0) => { F(s, (string s1) => { F(s, (string s2) => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1137,6 +1197,26 @@ static void F(object o, Func f) { } // (9,11): error CS0103: The name 's' does not exist in the current context // F(s, s2 => { Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } [Fact] @@ -1152,7 +1232,7 @@ static void F1(string s) F(s, void (string s0) => { F(s, void (string s1) => { F(s, void (string s2) => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1170,6 +1250,26 @@ static void F(object o, Func f) { } var diagnostics = comp.GetDiagnostics(); Assert.Equal((3, 9), (data.LambdaBindingCount, data.UnboundLambdaStateCount)); diagnostics.Verify(); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } [Fact] @@ -1185,7 +1285,7 @@ static void F1() F(s, void (string s0) => { F(s, void (string s1) => { F(s, void (string s2) => { - F0(s0, s1, s2); + /**/F0(s0, s1, s2);/**/ }); }); }); @@ -1212,6 +1312,26 @@ static void F(object o, Func f) { } // (9,11): error CS0103: The name 's' does not exist in the current context // F(s, s2 => { Diagnostic(ErrorCode.ERR_NameNotInContext, "s").WithArguments("s").WithLocation(9, 11)); + + VerifyOperationTreeForTest(comp, + """ + IInvocationOperation (void Program.F0(System.String s0, System.String s1, System.String s2)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'F0(s0, s1, s2)') + Instance Receiver: + null + Arguments(3): + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s0) (OperationKind.Argument, Type: null) (Syntax: 's0') + IParameterReferenceOperation: s0 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's0') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s1) (OperationKind.Argument, Type: null) (Syntax: 's1') + IParameterReferenceOperation: s1 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s2) (OperationKind.Argument, Type: null) (Syntax: 's2') + IParameterReferenceOperation: s2 (OperationKind.ParameterReference, Type: System.String) (Syntax: 's2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + """); } } }