diff --git a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs index 6270abf708b6e71fe40c37db80c60e7bf185b703..989a9d21e95adc2d5abbff979de4b438ad936d9f 100644 --- a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs +++ b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs @@ -1214,8 +1214,13 @@ private static bool SameTest(BoundDagTest x, BoundDagTest y) case BoundKind.DagValueTest: return ((BoundDagValueTest)x).Value == ((BoundDagValueTest)y).Value; - default: + case BoundKind.DagNullTest: + case BoundKind.DagNonNullTest: return true; + + default: + // For an evaluation, we defer to its .Equals + return x.Equals(y); } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs index 4a08fbc9b4866b4857e0cbb109c5a6a0b16d542f..fd1de521b23439715a1e3370470b5e2b2a6fca34 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs @@ -17,7 +17,7 @@ private Symbol Symbol { switch (this) { - case BoundDagFieldEvaluation e: return e.Field; + case BoundDagFieldEvaluation e: return e.Field.CorrespondingTupleField ?? e.Field; case BoundDagPropertyEvaluation e: return e.Property; case BoundDagTypeEvaluation e: return e.Type; case BoundDagDeconstructEvaluation e: return e.DeconstructMethod; diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs index c7760a9133a385a6af32df45334f0c21d5fc4c72..5ebb3c3d63877c37cefd80c8a333e4d416ddac8f 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.CodeAnalysis.CSharp.Symbols; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp @@ -11,6 +12,8 @@ partial class BoundDagTemp /// public bool IsOriginalInput => this.Source == null; + public static BoundDagTemp ForOriginalInput(SyntaxNode syntax, TypeSymbol type) => new BoundDagTemp(syntax, type, null, 0); + public override bool Equals(object obj) => obj is BoundDagTemp other && this.Equals(other); public bool Equals(BoundDagTemp other) { diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs index e15247241dd607c791a47fc8e614b2adacd7f74c..f0d43a833d6533340c300829bfdab5986019449c 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs @@ -116,7 +116,7 @@ public BoundDecisionDag Rewrite(Func /// /// - public BoundDecisionDagNode TrivialReplacement(BoundDecisionDagNode dag, Func replacement) + public static BoundDecisionDagNode TrivialReplacement(BoundDecisionDagNode dag, Func replacement) { switch (dag) { @@ -192,5 +192,99 @@ BoundDecisionDagNode makeReplacement(BoundDecisionDagNode dag, Func + /// Starting with `this` state, produce a human-readable description of the state tables. + /// This is very useful for debugging and optimizing the dag state construction. + /// + internal new string Dump() + { + var allStates = this.TopologicallySortedNodes; + var stateIdentifierMap = PooledDictionary.GetInstance(); + for (int i = 0; i < allStates.Length; i++) + { + stateIdentifierMap.Add(allStates[i], i); + } + + int nextTempNumber = 0; + var tempIdentifierMap = PooledDictionary.GetInstance(); + int tempIdentifier(BoundDagEvaluation e) + { + return (e == null) ? 0 : tempIdentifierMap.TryGetValue(e, out int value) ? value : tempIdentifierMap[e] = ++nextTempNumber; + } + + string tempName(BoundDagTemp t) + { + return $"t{tempIdentifier(t.Source)}{(t.Index != 0 ? $".{t.Index.ToString()}" : "")}"; + } + + var resultBuilder = PooledStringBuilder.GetInstance(); + var result = resultBuilder.Builder; + + foreach (var state in allStates) + { + result.AppendLine($"State " + stateIdentifierMap[state]); + switch (state) + { + case BoundTestDecisionDagNode node: + result.AppendLine($" Test: {dump(node.Test)}"); + if (node.WhenTrue != null) + { + result.AppendLine($" WhenTrue: {stateIdentifierMap[node.WhenTrue]}"); + } + + if (node.WhenFalse != null) + { + result.AppendLine($" WhenFalse: {stateIdentifierMap[node.WhenFalse]}"); + } + break; + case BoundEvaluationDecisionDagNode node: + result.AppendLine($" Test: {dump(node.Evaluation)}"); + if (node.Next != null) + { + result.AppendLine($" Next: {stateIdentifierMap[node.Next]}"); + } + break; + case BoundWhenDecisionDagNode node: + result.AppendLine($" WhenClause: " + node.WhenExpression.Syntax); + if (node.WhenTrue != null) + { + result.AppendLine($" WhenTrue: {stateIdentifierMap[node.WhenTrue]}"); + } + + if (node.WhenFalse != null) + { + result.AppendLine($" WhenFalse: {stateIdentifierMap[node.WhenFalse]}"); + } + break; + case BoundLeafDecisionDagNode node: + result.AppendLine($" Case: " + node.Syntax); + break; + } + } + + stateIdentifierMap.Free(); + tempIdentifierMap.Free(); + return resultBuilder.ToStringAndFree(); + + string dump(BoundDagTest d) + { + switch (d) + { + case BoundDagTypeEvaluation a: + return $"t{tempIdentifier(a)}={a.Kind}({a.Type.ToString()})"; + case BoundDagEvaluation e: + return $"t{tempIdentifier(e)}={e.Kind}"; + case BoundDagTypeTest b: + return $"?{d.Kind}({b.Type.ToString()}, {tempName(d.Input)})"; + case BoundDagValueTest v: + return $"?{d.Kind}({v.Value.ToString()}, {tempName(d.Input)})"; + default: + return $"?{d.Kind}({tempName(d.Input)})"; + } + } + } +#endif } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs index b9fd4a72ed06d2f5e87112283d04645cec4efe8c..0189fdf8946aaa6092cac9273f5e6718bfb8c94b 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs @@ -145,6 +145,18 @@ public bool MightAssignSomething(BoundExpression expr) return this._mightAssignSomething; } + public override BoundNode VisitReturnStatement(BoundReturnStatement node) + { + if (node.RefKind != RefKind.None) + { + // might be taking a ref to a variable in a lambda that the caller assigns. + this._mightAssignSomething = true; + return null; + } + + return base.VisitReturnStatement(node); + } + public override BoundNode VisitCall(BoundCall node) { _mightAssignSomething = @@ -173,12 +185,43 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct return null; } + public override BoundNode VisitDynamicInvocation(BoundDynamicInvocation node) + { + // perhaps we are passing a variable by ref and mutating it that way + _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; + return base.VisitDynamicInvocation(node); + } + public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpression node) { // perhaps we are passing a variable by ref and mutating it that way _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; return base.VisitObjectCreationExpression(node); } + + public override BoundNode VisitDynamicObjectCreationExpression(BoundDynamicObjectCreationExpression node) + { + _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; + return base.VisitDynamicObjectCreationExpression(node); + } + + public override BoundNode VisitObjectInitializerMember(BoundObjectInitializerMember node) + { + _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; + return base.VisitObjectInitializerMember(node); + } + + public override BoundNode VisitIndexerAccess(BoundIndexerAccess node) + { + _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; + return base.VisitIndexerAccess(node); + } + + public override BoundNode VisitDynamicIndexerAccess(BoundDynamicIndexerAccess node) + { + _mightAssignSomething = !node.ArgumentRefKindsOpt.IsDefault; + return base.VisitDynamicIndexerAccess(node); + } } protected BoundDecisionDag ShareTempsIfPossibleAndEvaluateInput( BoundDecisionDag decisionDag, @@ -398,11 +441,6 @@ private bool GenerateSwitchDispatch(BoundDecisionDagNode node, HashSet !usesOriginalInput(n)) && - false) + if (loweredInput.Type.IsTupleType && + loweredInput.Syntax.Kind() == SyntaxKind.TupleExpression && + loweredInput is BoundObjectCreationExpression expr && + !decisionDag.TopologicallySortedNodes.Any(n => usesOriginalInput(n))) { - // If the switch governing expression is a tuple literal that is not used anywhere, + // If the switch governing expression is a tuple literal whose whole value is not used anywhere, // (though perhaps its component parts are used), then we can save the component parts // and assign them into temps (or perhaps user variables) to avoid the creation of // the tuple altogether. - decisionDag = RewriteTupleInput(decisionDag, (BoundTupleLiteral)loweredInput, addCode); + decisionDag = RewriteTupleInput(decisionDag, expr, addCode); } else { @@ -418,10 +438,77 @@ bool usesOriginalInput(BoundDecisionDagNode node) /// A new decision dag that does not reference the input directly private BoundDecisionDag RewriteTupleInput( BoundDecisionDag decisionDag, - BoundTupleLiteral loweredInput, + BoundObjectCreationExpression loweredInput, Action addCode) { - throw new NotImplementedException(); + int count = loweredInput.Arguments.Length; + var tupleElementEvaluated = new bool[count]; + var rewrittenDag = decisionDag.Rewrite(makeReplacement); + + // If any remaining input elements remain unevaluated, evaluate them now + var originalInput = BoundDagTemp.ForOriginalInput(loweredInput.Syntax, loweredInput.Type); + for (int i = 0; i < count; i++) + { + if (!tupleElementEvaluated[i]) + { + var expr = loweredInput.Arguments[i]; + var field = loweredInput.Type.TupleElements[i].CorrespondingTupleField; + Debug.Assert(field != null); + var fieldFetchEvaluation = new BoundDagFieldEvaluation(expr.Syntax, field, originalInput); + var temp = new BoundDagTemp(expr.Syntax, expr.Type, fieldFetchEvaluation, 0); + storeToTemp(temp, expr); + } + } + + return rewrittenDag; + + void storeToTemp(BoundDagTemp temp, BoundExpression expr) + { + if ((expr.Kind == BoundKind.Parameter || expr.Kind == BoundKind.Local) && _tempAllocator.TrySetTemp(temp, expr)) + { + // we've arranged to use the input value from the variable it is already stored in + } + else + { + var tempToHoldInput = _tempAllocator.GetTemp(temp); + addCode(_factory.AssignmentExpression(tempToHoldInput, expr)); + } + } + + BoundDecisionDagNode makeReplacement(BoundDecisionDagNode node, Func replacement) + { + switch (node) + { + case BoundEvaluationDecisionDagNode evalNode: + if (evalNode.Evaluation is BoundDagFieldEvaluation eval && + eval.Input.IsOriginalInput && + eval.Field is var field && + field.IsTupleField && + field.CorrespondingTupleField != null && + field.TupleElementIndex is int i) + { + if (!tupleElementEvaluated[i]) + { + // Store the value in the right temp + var temp = new BoundDagTemp(eval.Syntax, field.Type, eval, 0); + BoundExpression expr = loweredInput.Arguments[i]; + storeToTemp(temp, expr); + tupleElementEvaluated[i] = true; + } + + return replacement(evalNode.Next); + } + + Debug.Assert(!evalNode.Evaluation.Input.IsOriginalInput); + break; + + case BoundTestDecisionDagNode testNode: + Debug.Assert(!testNode.Test.Input.IsOriginalInput); + break; + } + + return BoundDecisionDag.TrivialReplacement(node, replacement); + } } } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs index 36a474044eb6b6ba201af96a17a6fbed3d9b341a..0168f1a730fc783f07a9e1c3b1b8e8c097cb545c 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs @@ -1121,7 +1121,9 @@ .maxstack 2 }"); } - [Fact, WorkItem(20641, "https://github.com/dotnet/roslyn/issues/20641")] + [Fact] + [WorkItem(20641, "https://github.com/dotnet/roslyn/issues/20641")] + [WorkItem(1395, "https://github.com/dotnet/csharplang/issues/1395")] public void TupleSwitch01() { var source = @"using System; @@ -1134,13 +1136,20 @@ public enum DoorState { Opened, Closed, Locked } public enum Action { Open, Close, Lock, Unlock } - public void Act(Action action, bool haveKey = false) + public void Act0(Action action, bool haveKey = false) { Console.Write($""{State} {action}{(haveKey ? "" withKey"" : null)}""); State = ChangeState0(State, action, haveKey); Console.WriteLine($"" -> {State}""); } + public void Act1(Action action, bool haveKey = false) + { + Console.Write($""{State} {action}{(haveKey ? "" withKey"" : null)}""); + State = ChangeState1(State, action, haveKey); + Console.WriteLine($"" -> {State}""); + } + public static DoorState ChangeState0(DoorState state, Action action, bool haveKey = false) { switch ((state, action)) @@ -1172,13 +1181,23 @@ class Program static void Main(string[] args) { var door = new Door(); - door.Act(Door.Action.Close); - door.Act(Door.Action.Lock); - door.Act(Door.Action.Lock, true); - door.Act(Door.Action.Open); - door.Act(Door.Action.Unlock); - door.Act(Door.Action.Unlock, true); - door.Act(Door.Action.Open); + door.Act0(Door.Action.Close); + door.Act0(Door.Action.Lock); + door.Act0(Door.Action.Lock, true); + door.Act0(Door.Action.Open); + door.Act0(Door.Action.Unlock); + door.Act0(Door.Action.Unlock, true); + door.Act0(Door.Action.Open); + Console.WriteLine(); + + door = new Door(); + door.Act1(Door.Action.Close); + door.Act1(Door.Action.Lock); + door.Act1(Door.Action.Lock, true); + door.Act1(Door.Action.Open); + door.Act1(Door.Action.Unlock); + door.Act1(Door.Action.Unlock, true); + door.Act1(Door.Action.Open); } }"; var expectedOutput = @@ -1189,132 +1208,105 @@ static void Main(string[] args) Locked Unlock -> Locked Locked Unlock withKey -> Closed Closed Open -> Opened + +Opened Close -> Closed +Closed Lock -> Closed +Closed Lock withKey -> Locked +Locked Open -> Locked +Locked Unlock -> Locked +Locked Unlock withKey -> Closed +Closed Open -> Opened "; var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe, parseOptions: TestOptions.Regular.WithRecursivePatterns()); compilation.VerifyDiagnostics(); var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput); compVerifier.VerifyIL("Door.ChangeState0", @"{ - // Code size 94 (0x5e) - .maxstack 3 - .locals init (Door.DoorState V_0, //oldState - System.ValueTuple V_1, - Door.Action V_2) - IL_0000: ldloca.s V_1 - IL_0002: ldarg.0 - IL_0003: ldarg.1 - IL_0004: call ""System.ValueTuple..ctor(Door.DoorState, Door.Action)"" - IL_0009: ldloc.1 - IL_000a: ldfld ""Door.DoorState System.ValueTuple.Item1"" - IL_000f: stloc.0 - IL_0010: ldloc.0 - IL_0011: switch ( - IL_0024, - IL_0031, - IL_0041) - IL_0022: br.s IL_005c - IL_0024: ldloc.1 - IL_0025: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_002a: stloc.2 + // Code size 59 (0x3b) + .maxstack 2 + .locals init (Door.DoorState V_0) //oldState + IL_0000: ldarg.0 + IL_0001: stloc.0 + IL_0002: ldloc.0 + IL_0003: switch ( + IL_0016, + IL_001c, + IL_0025) + IL_0014: br.s IL_0039 + IL_0016: ldc.i4.1 + IL_0017: ldarg.1 + IL_0018: beq.s IL_002b + IL_001a: br.s IL_0039 + IL_001c: ldarg.1 + IL_001d: brfalse.s IL_002d + IL_001f: ldarg.1 + IL_0020: ldc.i4.2 + IL_0021: beq.s IL_002f + IL_0023: br.s IL_0039 + IL_0025: ldc.i4.3 + IL_0026: ldarg.1 + IL_0027: beq.s IL_0034 + IL_0029: br.s IL_0039 IL_002b: ldc.i4.1 - IL_002c: ldloc.2 - IL_002d: beq.s IL_004e - IL_002f: br.s IL_005c - IL_0031: ldloc.1 - IL_0032: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_0037: stloc.2 - IL_0038: ldloc.2 - IL_0039: brfalse.s IL_0050 - IL_003b: ldloc.2 - IL_003c: ldc.i4.2 - IL_003d: beq.s IL_0052 - IL_003f: br.s IL_005c - IL_0041: ldloc.1 - IL_0042: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_0047: stloc.2 - IL_0048: ldc.i4.3 - IL_0049: ldloc.2 - IL_004a: beq.s IL_0057 - IL_004c: br.s IL_005c - IL_004e: ldc.i4.1 - IL_004f: ret - IL_0050: ldc.i4.0 - IL_0051: ret - IL_0052: ldarg.2 - IL_0053: brfalse.s IL_005c - IL_0055: ldc.i4.2 - IL_0056: ret - IL_0057: ldarg.2 - IL_0058: brfalse.s IL_005c - IL_005a: ldc.i4.1 - IL_005b: ret - IL_005c: ldloc.0 - IL_005d: ret + IL_002c: ret + IL_002d: ldc.i4.0 + IL_002e: ret + IL_002f: ldarg.2 + IL_0030: brfalse.s IL_0039 + IL_0032: ldc.i4.2 + IL_0033: ret + IL_0034: ldarg.2 + IL_0035: brfalse.s IL_0039 + IL_0037: ldc.i4.1 + IL_0038: ret + IL_0039: ldloc.0 + IL_003a: ret }"); compVerifier.VerifyIL("Door.ChangeState1", @"{ - // Code size 104 (0x68) - .maxstack 3 - .locals init (System.ValueTuple V_0, - Door.DoorState V_1, - Door.Action V_2, - Door.DoorState V_3) - IL_0000: ldloca.s V_0 - IL_0002: ldarg.0 - IL_0003: ldarg.1 - IL_0004: call ""System.ValueTuple..ctor(Door.DoorState, Door.Action)"" - IL_0009: ldloc.0 - IL_000a: ldfld ""Door.DoorState System.ValueTuple.Item1"" - IL_000f: stloc.1 - IL_0010: ldloc.1 - IL_0011: switch ( - IL_0024, - IL_0031, - IL_0041) - IL_0022: br.s IL_0064 - IL_0024: ldloc.0 - IL_0025: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_002a: stloc.2 - IL_002b: ldc.i4.1 - IL_002c: ldloc.2 - IL_002d: beq.s IL_004e - IL_002f: br.s IL_0064 - IL_0031: ldloc.0 - IL_0032: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_0037: stloc.2 - IL_0038: ldloc.2 - IL_0039: brfalse.s IL_0052 - IL_003b: ldloc.2 - IL_003c: ldc.i4.2 - IL_003d: beq.s IL_0056 - IL_003f: br.s IL_0064 + // Code size 67 (0x43) + .maxstack 2 + .locals init (Door.DoorState V_0) + IL_0000: ldarg.0 + IL_0001: switch ( + IL_0014, + IL_001a, + IL_0023) + IL_0012: br.s IL_003f + IL_0014: ldc.i4.1 + IL_0015: ldarg.1 + IL_0016: beq.s IL_0029 + IL_0018: br.s IL_003f + IL_001a: ldarg.1 + IL_001b: brfalse.s IL_002d + IL_001d: ldarg.1 + IL_001e: ldc.i4.2 + IL_001f: beq.s IL_0031 + IL_0021: br.s IL_003f + IL_0023: ldc.i4.3 + IL_0024: ldarg.1 + IL_0025: beq.s IL_0038 + IL_0027: br.s IL_003f + IL_0029: ldc.i4.1 + IL_002a: stloc.0 + IL_002b: br.s IL_0041 + IL_002d: ldc.i4.0 + IL_002e: stloc.0 + IL_002f: br.s IL_0041 + IL_0031: ldarg.2 + IL_0032: brfalse.s IL_003f + IL_0034: ldc.i4.2 + IL_0035: stloc.0 + IL_0036: br.s IL_0041 + IL_0038: ldarg.2 + IL_0039: brfalse.s IL_003f + IL_003b: ldc.i4.1 + IL_003c: stloc.0 + IL_003d: br.s IL_0041 + IL_003f: ldarg.0 + IL_0040: stloc.0 IL_0041: ldloc.0 - IL_0042: ldfld ""Door.Action System.ValueTuple.Item2"" - IL_0047: stloc.2 - IL_0048: ldc.i4.3 - IL_0049: ldloc.2 - IL_004a: beq.s IL_005d - IL_004c: br.s IL_0064 - IL_004e: ldc.i4.1 - IL_004f: stloc.3 - IL_0050: br.s IL_0066 - IL_0052: ldc.i4.0 - IL_0053: stloc.3 - IL_0054: br.s IL_0066 - IL_0056: ldarg.2 - IL_0057: brfalse.s IL_0064 - IL_0059: ldc.i4.2 - IL_005a: stloc.3 - IL_005b: br.s IL_0066 - IL_005d: ldarg.2 - IL_005e: brfalse.s IL_0064 - IL_0060: ldc.i4.1 - IL_0061: stloc.3 - IL_0062: br.s IL_0066 - IL_0064: ldarg.0 - IL_0065: stloc.3 - IL_0066: ldloc.3 - IL_0067: ret + IL_0042: ret }"); } }