From 11f27836ddd565ec61f4e24703e6dcfd65b4e858 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 23 Jun 2017 23:05:25 -0700 Subject: [PATCH] Use the last effect as the result of the sequence, when lowering deconstruction --- ...writer_DeconstructionAssignmentOperator.cs | 62 ++++- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 252 ++++++++++++------ 2 files changed, 219 insertions(+), 95 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index a58a6d0f9add3..fdd0f29560675 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -39,13 +39,31 @@ private BoundExpression RewriteDeconstruction(BoundTupleExpression left, Convers ArrayBuilder lhsTargets = GetAssignmentTargetsAndSideEffects(left, temps, effects.init); BoundExpression returnValue = ApplyDeconstructionConversion(lhsTargets, right, conversion, temps, effects, isUsed, inInit: true); - if (!returnValue.HasErrors) + + BoundExpression result; + if (!isUsed) + { + // When a deconstruction is not used, we use the last effect is used as return value + var last = effects.PopLast(); + if (last is null) + { + result = null; + } + else + { + result = _factory.Sequence(temps.ToImmutableAndFree(), effects.ToImmutableAndFree(), last); + } + } + else { - returnValue = VisitExpression(returnValue); + if (!returnValue.HasErrors && isUsed) + { + returnValue = VisitExpression(returnValue); + } + result = _factory.Sequence(temps.ToImmutableAndFree(), effects.ToImmutableAndFree(), returnValue); } - BoundExpression result = _factory.Sequence(temps.ToImmutableAndFree(), effects.ToImmutableAndFree(), returnValue); - Binder.DeconstructionVariable.FreeDeconstructionVariables(lhsTargets); + Binder.DeconstructionVariable.FreeDeconstructionVariables(lhsTargets); return result; } @@ -68,7 +86,7 @@ private BoundExpression RewriteDeconstruction(BoundTupleExpression left, Convers Debug.Assert(!underlyingConversions.IsDefault); Debug.Assert(leftTargets.Count == rightParts.Length && leftTargets.Count == conversion.UnderlyingConversions.Length); - var builder = ArrayBuilder.GetInstance(leftTargets.Count); + var builder = isUsed ? ArrayBuilder.GetInstance(leftTargets.Count) : null; for (int i = 0; i < leftTargets.Count; i++) { BoundExpression resultPart; @@ -95,7 +113,7 @@ private BoundExpression RewriteDeconstruction(BoundTupleExpression left, Convers used: true, isChecked: false, isCompoundAssignment: false)); } } - builder.Add(resultPart); + builder?.Add(resultPart); } if (isUsed) @@ -109,8 +127,7 @@ private BoundExpression RewriteDeconstruction(BoundTupleExpression left, Convers } else { - // When a deconstruction is not used, we use the last value of the LHS as the return value for the lowered form - return builder.ToImmutableAndFree().Last(); + return null; } } @@ -329,6 +346,35 @@ internal static DeconstructionSideEffects GetInstance() return result; } + internal BoundExpression PopLast() + { + if (assignments.Count != 0) + { + var last = assignments.Last(); + assignments.RemoveLast(); + return last; + } + if (conversions.Count != 0) + { + var last = conversions.Last(); + conversions.RemoveLast(); + return last; + } + if (deconstructions.Count != 0) + { + var last = deconstructions.Last(); + deconstructions.RemoveLast(); + return last; + } + if (init.Count != 0) + { + var last = init.Last(); + init.RemoveLast(); + return last; + } + return null; + } + internal ImmutableArray ToImmutableAndFree() { init.AddRange(deconstructions); diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index 31382e769f845..25652e893ead6 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -3514,7 +3514,7 @@ static void Main() comp.VerifyIL("C.Main", @"{ - // Code size 72 (0x48) + // Code size 70 (0x46) .maxstack 2 .locals init (System.Collections.Generic.IEnumerator<(int, int)> V_0, int V_1, //x1 @@ -3524,35 +3524,33 @@ .maxstack 2 IL_000a: stloc.0 .try { - IL_000b: br.s IL_0033 + IL_000b: br.s IL_0031 IL_000d: ldloc.0 IL_000e: callvirt ""(int, int) System.Collections.Generic.IEnumerator<(int, int)>.Current.get"" IL_0013: dup IL_0014: ldfld ""int System.ValueTuple.Item1"" IL_0019: stloc.1 - IL_001a: dup - IL_001b: ldfld ""int System.ValueTuple.Item2"" - IL_0020: stloc.2 - IL_0021: pop - IL_0022: ldloc.1 - IL_0023: box ""int"" - IL_0028: ldloc.2 - IL_0029: box ""int"" - IL_002e: call ""void C.Print(object, object)"" - IL_0033: ldloc.0 - IL_0034: callvirt ""bool System.Collections.IEnumerator.MoveNext()"" - IL_0039: brtrue.s IL_000d - IL_003b: leave.s IL_0047 + IL_001a: ldfld ""int System.ValueTuple.Item2"" + IL_001f: stloc.2 + IL_0020: ldloc.1 + IL_0021: box ""int"" + IL_0026: ldloc.2 + IL_0027: box ""int"" + IL_002c: call ""void C.Print(object, object)"" + IL_0031: ldloc.0 + IL_0032: callvirt ""bool System.Collections.IEnumerator.MoveNext()"" + IL_0037: brtrue.s IL_000d + IL_0039: leave.s IL_0045 } finally { - IL_003d: ldloc.0 - IL_003e: brfalse.s IL_0046 - IL_0040: ldloc.0 - IL_0041: callvirt ""void System.IDisposable.Dispose()"" - IL_0046: endfinally + IL_003b: ldloc.0 + IL_003c: brfalse.s IL_0044 + IL_003e: ldloc.0 + IL_003f: callvirt ""void System.IDisposable.Dispose()"" + IL_0044: endfinally } - IL_0047: ret + IL_0045: ret }"); } @@ -3600,7 +3598,7 @@ static void Main() comp.VerifyDiagnostics(); comp.VerifyIL("C.Main", @"{ - // Code size 93 (0x5d) + // Code size 91 (0x5b) .maxstack 4 .locals init ((int, int)[] V_0, int V_1, @@ -3610,49 +3608,47 @@ .maxstack 4 IL_0005: stloc.0 IL_0006: ldc.i4.0 IL_0007: stloc.1 - IL_0008: br.s IL_0056 + IL_0008: br.s IL_0054 IL_000a: ldloc.0 IL_000b: ldloc.1 IL_000c: ldelem ""System.ValueTuple"" IL_0011: dup IL_0012: ldfld ""int System.ValueTuple.Item1"" IL_0017: stloc.2 - IL_0018: dup - IL_0019: ldfld ""int System.ValueTuple.Item2"" - IL_001e: stloc.3 - IL_001f: pop - IL_0020: ldc.i4.4 - IL_0021: newarr ""object"" - IL_0026: dup - IL_0027: ldc.i4.0 - IL_0028: ldloc.2 - IL_0029: box ""int"" - IL_002e: stelem.ref - IL_002f: dup - IL_0030: ldc.i4.1 - IL_0031: ldstr "" "" - IL_0036: stelem.ref - IL_0037: dup - IL_0038: ldc.i4.2 - IL_0039: ldloc.3 - IL_003a: box ""int"" - IL_003f: stelem.ref - IL_0040: dup - IL_0041: ldc.i4.3 - IL_0042: ldstr "" - "" - IL_0047: stelem.ref - IL_0048: call ""string string.Concat(params object[])"" - IL_004d: call ""void System.Console.Write(string)"" - IL_0052: ldloc.1 - IL_0053: ldc.i4.1 - IL_0054: add - IL_0055: stloc.1 - IL_0056: ldloc.1 - IL_0057: ldloc.0 - IL_0058: ldlen - IL_0059: conv.i4 - IL_005a: blt.s IL_000a - IL_005c: ret + IL_0018: ldfld ""int System.ValueTuple.Item2"" + IL_001d: stloc.3 + IL_001e: ldc.i4.4 + IL_001f: newarr ""object"" + IL_0024: dup + IL_0025: ldc.i4.0 + IL_0026: ldloc.2 + IL_0027: box ""int"" + IL_002c: stelem.ref + IL_002d: dup + IL_002e: ldc.i4.1 + IL_002f: ldstr "" "" + IL_0034: stelem.ref + IL_0035: dup + IL_0036: ldc.i4.2 + IL_0037: ldloc.3 + IL_0038: box ""int"" + IL_003d: stelem.ref + IL_003e: dup + IL_003f: ldc.i4.3 + IL_0040: ldstr "" - "" + IL_0045: stelem.ref + IL_0046: call ""string string.Concat(params object[])"" + IL_004b: call ""void System.Console.Write(string)"" + IL_0050: ldloc.1 + IL_0051: ldc.i4.1 + IL_0052: add + IL_0053: stloc.1 + IL_0054: ldloc.1 + IL_0055: ldloc.0 + IL_0056: ldlen + IL_0057: conv.i4 + IL_0058: blt.s IL_000a + IL_005a: ret }"); } @@ -3699,7 +3695,7 @@ static void Main() comp.VerifyDiagnostics(); comp.VerifyIL("C.Main", @"{ - // Code size 108 (0x6c) + // Code size 106 (0x6a) .maxstack 3 .locals init ((int, int)[,] V_0, int V_1, @@ -3722,12 +3718,12 @@ .maxstack 3 IL_0017: ldc.i4.0 IL_0018: callvirt ""int System.Array.GetLowerBound(int)"" IL_001d: stloc.3 - IL_001e: br.s IL_0067 + IL_001e: br.s IL_0065 IL_0020: ldloc.0 IL_0021: ldc.i4.1 IL_0022: callvirt ""int System.Array.GetLowerBound(int)"" IL_0027: stloc.s V_4 - IL_0029: br.s IL_005e + IL_0029: br.s IL_005c IL_002b: ldloc.0 IL_002c: ldloc.3 IL_002d: ldloc.s V_4 @@ -3735,30 +3731,28 @@ .maxstack 3 IL_0034: dup IL_0035: ldfld ""int System.ValueTuple.Item1"" IL_003a: stloc.s V_5 - IL_003c: dup - IL_003d: ldfld ""int System.ValueTuple.Item2"" - IL_0042: stloc.s V_6 - IL_0044: pop - IL_0045: ldloc.s V_5 - IL_0047: box ""int"" - IL_004c: ldloc.s V_6 - IL_004e: box ""int"" - IL_0053: call ""void C.Print(object, object)"" - IL_0058: ldloc.s V_4 - IL_005a: ldc.i4.1 - IL_005b: add - IL_005c: stloc.s V_4 - IL_005e: ldloc.s V_4 - IL_0060: ldloc.2 - IL_0061: ble.s IL_002b - IL_0063: ldloc.3 - IL_0064: ldc.i4.1 - IL_0065: add - IL_0066: stloc.3 - IL_0067: ldloc.3 - IL_0068: ldloc.1 - IL_0069: ble.s IL_0020 - IL_006b: ret + IL_003c: ldfld ""int System.ValueTuple.Item2"" + IL_0041: stloc.s V_6 + IL_0043: ldloc.s V_5 + IL_0045: box ""int"" + IL_004a: ldloc.s V_6 + IL_004c: box ""int"" + IL_0051: call ""void C.Print(object, object)"" + IL_0056: ldloc.s V_4 + IL_0058: ldc.i4.1 + IL_0059: add + IL_005a: stloc.s V_4 + IL_005c: ldloc.s V_4 + IL_005e: ldloc.2 + IL_005f: ble.s IL_002b + IL_0061: ldloc.3 + IL_0062: ldc.i4.1 + IL_0063: add + IL_0064: stloc.3 + IL_0065: ldloc.3 + IL_0066: ldloc.1 + IL_0067: ble.s IL_0020 + IL_0069: ret }"); } @@ -6109,7 +6103,6 @@ static void Main() ); } - [Fact] public void SimpleDiscardDeconstructInScript() { @@ -6149,6 +6142,26 @@ public void SimpleDiscardDeconstructInScript() CompileAndVerify(comp, sourceSymbolValidator: validator); } + [Fact] + public void SimpleDiscardDeconstructInScript2() + { + var source = +@" +public class C +{ + public C() { System.Console.Write(""ctor""); } + public void Deconstruct(out string x, out string y) { x = y = null; } +} +(string _, string _) = new C(); +"; + + + var comp = CreateCompilationWithMscorlib45(source, parseOptions: TestOptions.Script, options: TestOptions.DebugExe); + + comp.VerifyDiagnostics(); + CompileAndVerify(comp, expectedOutput: "ctor"); + } + [Fact] public void SingleDiscardInAssignmentInScript() { @@ -7624,5 +7637,70 @@ public static explicit operator (byte, byte)(C c) }"; CompileAndVerify(source, expectedOutput: @"3 4", additionalRefs: s_valueTupleRefs); } + + [Fact, WorkItem(19398, "https://github.com/dotnet/roslyn/issues/19398")] + public void DeconstructionLoweredToNothing() + { + var source = @" +class C +{ + static void M() + { + for (var(_, _) = (1, 2); ; (_, _) = (3, 4)) + { + } + } +}"; + var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular7, references: s_valueTupleRefs); + comp.VerifyDiagnostics(); + var verifier = CompileAndVerify(comp); + verifier.VerifyIL("C.M", @" +{ + // Code size 2 (0x2) + .maxstack 0 + IL_0000: br.s IL_0000 +}"); + } + + [Fact, WorkItem(19398, "https://github.com/dotnet/roslyn/issues/19398")] + public void DeconstructionLoweredToNothing2() + { + var source = @" +class C +{ + static void M() + { + (_, _) = (1, 2); + } +}"; + var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular7, references: s_valueTupleRefs); + comp.VerifyDiagnostics(); + var verifier = CompileAndVerify(comp); + verifier.VerifyIL("C.M", @" +{ + // Code size 1 (0x1) + .maxstack 0 + IL_0000: ret +}"); + } + + [Fact, WorkItem(19398, "https://github.com/dotnet/roslyn/issues/19398")] + public void DeconstructionLoweredToNothing3() + { + var source = @" +class C +{ + static void Main() + { + foreach (var(_, _) in new[] { (1, 2) }) + { + System.Console.Write(""once""); + } + } +}"; + var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular7, references: s_valueTupleRefs, options: TestOptions.DebugExe); + comp.VerifyDiagnostics(); + CompileAndVerify(comp, expectedOutput: "once"); + } } }