Skip to content

Commit 1320c36

Browse files
ahejlsbergahejlsberg
authored
Fix control flow analysis for break/continue with label (#35377)
* Use existing 'continue' target labels for labeled statements * Use linked list for active labels * Add regression test * Accept new baselines
1 parent 25ec62f commit 1320c36

File tree

5 files changed

+173
-46
lines changed

5 files changed

+173
-46
lines changed

‎src/compiler/binder.ts

Lines changed: 36 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ namespace ts {
88
}
99

1010
interface ActiveLabel {
11+
next: ActiveLabel | undefined;
1112
name: __String;
1213
breakTarget: FlowLabel;
13-
continueTarget: FlowLabel;
14+
continueTarget: FlowLabel | undefined;
1415
referenced: boolean;
1516
}
1617

@@ -199,7 +200,7 @@ namespace ts {
199200
let currentFalseTarget: FlowLabel | undefined;
200201
let currentExceptionTarget: FlowLabel | undefined;
201202
let preSwitchCaseFlow: FlowNode | undefined;
202-
let activeLabels: ActiveLabel[] | undefined;
203+
let activeLabelList: ActiveLabel | undefined;
203204
let hasExplicitReturn: boolean;
204205

205206
// state used for emit helpers
@@ -271,7 +272,7 @@ namespace ts {
271272
currentTrueTarget = undefined;
272273
currentFalseTarget = undefined;
273274
currentExceptionTarget = undefined;
274-
activeLabels = undefined!;
275+
activeLabelList = undefined;
275276
hasExplicitReturn = false;
276277
emitFlags = NodeFlags.None;
277278
subtreeTransformFlags = TransformFlags.None;
@@ -629,7 +630,7 @@ namespace ts {
629630
const saveContinueTarget = currentContinueTarget;
630631
const saveReturnTarget = currentReturnTarget;
631632
const saveExceptionTarget = currentExceptionTarget;
632-
const saveActiveLabels = activeLabels;
633+
const saveActiveLabelList = activeLabelList;
633634
const saveHasExplicitReturn = hasExplicitReturn;
634635
const isIIFE = containerFlags & ContainerFlags.IsFunctionExpression && !hasModifier(node, ModifierFlags.Async) &&
635636
!(<FunctionLikeDeclaration>node).asteriskToken && !!getImmediatelyInvokedFunctionExpression(node);
@@ -647,7 +648,7 @@ namespace ts {
647648
currentExceptionTarget = undefined;
648649
currentBreakTarget = undefined;
649650
currentContinueTarget = undefined;
650-
activeLabels = undefined;
651+
activeLabelList = undefined;
651652
hasExplicitReturn = false;
652653
bindChildren(node);
653654
// Reset all reachability check related flags on node (for incremental scenarios)
@@ -675,7 +676,7 @@ namespace ts {
675676
currentContinueTarget = saveContinueTarget;
676677
currentReturnTarget = saveReturnTarget;
677678
currentExceptionTarget = saveExceptionTarget;
678-
activeLabels = saveActiveLabels;
679+
activeLabelList = saveActiveLabelList;
679680
hasExplicitReturn = saveHasExplicitReturn;
680681
}
681682
else if (containerFlags & ContainerFlags.IsInterface) {
@@ -1063,8 +1064,18 @@ namespace ts {
10631064
currentContinueTarget = saveContinueTarget;
10641065
}
10651066

1067+
function setContinueTarget(node: Node, target: FlowLabel) {
1068+
let label = activeLabelList;
1069+
while (label && node.parent.kind === SyntaxKind.LabeledStatement) {
1070+
label.continueTarget = target;
1071+
label = label.next;
1072+
node = node.parent;
1073+
}
1074+
return target;
1075+
}
1076+
10661077
function bindWhileStatement(node: WhileStatement): void {
1067-
const preWhileLabel = createLoopLabel();
1078+
const preWhileLabel = setContinueTarget(node, createLoopLabel());
10681079
const preBodyLabel = createBranchLabel();
10691080
const postWhileLabel = createBranchLabel();
10701081
addAntecedent(preWhileLabel, currentFlow);
@@ -1078,13 +1089,8 @@ namespace ts {
10781089

10791090
function bindDoStatement(node: DoStatement): void {
10801091
const preDoLabel = createLoopLabel();
1081-
const enclosingLabeledStatement = node.parent.kind === SyntaxKind.LabeledStatement
1082-
? lastOrUndefined(activeLabels!)
1083-
: undefined;
1084-
// if do statement is wrapped in labeled statement then target labels for break/continue with or without
1085-
// label should be the same
1086-
const preConditionLabel = enclosingLabeledStatement ? enclosingLabeledStatement.continueTarget : createBranchLabel();
1087-
const postDoLabel = enclosingLabeledStatement ? enclosingLabeledStatement.breakTarget : createBranchLabel();
1092+
const preConditionLabel = setContinueTarget(node, createBranchLabel());
1093+
const postDoLabel = createBranchLabel();
10881094
addAntecedent(preDoLabel, currentFlow);
10891095
currentFlow = preDoLabel;
10901096
bindIterativeStatement(node.statement, postDoLabel, preConditionLabel);
@@ -1095,7 +1101,7 @@ namespace ts {
10951101
}
10961102

10971103
function bindForStatement(node: ForStatement): void {
1098-
const preLoopLabel = createLoopLabel();
1104+
const preLoopLabel = setContinueTarget(node, createLoopLabel());
10991105
const preBodyLabel = createBranchLabel();
11001106
const postLoopLabel = createBranchLabel();
11011107
bind(node.initializer);
@@ -1110,7 +1116,7 @@ namespace ts {
11101116
}
11111117

11121118
function bindForInOrForOfStatement(node: ForInOrOfStatement): void {
1113-
const preLoopLabel = createLoopLabel();
1119+
const preLoopLabel = setContinueTarget(node, createLoopLabel());
11141120
const postLoopLabel = createBranchLabel();
11151121
bind(node.expression);
11161122
addAntecedent(preLoopLabel, currentFlow);
@@ -1154,11 +1160,9 @@ namespace ts {
11541160
}
11551161

11561162
function findActiveLabel(name: __String) {
1157-
if (activeLabels) {
1158-
for (const label of activeLabels) {
1159-
if (label.name === name) {
1160-
return label;
1161-
}
1163+
for (let label = activeLabelList; label; label = label.next) {
1164+
if (label.name === name) {
1165+
return label;
11621166
}
11631167
}
11641168
return undefined;
@@ -1313,21 +1317,6 @@ namespace ts {
13131317
bindEach(node.statements);
13141318
}
13151319

1316-
function pushActiveLabel(name: __String, breakTarget: FlowLabel, continueTarget: FlowLabel): ActiveLabel {
1317-
const activeLabel: ActiveLabel = {
1318-
name,
1319-
breakTarget,
1320-
continueTarget,
1321-
referenced: false
1322-
};
1323-
(activeLabels || (activeLabels = [])).push(activeLabel);
1324-
return activeLabel;
1325-
}
1326-
1327-
function popActiveLabel() {
1328-
activeLabels!.pop();
1329-
}
1330-
13311320
function bindExpressionStatement(node: ExpressionStatement): void {
13321321
bind(node.expression);
13331322
// A top level call expression with a dotted function name and at least one argument
@@ -1341,21 +1330,22 @@ namespace ts {
13411330
}
13421331

13431332
function bindLabeledStatement(node: LabeledStatement): void {
1344-
const preStatementLabel = createLoopLabel();
13451333
const postStatementLabel = createBranchLabel();
1334+
activeLabelList = {
1335+
next: activeLabelList,
1336+
name: node.label.escapedText,
1337+
breakTarget: postStatementLabel,
1338+
continueTarget: undefined,
1339+
referenced: false
1340+
};
13461341
bind(node.label);
1347-
addAntecedent(preStatementLabel, currentFlow);
1348-
const activeLabel = pushActiveLabel(node.label.escapedText, postStatementLabel, preStatementLabel);
13491342
bind(node.statement);
1350-
popActiveLabel();
1351-
if (!activeLabel.referenced && !options.allowUnusedLabels) {
1343+
if (!activeLabelList.referenced && !options.allowUnusedLabels) {
13521344
errorOrSuggestionOnNode(unusedLabelIsError(options), node.label, Diagnostics.Unused_label);
13531345
}
1354-
if (!node.statement || node.statement.kind !== SyntaxKind.DoStatement) {
1355-
// do statement sets current flow inside bindDoStatement
1356-
addAntecedent(postStatementLabel, currentFlow);
1357-
currentFlow = finishFlowLabel(postStatementLabel);
1358-
}
1346+
activeLabelList = activeLabelList.next;
1347+
addAntecedent(postStatementLabel, currentFlow);
1348+
currentFlow = finishFlowLabel(postStatementLabel);
13591349
}
13601350

13611351
function bindDestructuringTargetFlow(node: Expression) {

‎tests/baselines/reference/controlFlowBreakContinueWithLabel.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//// [controlFlowBreakContinueWithLabel.ts]
2+
enum User { A, B }
3+
4+
let user: User = User.A
5+
6+
label: while (true) {
7+
switch (user) {
8+
case User.A:
9+
user = User.B;
10+
continue label;
11+
case User.B:
12+
break label;
13+
}
14+
}
15+
user;
16+
17+
18+
//// [controlFlowBreakContinueWithLabel.js]
19+
"use strict";
20+
var User;
21+
(function (User) {
22+
User[User["A"] = 0] = "A";
23+
User[User["B"] = 1] = "B";
24+
})(User || (User = {}));
25+
var user = User.A;
26+
label: while (true) {
27+
switch (user) {
28+
case User.A:
29+
user = User.B;
30+
continue label;
31+
case User.B:
32+
break label;
33+
}
34+
}
35+
user;

‎tests/baselines/reference/controlFlowBreakContinueWithLabel.symbols

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
=== tests/cases/compiler/controlFlowBreakContinueWithLabel.ts ===
2+
enum User { A, B }
3+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
4+
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
5+
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
6+
7+
let user: User = User.A
8+
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
9+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
10+
>User.A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
11+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
12+
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
13+
14+
label: while (true) {
15+
switch (user) {
16+
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
17+
18+
case User.A:
19+
>User.A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
20+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
21+
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
22+
23+
user = User.B;
24+
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
25+
>User.B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
26+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
27+
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
28+
29+
continue label;
30+
case User.B:
31+
>User.B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
32+
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
33+
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
34+
35+
break label;
36+
}
37+
}
38+
user;
39+
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
40+

‎tests/baselines/reference/controlFlowBreakContinueWithLabel.types

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
=== tests/cases/compiler/controlFlowBreakContinueWithLabel.ts ===
2+
enum User { A, B }
3+
>User : User
4+
>A : User.A
5+
>B : User.B
6+
7+
let user: User = User.A
8+
>user : User
9+
>User.A : User.A
10+
>User : typeof User
11+
>A : User.A
12+
13+
label: while (true) {
14+
>label : any
15+
>true : true
16+
17+
switch (user) {
18+
>user : User
19+
20+
case User.A:
21+
>User.A : User.A
22+
>User : typeof User
23+
>A : User.A
24+
25+
user = User.B;
26+
>user = User.B : User.B
27+
>user : User
28+
>User.B : User.B
29+
>User : typeof User
30+
>B : User.B
31+
32+
continue label;
33+
>label : any
34+
35+
case User.B:
36+
>User.B : User.B
37+
>User : typeof User
38+
>B : User.B
39+
40+
break label;
41+
>label : any
42+
}
43+
}
44+
user;
45+
>user : User.B
46+

‎tests/cases/compiler/controlFlowBreakContinueWithLabel.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// @strict: true
2+
3+
enum User { A, B }
4+
5+
let user: User = User.A
6+
7+
label: while (true) {
8+
switch (user) {
9+
case User.A:
10+
user = User.B;
11+
continue label;
12+
case User.B:
13+
break label;
14+
}
15+
}
16+
user;

0 commit comments

Comments
 (0)