Skip to content

Commit bd2d261

Browse files
stereotype441Commit Bot
authored and
Commit Bot
committed
Shared type analysis: add support for when clauses and fix label support.
Support for `when` clauses requires flow analysis integration, so that `when` clauses can promote variables, e.g.: f(int x, String? y) { switch (x) { case 0 when y != null: // y is known to be non-null here } } Support for labels in switch statements had a small flaw: we weren't reporting an error in the case where a label shared a case body with a pattern that tried to bind a variable, e.g.: f(int x) { switch (x) { L: // Error: does not mind the variable `y` case var y: ... } } Change-Id: I0b2bb4721a6b3a8f7898df682b24b75ddb6e44ae Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256605 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent ac86c13 commit bd2d261

File tree

6 files changed

+452
-148
lines changed

6 files changed

+452
-148
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,17 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
530530
@visibleForTesting
531531
SsaNode<Type>? ssaNodeForTesting(Variable variable);
532532

533+
/// Call this method just after visiting a `when` part of a case clause. See
534+
/// [switchStatement_expressionEnd] for details.
535+
///
536+
/// [when] should be the expression following the `when` keyword.
537+
void switchStatement_afterWhen(Expression when);
538+
539+
/// Call this method just before visiting a sequence of two or more `case` or
540+
/// `default` clauses that share a body. See [switchStatement_expressionEnd]
541+
/// for details.`
542+
void switchStatement_beginAlternatives();
543+
533544
/// Call this method just before visiting one of the cases in the body of a
534545
/// switch statement. See [switchStatement_expressionEnd] for details.
535546
///
@@ -547,16 +558,35 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
547558
/// were listed in cases.
548559
void switchStatement_end(bool isExhaustive);
549560

561+
/// Call this method just after visiting a `case` or `default` clause, if it
562+
/// shares a body with at least one other `case` or `default` clause. See
563+
/// [switchStatement_expressionEnd] for details.`
564+
void switchStatement_endAlternative();
565+
566+
/// Call this method just after visiting a sequence of two or more `case` or
567+
/// `default` clauses that share a body. See [switchStatement_expressionEnd]
568+
/// for details.`
569+
void switchStatement_endAlternatives();
570+
550571
/// Call this method just after visiting the expression part of a switch
551572
/// statement or expression. [switchStatement] should be the switch statement
552573
/// itself (or `null` if this is a switch expression).
553574
///
554575
/// The order of visiting a switch statement should be:
555576
/// - Visit the switch expression.
556577
/// - Call [switchStatement_expressionEnd].
557-
/// - For each switch case (including the default case, if any):
578+
/// - For each case body:
558579
/// - Call [switchStatement_beginCase].
559-
/// - Visit the case.
580+
/// - If there is more than one `case` or `default` clause associated with
581+
/// this case body, call [switchStatement_beginAlternatives].
582+
/// - For each `case` or `default` clause associated with this case body:
583+
/// - If a `when` clause is present, visit it and then call
584+
/// [switchStatement_afterWhen].
585+
/// - If there is more than one `case` or `default` clause associated with
586+
/// this case body, call [switchStatement_endAlternative].
587+
/// - If there is more than one `case` or `default` clause associated with
588+
/// this case body, call [switchStatement_endAlternatives].
589+
/// - Visit the case body.
560590
/// - Call [switchStatement_end].
561591
void switchStatement_expressionEnd(Statement? switchStatement);
562592

@@ -1172,6 +1202,18 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
11721202
isQuery: true);
11731203
}
11741204

1205+
@override
1206+
void switchStatement_afterWhen(Expression when) {
1207+
_wrap('switchStatement_afterWhen($when)',
1208+
() => _wrapped.switchStatement_afterWhen(when));
1209+
}
1210+
1211+
@override
1212+
void switchStatement_beginAlternatives() {
1213+
_wrap('switchStatement_beginAlternatives()',
1214+
() => _wrapped.switchStatement_beginAlternatives());
1215+
}
1216+
11751217
@override
11761218
void switchStatement_beginCase(bool hasLabel, Statement? node) {
11771219
_wrap('switchStatement_beginCase($hasLabel, $node)',
@@ -1184,6 +1226,18 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
11841226
() => _wrapped.switchStatement_end(isExhaustive));
11851227
}
11861228

1229+
@override
1230+
void switchStatement_endAlternative() {
1231+
_wrap('switchStatement_endAlternative()',
1232+
() => _wrapped.switchStatement_endAlternative());
1233+
}
1234+
1235+
@override
1236+
void switchStatement_endAlternatives() {
1237+
_wrap('switchStatement_endAlternatives()',
1238+
() => _wrapped.switchStatement_endAlternatives());
1239+
}
1240+
11871241
@override
11881242
void switchStatement_expressionEnd(Statement? switchStatement) {
11891243
_wrap('switchStatement_expressionEnd($switchStatement)',
@@ -3685,6 +3739,22 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
36853739
SsaNode<Type>? ssaNodeForTesting(Variable variable) => _current
36863740
.variableInfo[promotionKeyStore.keyForVariable(variable)]?.ssaNode;
36873741

3742+
@override
3743+
void switchStatement_afterWhen(Expression when) {
3744+
ExpressionInfo<Type>? expressionInfo = _getExpressionInfo(when);
3745+
if (expressionInfo != null) {
3746+
_current = expressionInfo.ifTrue;
3747+
}
3748+
}
3749+
3750+
@override
3751+
void switchStatement_beginAlternatives() {
3752+
_current = _current.split();
3753+
_SwitchAlternativesContext<Type> context =
3754+
new _SwitchAlternativesContext<Type>(_current);
3755+
_stack.add(context);
3756+
}
3757+
36883758
@override
36893759
void switchStatement_beginCase(bool hasLabel, Statement? node) {
36903760
_SimpleStatementContext<Type> context =
@@ -3714,6 +3784,21 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
37143784
_current = breakState.unsplit();
37153785
}
37163786

3787+
@override
3788+
void switchStatement_endAlternative() {
3789+
_SwitchAlternativesContext<Type> context =
3790+
_stack.last as _SwitchAlternativesContext<Type>;
3791+
context._combinedModel = _join(context._combinedModel, _current);
3792+
_current = context._previous;
3793+
}
3794+
3795+
@override
3796+
void switchStatement_endAlternatives() {
3797+
_SwitchAlternativesContext<Type> context =
3798+
_stack.removeLast() as _SwitchAlternativesContext<Type>;
3799+
_current = context._combinedModel!.unsplit();
3800+
}
3801+
37173802
@override
37183803
void switchStatement_expressionEnd(Statement? switchStatement) {
37193804
_current = _current.split();
@@ -4514,12 +4599,24 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
45144599
throw new StateError('ssaNodeForTesting requires null-aware flow analysis');
45154600
}
45164601

4602+
@override
4603+
void switchStatement_afterWhen(Expression when) {}
4604+
4605+
@override
4606+
void switchStatement_beginAlternatives() {}
4607+
45174608
@override
45184609
void switchStatement_beginCase(bool hasLabel, Statement? node) {}
45194610

45204611
@override
45214612
void switchStatement_end(bool isExhaustive) {}
45224613

4614+
@override
4615+
void switchStatement_endAlternative() {}
4616+
4617+
@override
4618+
void switchStatement_endAlternatives() {}
4619+
45234620
@override
45244621
void switchStatement_expressionEnd(Statement? switchStatement) {}
45254622

@@ -4767,6 +4864,14 @@ class _SimpleStatementContext<Type extends Object>
47674864
'checkpoint: $_checkpoint)';
47684865
}
47694866

4867+
class _SwitchAlternativesContext<Type extends Object> extends _FlowContext {
4868+
final FlowModel<Type> _previous;
4869+
4870+
FlowModel<Type>? _combinedModel;
4871+
4872+
_SwitchAlternativesContext(this._previous);
4873+
}
4874+
47704875
/// Specialization of [ExpressionInfo] for the case where the information we
47714876
/// have about the expression is trivial (meaning we know by construction that
47724877
/// the expression's [after], [ifTrue], and [ifFalse] models are all the same).

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,36 @@ class ExpressionCaseInfo<Expression, Node> {
1414
/// For a `case` clause, the case pattern. For a `default` clause, `null`.
1515
final Node? pattern;
1616

17+
/// For a `case` clause that has a `when` part, the expression following
18+
/// `when`. Otherwise `null`.
19+
final Expression? when;
20+
1721
/// The body of the `case` or `default` clause.
1822
final Expression body;
1923

20-
ExpressionCaseInfo(this.pattern, this.body);
24+
ExpressionCaseInfo({required this.pattern, this.when, required this.body});
2125
}
2226

2327
/// Information supplied by the client to [TypeAnalyzer.analyzeSwitchStatement]
2428
/// about an individual `case` or `default` clause.
2529
///
2630
/// The client is free to `implement` or `extend` this class.
27-
class StatementCaseInfo<Statement, Node> {
31+
class StatementCaseInfo<Statement, Expression, Node> {
2832
/// The AST node for this `case` or `default` clause. This is used for error
2933
/// reporting, in case errors arise from mismatch among the variables bound by
3034
/// various cases that share a body.
3135
final Node node;
3236

33-
/// Indicates whether this `case` or `default` clause is preceded by one or
34-
/// more `goto` labels.
35-
final bool hasLabel;
37+
/// The labels preceding this `case` or `default` clause, if any.
38+
final List<Node> labels;
3639

3740
/// For a `case` clause, the case pattern. For a `default` clause, `null`.
3841
final Node? pattern;
3942

43+
/// For a `case` clause that has a `when` part, the expression following
44+
/// `when`. Otherwise `null`.
45+
final Expression? when;
46+
4047
/// The statements following this `case` or `default` clause. If this list is
4148
/// empty, and this is not the last `case` or `default` clause, this clause
4249
/// will be considered to share a body with the `case` or `default` clause
@@ -45,8 +52,9 @@ class StatementCaseInfo<Statement, Node> {
4552

4653
StatementCaseInfo(
4754
{required this.node,
48-
required this.hasLabel,
55+
this.labels = const [],
4956
required this.pattern,
57+
this.when,
5058
required this.body});
5159
}
5260

@@ -70,6 +78,9 @@ class StatementCaseInfo<Statement, Node> {
7078
mixin TypeAnalyzer<Node extends Object, Statement extends Node,
7179
Expression extends Node, Variable extends Object, Type extends Object>
7280
implements VariableBindingCallbacks<Node, Variable, Type> {
81+
/// Returns the type `bool`.
82+
Type get boolType;
83+
7384
/// Returns the type `double`.
7485
Type get doubleType;
7586

@@ -175,6 +186,8 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
175186
/// - [analyzeExpression]
176187
/// - For each `case` or `default` clause:
177188
/// - [dispatchPattern] if this is a `case` clause
189+
/// - [analyzeExpression] if this is a `case` clause with a `when` part
190+
/// - [handleCaseHead] if this is a `case` clause
178191
/// - [handleDefault] if this is a `default` clause
179192
/// - [handleCase_afterCaseHeads]
180193
/// - [analyzeExpression]
@@ -196,10 +209,17 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
196209
if (pattern != null) {
197210
dispatchPattern(pattern)
198211
.match(expressionType, bindings, isFinal: true, isLate: false);
212+
Expression? when = caseInfo.when;
213+
bool hasWhen = when != null;
214+
if (hasWhen) {
215+
analyzeExpression(when, boolType);
216+
flow?.switchStatement_afterWhen(when);
217+
}
218+
handleCaseHead(hasWhen: hasWhen);
199219
} else {
200220
handleDefault();
201221
}
202-
handleCase_afterCaseHeads(1);
222+
handleCase_afterCaseHeads(const [], 1);
203223
Type type = analyzeExpression(caseInfo.body, context);
204224
if (lubType == null) {
205225
lubType = type;
@@ -217,8 +237,11 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
217237
/// Invokes the following [TypeAnalyzer] methods (in order):
218238
/// - [dispatchExpression]
219239
/// - For each `case` or `default` body:
220-
/// - [dispatchPattern] for each `case` pattern associated with the body
221-
/// - [handleDefault] if a `default` clause is associated with the body
240+
/// - For each `case` or `default` clause associated with the body:
241+
/// - [dispatchPattern] if this is a `case` clause
242+
/// - [analyzeExpression] if this is a `case` clause with a `when` part
243+
/// - [handleCaseHead] if this is a `case` clause
244+
/// - [handleDefault] if this is a `default` clause
222245
/// - [handleCase_afterCaseHeads]
223246
/// - [dispatchStatement] for each statement in the body
224247
/// - [finishStatementCase]
@@ -228,42 +251,65 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
228251
/// length of [cases] because a case with no statements get merged into the
229252
/// case that follows).
230253
int analyzeSwitchStatement(Statement node, Expression scrutinee,
231-
List<StatementCaseInfo<Statement, Node>> cases) {
254+
List<StatementCaseInfo<Statement, Expression, Node>> cases) {
232255
Type expressionType = analyzeExpression(scrutinee, unknownType);
233256
flow?.switchStatement_expressionEnd(node);
234-
bool hasLabel = false;
235-
List<StatementCaseInfo<Statement, Node>>? casesInThisExecutionPath;
257+
List<Node> labels = [];
258+
List<StatementCaseInfo<Statement, Expression, Node>>?
259+
casesInThisExecutionPath;
236260
int numExecutionPaths = 0;
237261
for (int i = 0; i < cases.length; i++) {
238-
StatementCaseInfo<Statement, Node> caseInfo = cases[i];
239-
hasLabel = hasLabel || caseInfo.hasLabel;
262+
StatementCaseInfo<Statement, Expression, Node> caseInfo = cases[i];
263+
labels.addAll(caseInfo.labels);
240264
(casesInThisExecutionPath ??= []).add(caseInfo);
241265
if (i == cases.length - 1 || caseInfo.body.isNotEmpty) {
242266
numExecutionPaths++;
243-
flow?.switchStatement_beginCase(hasLabel, node);
267+
flow?.switchStatement_beginCase(labels.isNotEmpty, node);
244268
VariableBindings<Node, Variable, Type> bindings =
245269
new VariableBindings(this);
246270
bindings.startAlternatives();
247-
for (int i = 0; i < casesInThisExecutionPath.length; i++) {
248-
StatementCaseInfo<Statement, Node> caseInfo =
271+
// Labels count as empty patterns for the purposes of bindings.
272+
for (Node label in labels) {
273+
bindings.startAlternative(label);
274+
bindings.finishAlternative();
275+
}
276+
int numCasesInThisExecutionPath = casesInThisExecutionPath.length;
277+
if (numCasesInThisExecutionPath > 1) {
278+
flow?.switchStatement_beginAlternatives();
279+
}
280+
for (int i = 0; i < numCasesInThisExecutionPath; i++) {
281+
StatementCaseInfo<Statement, Expression, Node> caseInfo =
249282
casesInThisExecutionPath[i];
250283
bindings.startAlternative(caseInfo.node);
251284
Node? pattern = caseInfo.pattern;
252285
if (pattern != null) {
253286
dispatchPattern(pattern)
254287
.match(expressionType, bindings, isFinal: true, isLate: false);
288+
Expression? when = caseInfo.when;
289+
bool hasWhen = when != null;
290+
if (hasWhen) {
291+
analyzeExpression(when, boolType);
292+
flow?.switchStatement_afterWhen(when);
293+
}
294+
handleCaseHead(hasWhen: hasWhen);
255295
} else {
256296
handleDefault();
257297
}
258298
bindings.finishAlternative();
299+
if (numCasesInThisExecutionPath > 1) {
300+
flow?.switchStatement_endAlternative();
301+
}
259302
}
260303
bindings.finishAlternatives();
261-
handleCase_afterCaseHeads(casesInThisExecutionPath.length);
304+
if (numCasesInThisExecutionPath > 1) {
305+
flow?.switchStatement_endAlternatives();
306+
}
307+
handleCase_afterCaseHeads(labels, numCasesInThisExecutionPath);
262308
for (Statement statement in caseInfo.body) {
263309
dispatchStatement(statement);
264310
}
265311
finishStatementCase(node, i, caseInfo.body.length);
266-
hasLabel = false;
312+
labels.clear();
267313
casesInThisExecutionPath = null;
268314
}
269315
}
@@ -334,7 +380,10 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
334380
void finishStatementCase(Statement node, int caseIndex, int numStatements);
335381

336382
/// See [analyzeSwitchStatement] and [analyzeSwitchExpression].
337-
void handleCase_afterCaseHeads(int numHeads);
383+
void handleCase_afterCaseHeads(List<Node> labels, int numHeads);
384+
385+
/// See [analyzeSwitchStatement] and [analyzeSwitchExpression].
386+
void handleCaseHead({required bool hasWhen});
338387

339388
/// See [analyzeConstOrLiteralPattern].
340389
void handleConstOrLiteralPattern();

0 commit comments

Comments
 (0)