Skip to content

Commit c229bc4

Browse files
authored
List patterns: Recreate the decision dag for lowering (#59019)
1 parent ccf44fc commit c229bc4

20 files changed

+420
-116
lines changed

src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ private BoundExpression ConvertSwitchExpression(BoundUnconvertedSwitchExpression
461461

462462
var newSwitchArms = builder.ToImmutableAndFree();
463463
return new BoundConvertedSwitchExpression(
464-
source.Syntax, source.Type, targetTyped, source.Expression, newSwitchArms, source.DecisionDag,
464+
source.Syntax, source.Type, targetTyped, source.Expression, newSwitchArms, source.ReachabilityDecisionDag,
465465
source.DefaultLabel, source.ReportedNotExhaustive, destination, hasErrors || source.HasErrors).WithSuppression(source.IsSuppressed);
466466
}
467467

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,20 @@ internal sealed partial class DecisionDagBuilder
6060
private readonly Conversions _conversions;
6161
private readonly BindingDiagnosticBag _diagnostics;
6262
private readonly LabelSymbol _defaultLabel;
63+
/// <summary>
64+
/// We might need to build a dedicated dag for lowering during which we
65+
/// avoid synthesizing tests to relate alternative indexers. This won't
66+
/// affect code semantics but it results in a better code generation.
67+
/// </summary>
68+
private readonly bool _forLowering;
6369

64-
private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLabel, BindingDiagnosticBag diagnostics)
70+
private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLabel, bool forLowering, BindingDiagnosticBag diagnostics)
6571
{
6672
this._compilation = compilation;
6773
this._conversions = compilation.Conversions;
6874
_diagnostics = diagnostics;
6975
_defaultLabel = defaultLabel;
76+
_forLowering = forLowering;
7077
}
7178

7279
/// <summary>
@@ -78,9 +85,10 @@ public static BoundDecisionDag CreateDecisionDagForSwitchStatement(
7885
BoundExpression switchGoverningExpression,
7986
ImmutableArray<BoundSwitchSection> switchSections,
8087
LabelSymbol defaultLabel,
81-
BindingDiagnosticBag diagnostics)
88+
BindingDiagnosticBag diagnostics,
89+
bool forLowering = false)
8290
{
83-
var builder = new DecisionDagBuilder(compilation, defaultLabel, diagnostics);
91+
var builder = new DecisionDagBuilder(compilation, defaultLabel, forLowering, diagnostics);
8492
return builder.CreateDecisionDagForSwitchStatement(syntax, switchGoverningExpression, switchSections);
8593
}
8694

@@ -93,9 +101,10 @@ public static BoundDecisionDag CreateDecisionDagForSwitchExpression(
93101
BoundExpression switchExpressionInput,
94102
ImmutableArray<BoundSwitchExpressionArm> switchArms,
95103
LabelSymbol defaultLabel,
96-
BindingDiagnosticBag diagnostics)
104+
BindingDiagnosticBag diagnostics,
105+
bool forLowering = false)
97106
{
98-
var builder = new DecisionDagBuilder(compilation, defaultLabel, diagnostics);
107+
var builder = new DecisionDagBuilder(compilation, defaultLabel, forLowering, diagnostics);
99108
return builder.CreateDecisionDagForSwitchExpression(syntax, switchExpressionInput, switchArms);
100109
}
101110

@@ -109,9 +118,10 @@ public static BoundDecisionDag CreateDecisionDagForIsPattern(
109118
BoundPattern pattern,
110119
LabelSymbol whenTrueLabel,
111120
LabelSymbol whenFalseLabel,
112-
BindingDiagnosticBag diagnostics)
121+
BindingDiagnosticBag diagnostics,
122+
bool forLowering = false)
113123
{
114-
var builder = new DecisionDagBuilder(compilation, defaultLabel: whenFalseLabel, diagnostics);
124+
var builder = new DecisionDagBuilder(compilation, defaultLabel: whenFalseLabel, forLowering, diagnostics);
115125
return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, whenTrueLabel);
116126
}
117127

@@ -1340,7 +1350,7 @@ void handleRelationWithValue(
13401350
/// <summary>Returns true if the tests are related i.e. they have the same input, otherwise false.</summary>
13411351
/// <param name="relationCondition">The pre-condition under which these tests are related.</param>
13421352
/// <param name="relationEffect">A possible assignment node which will correspond two non-identical but related test inputs.</param>
1343-
private static bool CheckInputRelation(
1353+
private bool CheckInputRelation(
13441354
SyntaxNode syntax,
13451355
DagState state,
13461356
BoundDagTest test,
@@ -1430,7 +1440,7 @@ other is not (BoundDagNonNullTest or BoundDagExplicitNullTest) &&
14301440
continue;
14311441
}
14321442

1433-
if (lengthValues.Any(BinaryOperatorKind.Equal, lengthValue))
1443+
if (!_forLowering && lengthValues.Any(BinaryOperatorKind.Equal, lengthValue))
14341444
{
14351445
// Otherwise, we add a test to make the result conditional on the length value.
14361446
(conditions ??= ArrayBuilder<Tests>.GetInstance()).Add(new Tests.One(new BoundDagValueTest(syntax, ConstantValue.Create(lengthValue), s1LengthTemp)));
@@ -1975,7 +1985,7 @@ public override void Filter(
19751985
SyntaxNode syntax = test.Syntax;
19761986
BoundDagTest other = this.Test;
19771987
if (other is BoundDagEvaluation ||
1978-
!CheckInputRelation(syntax, state, test, other,
1988+
!builder.CheckInputRelation(syntax, state, test, other,
19791989
relationCondition: out Tests relationCondition,
19801990
relationEffect: out Tests relationEffect))
19811991
{

src/Compilers/CSharp/Portable/Binder/SwitchBinder_Patterns.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ internal override BoundStatement BindSwitchStatementCore(SwitchStatementSyntax n
6666
switchSections: switchSections,
6767
defaultLabel: defaultLabel,
6868
breakLabel: this.BreakLabel,
69-
decisionDag: decisionDag);
69+
reachabilityDecisionDag: decisionDag);
7070
}
7171

7272
private void CheckSwitchErrors(

src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ BoundDecisionDagNode makeReplacement(BoundDecisionDagNode dag, IReadOnlyDictiona
194194
}
195195
}
196196

197+
public bool ContainsAnySynthesizedNodes()
198+
{
199+
return this.TopologicallySortedNodes.Any(node => node is BoundEvaluationDecisionDagNode e && e.Evaluation.Kind == BoundKind.DagAssignmentEvaluation);
200+
}
201+
197202
#if DEBUG
198203
/// <summary>
199204
/// Starting with `this` state, produce a human-readable description of the state tables.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
7+
namespace Microsoft.CodeAnalysis.CSharp
8+
{
9+
internal partial class BoundIsPatternExpression
10+
{
11+
public BoundDecisionDag GetDecisionDagForLowering(CSharpCompilation compilation)
12+
{
13+
BoundDecisionDag decisionDag = this.ReachabilityDecisionDag;
14+
if (decisionDag.ContainsAnySynthesizedNodes())
15+
{
16+
decisionDag = DecisionDagBuilder.CreateDecisionDagForIsPattern(
17+
compilation,
18+
this.Syntax,
19+
this.Expression,
20+
this.Pattern,
21+
this.WhenTrueLabel,
22+
this.WhenFalseLabel,
23+
BindingDiagnosticBag.Discarded,
24+
forLowering: true);
25+
Debug.Assert(!decisionDag.ContainsAnySynthesizedNodes());
26+
}
27+
28+
return decisionDag;
29+
}
30+
}
31+
}

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,7 +1088,7 @@
10881088
<Field Name="InnerLocals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
10891089
<Field Name="InnerLocalFunctions" Type="ImmutableArray&lt;LocalFunctionSymbol&gt;"/>
10901090
<Field Name="SwitchSections" Type="ImmutableArray&lt;BoundSwitchSection&gt;"/>
1091-
<Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
1091+
<Field Name="ReachabilityDecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
10921092
<Field Name="DefaultLabel" Type="BoundSwitchLabel?"/>
10931093
<Field Name="BreakLabel" Type="GeneratedLabelSymbol"/>
10941094
</Node>
@@ -1411,7 +1411,7 @@
14111411
<AbstractNode Name="BoundSwitchExpression" Base="BoundExpression">
14121412
<Field Name="Expression" Type="BoundExpression" Null="disallow"/>
14131413
<Field Name="SwitchArms" Type="ImmutableArray&lt;BoundSwitchExpressionArm&gt;"/>
1414-
<Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
1414+
<Field Name="ReachabilityDecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
14151415
<Field Name="DefaultLabel" Type="LabelSymbol?"/>
14161416
<Field Name="ReportedNotExhaustive" Type="bool"/>
14171417
</AbstractNode>
@@ -2200,7 +2200,7 @@
22002200
<Field Name="Expression" Type="BoundExpression" Null="disallow"/>
22012201
<Field Name="Pattern" Type="BoundPattern" Null="disallow"/>
22022202
<Field Name="IsNegated" Type="bool"/>
2203-
<Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
2203+
<Field Name="ReachabilityDecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
22042204
<Field Name="WhenTrueLabel" Type="LabelSymbol" Null="disallow"/>
22052205
<Field Name="WhenFalseLabel" Type="LabelSymbol" Null="disallow"/>
22062206
</Node>
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
using Microsoft.CodeAnalysis.CSharp.Symbols;
7+
8+
namespace Microsoft.CodeAnalysis.CSharp
9+
{
10+
internal partial class BoundSwitchExpression
11+
{
12+
public BoundDecisionDag GetDecisionDagForLowering(CSharpCompilation compilation, out LabelSymbol? defaultLabel)
13+
{
14+
defaultLabel = this.DefaultLabel;
15+
16+
BoundDecisionDag decisionDag = this.ReachabilityDecisionDag;
17+
if (decisionDag.ContainsAnySynthesizedNodes())
18+
{
19+
decisionDag = DecisionDagBuilder.CreateDecisionDagForSwitchExpression(
20+
compilation,
21+
this.Syntax,
22+
this.Expression,
23+
this.SwitchArms,
24+
// there's no default label if the original switch is exhaustive.
25+
// we generate a new label here because the new dag might not be.
26+
defaultLabel ??= new GeneratedLabelSymbol("default"),
27+
BindingDiagnosticBag.Discarded,
28+
forLowering: true);
29+
Debug.Assert(!decisionDag.ContainsAnySynthesizedNodes());
30+
}
31+
32+
return decisionDag;
33+
}
34+
}
35+
}

src/Compilers/CSharp/Portable/BoundTree/BoundSwitchStatement.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,32 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections.Immutable;
6+
using System.Diagnostics;
67

78
namespace Microsoft.CodeAnalysis.CSharp
89
{
910
internal partial class BoundSwitchStatement : IBoundSwitchStatement
1011
{
1112
BoundNode IBoundSwitchStatement.Value => this.Expression;
1213
ImmutableArray<BoundStatementList> IBoundSwitchStatement.Cases => StaticCast<BoundStatementList>.From(this.SwitchSections);
14+
15+
public BoundDecisionDag GetDecisionDagForLowering(CSharpCompilation compilation)
16+
{
17+
BoundDecisionDag decisionDag = this.ReachabilityDecisionDag;
18+
if (decisionDag.ContainsAnySynthesizedNodes())
19+
{
20+
decisionDag = DecisionDagBuilder.CreateDecisionDagForSwitchStatement(
21+
compilation,
22+
this.Syntax,
23+
this.Expression,
24+
this.SwitchSections,
25+
this.DefaultLabel?.Label ?? this.BreakLabel,
26+
BindingDiagnosticBag.Discarded,
27+
forLowering: true);
28+
Debug.Assert(!decisionDag.ContainsAnySynthesizedNodes());
29+
}
30+
31+
return decisionDag;
32+
}
1333
}
1434
}

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
947947
}
948948

949949
VisitPattern(pattern);
950-
var reachableLabels = node.DecisionDag.ReachableLabels;
950+
var reachableLabels = node.ReachabilityDecisionDag.ReachableLabels;
951951
if (!reachableLabels.Contains(node.WhenTrueLabel))
952952
{
953953
SetState(this.StateWhenFalse);

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass_Switch.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected virtual TLocalState VisitSwitchStatementDispatch(BoundSwitchStatement
4343

4444
TLocalState initialState = this.State.Clone();
4545

46-
var reachableLabels = node.DecisionDag.ReachableLabels;
46+
var reachableLabels = node.ReachabilityDecisionDag.ReachableLabels;
4747
foreach (var section in node.SwitchSections)
4848
{
4949
foreach (var label in section.SwitchLabels)
@@ -71,7 +71,7 @@ protected virtual TLocalState VisitSwitchStatementDispatch(BoundSwitchStatement
7171
}
7272

7373
TLocalState afterSwitchState = UnreachableState();
74-
if (node.DecisionDag.ReachableLabels.Contains(node.BreakLabel) ||
74+
if (node.ReachabilityDecisionDag.ReachableLabels.Contains(node.BreakLabel) ||
7575
(node.DefaultLabel == null && node.Expression.ConstantValue == null && IsTraditionalSwitch(node)))
7676
{
7777
Join(ref afterSwitchState, ref initialState);
@@ -157,7 +157,7 @@ private BoundNode VisitSwitchExpression(BoundSwitchExpression node)
157157
VisitRvalue(node.Expression);
158158
var dispatchState = this.State;
159159
var endState = UnreachableState();
160-
var reachableLabels = node.DecisionDag.ReachableLabels;
160+
var reachableLabels = node.ReachabilityDecisionDag.ReachableLabels;
161161
foreach (var arm in node.SwitchArms)
162162
{
163163
SetState(dispatchState.Clone());

0 commit comments

Comments
 (0)