From 3c35f654df3b689ed2723514438e9fec3d86d7e7 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Wed, 22 Feb 2023 06:48:35 -0800 Subject: [PATCH] Avoid deep recursion while emitting deeply nested logical conditions Fixes #66900. --- .../CSharp/Portable/CodeGen/EmitStatement.cs | 88 ++++++++++++++----- .../Test/Emit/CodeGen/CodeGenOperators.cs | 38 ++++++-- .../Test/Semantic/Semantics/RecordTests.cs | 44 ++++++++++ 3 files changed, 139 insertions(+), 31 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs index 30f8e1f7b5877..3edeec7bdaabf 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs @@ -10,6 +10,7 @@ using System.Diagnostics; using System.Linq; using System.Reflection.Metadata; +using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis.CodeGen; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -415,43 +416,84 @@ private void EmitCondBranchCore(BoundExpression condition, ref object dest, bool switch (condition.Kind) { case BoundKind.BinaryOperator: + var binOp = (BoundBinaryOperator)condition; - bool testBothArgs = sense; + Debug.Assert(binOp.ConstantValueOpt is null); - switch (binOp.OperatorKind.OperatorWithLogical()) +#nullable enable + if (binOp.OperatorKind.OperatorWithLogical() is BinaryOperatorKind.LogicalOr or BinaryOperatorKind.LogicalAnd) { - case BinaryOperatorKind.LogicalOr: - testBothArgs = !testBothArgs; - // Fall through - goto case BinaryOperatorKind.LogicalAnd; - - case BinaryOperatorKind.LogicalAnd: - if (testBothArgs) - { - // gotoif(a != sense) fallThrough - // gotoif(b == sense) dest - // fallThrough: - - object fallThrough = null; + var stack = ArrayBuilder<(BoundExpression? condition, StrongBox destBox, bool sense)>.GetInstance(); + var destBox = new StrongBox(dest); + stack.Push((binOp, destBox, sense)); - EmitCondBranch(binOp.Left, ref fallThrough, !sense); - EmitCondBranch(binOp.Right, ref dest, sense); + do + { + (BoundExpression? condition, StrongBox destBox, bool sense) top = stack.Pop(); + if (top.condition is null) + { + // This is a special entry to indicate that it is time to append the block + object? fallThrough = top.destBox.Value; if (fallThrough != null) { _builder.MarkLabel(fallThrough); } } - else + else if (top.condition.ConstantValueOpt is null && + top.condition is BoundBinaryOperator binary && + binary.OperatorKind.OperatorWithLogical() is BinaryOperatorKind.LogicalOr or BinaryOperatorKind.LogicalAnd) { - // gotoif(a == sense) labDest - // gotoif(b == sense) labDest + if (binary.OperatorKind.OperatorWithLogical() is BinaryOperatorKind.LogicalOr ? !top.sense : top.sense) + { + // gotoif(a != sense) fallThrough + // gotoif(b == sense) dest + // fallThrough: + + var fallThrough = new StrongBox(); + + // Note, operations are pushed to the stack in opposite order + stack.Push((null, fallThrough, true)); // This is a special entry to indicate that it is time to append the fallThrough block + stack.Push((binary.Right, top.destBox, top.sense)); + stack.Push((binary.Left, fallThrough, !top.sense)); + } + else + { + // gotoif(a == sense) labDest + // gotoif(b == sense) labDest - EmitCondBranch(binOp.Left, ref dest, sense); - condition = binOp.Right; + // Note, operations are pushed to the stack in opposite order + stack.Push((binary.Right, top.destBox, top.sense)); + stack.Push((binary.Left, top.destBox, top.sense)); + } + } + else if (stack.Count == 0 && ReferenceEquals(destBox.Value, top.destBox.Value)) + { + // Instead of recursion we can restart from the top with new condition + condition = top.condition; + sense = top.sense; + dest = destBox.Value; + stack.Free(); goto oneMoreTime; } - return; + else + { + EmitCondBranch(top.condition, ref top.destBox.Value, top.sense); + } + } + while (stack.Count != 0); + + dest = destBox.Value; + stack.Free(); + return; + } +#nullable disable + + switch (binOp.OperatorKind.OperatorWithLogical()) + { + case BinaryOperatorKind.LogicalOr: + case BinaryOperatorKind.LogicalAnd: + throw ExceptionUtilities.Unreachable(); case BinaryOperatorKind.Equal: case BinaryOperatorKind.NotEqual: diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs index 7c02c5eec7853..7f6fa3d64c858 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs @@ -5286,9 +5286,9 @@ public static bool Calculate(bool[] a, bool[] f) } diagnostics.Verify( - // (10,16): error CS8078: An expression is too long or complex to compile - // return a[0] && f[0] || a[1] && f[1] || a[2] && f[2] || ... - Diagnostic(ErrorCode.ERR_InsufficientStack, "a").WithLocation(10, 16) + // (10,16): error CS8078: An expression is too long or complex to compile + // return a[0] & f[0] | a[1] & f[1] | a[2] & f[2] | ... + Diagnostic(ErrorCode.ERR_InsufficientStack, "a").WithLocation(10, 16) ); } @@ -5301,10 +5301,10 @@ private static string BuildSequenceOfBinaryExpressions_03(int count = 8192) builder.Append("a["); builder.Append(i); builder.Append("]"); - builder.Append(" && "); + builder.Append(" & "); builder.Append("f["); builder.Append(i); - builder.Append("] || "); + builder.Append("] | "); } builder.Append("a["); @@ -5415,7 +5415,7 @@ static void Main() public static bool Calculate(S1[] a, S1[] f) { -" + $" return {BuildSequenceOfBinaryExpressions_03()};" + @" +" + $" return {BuildSequenceOfBinaryExpressions_06()};" + @" } } @@ -5456,6 +5456,28 @@ public static implicit operator bool (S1 x) ); } + private static string BuildSequenceOfBinaryExpressions_06(int count = 8192) + { + var builder = new System.Text.StringBuilder(); + int i; + for (i = 0; i < count; i++) + { + builder.Append("a["); + builder.Append(i); + builder.Append("]"); + builder.Append(" && "); + builder.Append("f["); + builder.Append(i); + builder.Append("] || "); + } + + builder.Append("a["); + builder.Append(i); + builder.Append("]"); + + return builder.ToString(); + } + [ConditionalFact(typeof(ClrOnly), typeof(NoIOperationValidation), Reason = "https://github.com/dotnet/roslyn/issues/29428")] [WorkItem(63689, "https://github.com/dotnet/roslyn/issues/63689")] public void EmitSequenceOfBinaryExpressions_07() @@ -5478,7 +5500,7 @@ static void Main() public static bool Calculate(S1[] a, S1[] f) { -" + $" return {BuildSequenceOfBinaryExpressions_03(count)};" + @" +" + $" return {BuildSequenceOfBinaryExpressions_06(count)};" + @" } } @@ -5544,7 +5566,7 @@ static void Main() public static bool Calculate(bool[] a, bool[] f) { -" + $" return {BuildSequenceOfBinaryExpressions_03(2048)};" + @" +" + $" return {BuildSequenceOfBinaryExpressions_06(2048)};" + @" } } "; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 2066f875d76e8..f5cc9840b212e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -30419,5 +30419,49 @@ class Attr : System.Attribute {} Assert.DoesNotContain("System.Int32 y", model.LookupSymbols(attrApplication.ArgumentList!.OpenParenToken.SpanStart + 1).Select(s => s.ToTestDisplayString())); Assert.DoesNotContain("System.Int32 y", model.LookupSymbols(mDefinition.SpanStart).Select(s => s.ToTestDisplayString())); } + + [Fact] + [WorkItem(66900, "https://github.com/dotnet/roslyn/issues/66900")] + public void Issue66900() + { + // public record ClassWithManyConstructorParameters(int P0, int P1, int P2, ...) + // { + // public static ClassWithManyConstructorParameters Create() + // { + // return new ClassWithManyConstructorParameters(P0: 0, P1: 1, P2: 2, ...); + // } + // } + + var src = @" +public record ClassWithManyConstructorParameters(int P0"; + + const int count = 2000; + + for (int i = 1; i < count; i++) + { + src += ", int P" + i; + } + + src += @") +{ + public static ClassWithManyConstructorParameters Create() + { + return new ClassWithManyConstructorParameters(P0: 0"; + + for (int i = 1; i < count; i++) + { + src += ", P" + i + ": " + i; + } + + src += @"); + } +} +"; + var comp = CreateCompilation(new[] { src, IsExternalInitTypeDefinition }, targetFramework: TargetFramework.NetCoreApp); + CompileAndVerify(comp, verify: Verification.Skipped).VerifyDiagnostics(); + + comp = CreateCompilation(new[] { src, IsExternalInitTypeDefinition }, targetFramework: TargetFramework.DesktopLatestExtended); + CompileAndVerify(comp, verify: Verification.Skipped).VerifyDiagnostics(); + } } }