Skip to content

Fix control flow analysis for break/continue with label #35377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 36 additions & 46 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ namespace ts {
}

interface ActiveLabel {
next: ActiveLabel | undefined;
name: __String;
breakTarget: FlowLabel;
continueTarget: FlowLabel;
continueTarget: FlowLabel | undefined;
referenced: boolean;
}

Expand Down Expand Up @@ -199,7 +200,7 @@ namespace ts {
let currentFalseTarget: FlowLabel | undefined;
let currentExceptionTarget: FlowLabel | undefined;
let preSwitchCaseFlow: FlowNode | undefined;
let activeLabels: ActiveLabel[] | undefined;
let activeLabelList: ActiveLabel | undefined;
let hasExplicitReturn: boolean;

// state used for emit helpers
Expand Down Expand Up @@ -271,7 +272,7 @@ namespace ts {
currentTrueTarget = undefined;
currentFalseTarget = undefined;
currentExceptionTarget = undefined;
activeLabels = undefined!;
activeLabelList = undefined;
hasExplicitReturn = false;
emitFlags = NodeFlags.None;
subtreeTransformFlags = TransformFlags.None;
Expand Down Expand Up @@ -629,7 +630,7 @@ namespace ts {
const saveContinueTarget = currentContinueTarget;
const saveReturnTarget = currentReturnTarget;
const saveExceptionTarget = currentExceptionTarget;
const saveActiveLabels = activeLabels;
const saveActiveLabelList = activeLabelList;
const saveHasExplicitReturn = hasExplicitReturn;
const isIIFE = containerFlags & ContainerFlags.IsFunctionExpression && !hasModifier(node, ModifierFlags.Async) &&
!(<FunctionLikeDeclaration>node).asteriskToken && !!getImmediatelyInvokedFunctionExpression(node);
Expand All @@ -647,7 +648,7 @@ namespace ts {
currentExceptionTarget = undefined;
currentBreakTarget = undefined;
currentContinueTarget = undefined;
activeLabels = undefined;
activeLabelList = undefined;
hasExplicitReturn = false;
bindChildren(node);
// Reset all reachability check related flags on node (for incremental scenarios)
Expand Down Expand Up @@ -675,7 +676,7 @@ namespace ts {
currentContinueTarget = saveContinueTarget;
currentReturnTarget = saveReturnTarget;
currentExceptionTarget = saveExceptionTarget;
activeLabels = saveActiveLabels;
activeLabelList = saveActiveLabelList;
hasExplicitReturn = saveHasExplicitReturn;
}
else if (containerFlags & ContainerFlags.IsInterface) {
Expand Down Expand Up @@ -1063,8 +1064,18 @@ namespace ts {
currentContinueTarget = saveContinueTarget;
}

function setContinueTarget(node: Node, target: FlowLabel) {
let label = activeLabelList;
while (label && node.parent.kind === SyntaxKind.LabeledStatement) {
label.continueTarget = target;
label = label.next;
node = node.parent;
}
return target;
}

function bindWhileStatement(node: WhileStatement): void {
const preWhileLabel = createLoopLabel();
const preWhileLabel = setContinueTarget(node, createLoopLabel());
const preBodyLabel = createBranchLabel();
const postWhileLabel = createBranchLabel();
addAntecedent(preWhileLabel, currentFlow);
Expand All @@ -1078,13 +1089,8 @@ namespace ts {

function bindDoStatement(node: DoStatement): void {
const preDoLabel = createLoopLabel();
const enclosingLabeledStatement = node.parent.kind === SyntaxKind.LabeledStatement
? lastOrUndefined(activeLabels!)
: undefined;
// if do statement is wrapped in labeled statement then target labels for break/continue with or without
// label should be the same
const preConditionLabel = enclosingLabeledStatement ? enclosingLabeledStatement.continueTarget : createBranchLabel();
const postDoLabel = enclosingLabeledStatement ? enclosingLabeledStatement.breakTarget : createBranchLabel();
const preConditionLabel = setContinueTarget(node, createBranchLabel());
const postDoLabel = createBranchLabel();
addAntecedent(preDoLabel, currentFlow);
currentFlow = preDoLabel;
bindIterativeStatement(node.statement, postDoLabel, preConditionLabel);
Expand All @@ -1095,7 +1101,7 @@ namespace ts {
}

function bindForStatement(node: ForStatement): void {
const preLoopLabel = createLoopLabel();
const preLoopLabel = setContinueTarget(node, createLoopLabel());
const preBodyLabel = createBranchLabel();
const postLoopLabel = createBranchLabel();
bind(node.initializer);
Expand All @@ -1110,7 +1116,7 @@ namespace ts {
}

function bindForInOrForOfStatement(node: ForInOrOfStatement): void {
const preLoopLabel = createLoopLabel();
const preLoopLabel = setContinueTarget(node, createLoopLabel());
const postLoopLabel = createBranchLabel();
bind(node.expression);
addAntecedent(preLoopLabel, currentFlow);
Expand Down Expand Up @@ -1154,11 +1160,9 @@ namespace ts {
}

function findActiveLabel(name: __String) {
if (activeLabels) {
for (const label of activeLabels) {
if (label.name === name) {
return label;
}
for (let label = activeLabelList; label; label = label.next) {
if (label.name === name) {
return label;
}
}
return undefined;
Expand Down Expand Up @@ -1313,21 +1317,6 @@ namespace ts {
bindEach(node.statements);
}

function pushActiveLabel(name: __String, breakTarget: FlowLabel, continueTarget: FlowLabel): ActiveLabel {
const activeLabel: ActiveLabel = {
name,
breakTarget,
continueTarget,
referenced: false
};
(activeLabels || (activeLabels = [])).push(activeLabel);
return activeLabel;
}

function popActiveLabel() {
activeLabels!.pop();
}

function bindExpressionStatement(node: ExpressionStatement): void {
bind(node.expression);
// A top level call expression with a dotted function name and at least one argument
Expand All @@ -1341,21 +1330,22 @@ namespace ts {
}

function bindLabeledStatement(node: LabeledStatement): void {
const preStatementLabel = createLoopLabel();
const postStatementLabel = createBranchLabel();
activeLabelList = {
next: activeLabelList,
name: node.label.escapedText,
breakTarget: postStatementLabel,
continueTarget: undefined,
referenced: false
};
bind(node.label);
addAntecedent(preStatementLabel, currentFlow);
const activeLabel = pushActiveLabel(node.label.escapedText, postStatementLabel, preStatementLabel);
bind(node.statement);
popActiveLabel();
if (!activeLabel.referenced && !options.allowUnusedLabels) {
if (!activeLabelList.referenced && !options.allowUnusedLabels) {
errorOrSuggestionOnNode(unusedLabelIsError(options), node.label, Diagnostics.Unused_label);
}
if (!node.statement || node.statement.kind !== SyntaxKind.DoStatement) {
// do statement sets current flow inside bindDoStatement
addAntecedent(postStatementLabel, currentFlow);
currentFlow = finishFlowLabel(postStatementLabel);
}
activeLabelList = activeLabelList.next;
addAntecedent(postStatementLabel, currentFlow);
currentFlow = finishFlowLabel(postStatementLabel);
}

function bindDestructuringTargetFlow(node: Expression) {
Expand Down
35 changes: 35 additions & 0 deletions tests/baselines/reference/controlFlowBreakContinueWithLabel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [controlFlowBreakContinueWithLabel.ts]
enum User { A, B }

let user: User = User.A

label: while (true) {
switch (user) {
case User.A:
user = User.B;
continue label;
case User.B:
break label;
}
}
user;


//// [controlFlowBreakContinueWithLabel.js]
"use strict";
var User;
(function (User) {
User[User["A"] = 0] = "A";
User[User["B"] = 1] = "B";
})(User || (User = {}));
var user = User.A;
label: while (true) {
switch (user) {
case User.A:
user = User.B;
continue label;
case User.B:
break label;
}
}
user;
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/compiler/controlFlowBreakContinueWithLabel.ts ===
enum User { A, B }
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))

let user: User = User.A
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>User.A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))

label: while (true) {
switch (user) {
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))

case User.A:
>User.A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>A : Symbol(User.A, Decl(controlFlowBreakContinueWithLabel.ts, 0, 11))

user = User.B;
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))
>User.B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))

continue label;
case User.B:
>User.B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))
>User : Symbol(User, Decl(controlFlowBreakContinueWithLabel.ts, 0, 0))
>B : Symbol(User.B, Decl(controlFlowBreakContinueWithLabel.ts, 0, 14))

break label;
}
}
user;
>user : Symbol(user, Decl(controlFlowBreakContinueWithLabel.ts, 2, 3))

46 changes: 46 additions & 0 deletions tests/baselines/reference/controlFlowBreakContinueWithLabel.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
=== tests/cases/compiler/controlFlowBreakContinueWithLabel.ts ===
enum User { A, B }
>User : User
>A : User.A
>B : User.B

let user: User = User.A
>user : User
>User.A : User.A
>User : typeof User
>A : User.A

label: while (true) {
>label : any
>true : true

switch (user) {
>user : User

case User.A:
>User.A : User.A
>User : typeof User
>A : User.A

user = User.B;
>user = User.B : User.B
>user : User
>User.B : User.B
>User : typeof User
>B : User.B

continue label;
>label : any

case User.B:
>User.B : User.B
>User : typeof User
>B : User.B

break label;
>label : any
}
}
user;
>user : User.B

16 changes: 16 additions & 0 deletions tests/cases/compiler/controlFlowBreakContinueWithLabel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @strict: true

enum User { A, B }

let user: User = User.A

label: while (true) {
switch (user) {
case User.A:
user = User.B;
continue label;
case User.B:
break label;
}
}
user;