Skip to content

Commit cafa175

Browse files
authored
Properly reflect CFA effects of return in try or catch blocks (#35730)
* Properly reflect CFA effects of return in try or catch blocks * Add tests * Accept new baselines
1 parent f90cde4 commit cafa175

File tree

6 files changed

+572
-5
lines changed

6 files changed

+572
-5
lines changed

src/compiler/binder.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,8 +1198,10 @@ namespace ts {
11981198
// exceptions. We add all mutation flow nodes as antecedents of this label such that we can analyze them
11991199
// as possible antecedents of the start of catch or finally blocks. Furthermore, we add the current
12001200
// control flow to represent exceptions that occur before any mutations.
1201+
const saveReturnTarget = currentReturnTarget;
12011202
const saveExceptionTarget = currentExceptionTarget;
1202-
currentExceptionTarget = createBranchLabel();
1203+
currentReturnTarget = createBranchLabel();
1204+
currentExceptionTarget = node.catchClause ? createBranchLabel() : currentReturnTarget;
12031205
addAntecedent(currentExceptionTarget, currentFlow);
12041206
bind(node.tryBlock);
12051207
addAntecedent(preFinallyLabel, currentFlow);
@@ -1211,22 +1213,24 @@ namespace ts {
12111213
// The currentExceptionTarget now represents control flows from exceptions in the catch clause.
12121214
// Effectively, in a try-catch-finally, if an exception occurs in the try block, the catch block
12131215
// acts like a second try block.
1214-
currentExceptionTarget = createBranchLabel();
1216+
currentExceptionTarget = currentReturnTarget;
12151217
addAntecedent(currentExceptionTarget, currentFlow);
12161218
bind(node.catchClause);
12171219
addAntecedent(preFinallyLabel, currentFlow);
12181220
flowAfterCatch = currentFlow;
12191221
}
12201222
const exceptionTarget = finishFlowLabel(currentExceptionTarget);
1223+
currentReturnTarget = saveReturnTarget;
12211224
currentExceptionTarget = saveExceptionTarget;
12221225
if (node.finallyBlock) {
12231226
// Possible ways control can reach the finally block:
12241227
// 1) Normal completion of try block of a try-finally or try-catch-finally
12251228
// 2) Normal completion of catch block (following exception in try block) of a try-catch-finally
1226-
// 3) Exception in try block of a try-finally
1227-
// 4) Exception in catch block of a try-catch-finally
1229+
// 3) Return in try or catch block of a try-finally or try-catch-finally
1230+
// 4) Exception in try block of a try-finally
1231+
// 5) Exception in catch block of a try-catch-finally
12281232
// When analyzing a control flow graph that starts inside a finally block we want to consider all
1229-
// four possibilities above. However, when analyzing a control flow graph that starts outside (past)
1233+
// five possibilities above. However, when analyzing a control flow graph that starts outside (past)
12301234
// the finally block, we only want to consider the first two (if we're past a finally block then it
12311235
// must have completed normally). To make this possible, we inject two extra nodes into the control
12321236
// flow graph: An after-finally with an antecedent of the control flow at the end of the finally
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
tests/cases/compiler/tryCatchFinallyControlFlow.ts(105,5): error TS7027: Unreachable code detected.
2+
3+
4+
==== tests/cases/compiler/tryCatchFinallyControlFlow.ts (1 errors) ====
5+
// Repro from #34797
6+
7+
function f1() {
8+
let a: number | null = null;
9+
try {
10+
a = 123;
11+
return a;
12+
}
13+
catch (e) {
14+
throw e;
15+
}
16+
finally {
17+
if (a != null && a.toFixed(0) == "123") {
18+
}
19+
}
20+
}
21+
22+
function f2() {
23+
let x: 0 | 1 | 2 | 3 = 0;
24+
try {
25+
x = 1;
26+
}
27+
catch (e) {
28+
x = 2;
29+
throw e;
30+
}
31+
finally {
32+
x; // 0 | 1 | 2
33+
}
34+
x; // 1
35+
}
36+
37+
function f3() {
38+
let x: 0 | 1 | 2 | 3 = 0;
39+
try {
40+
x = 1;
41+
}
42+
catch (e) {
43+
x = 2;
44+
return;
45+
}
46+
finally {
47+
x; // 0 | 1 | 2
48+
}
49+
x; // 1
50+
}
51+
52+
function f4() {
53+
let x: 0 | 1 | 2 | 3 = 0;
54+
try {
55+
x = 1;
56+
}
57+
catch (e) {
58+
x = 2;
59+
}
60+
finally {
61+
x; // 0 | 1 | 2
62+
}
63+
x; // 1 | 2
64+
}
65+
66+
function f5() {
67+
let x: 0 | 1 | 2 | 3 = 0;
68+
try {
69+
x = 1;
70+
return;
71+
}
72+
catch (e) {
73+
x = 2;
74+
}
75+
finally {
76+
x; // 0 | 1 | 2
77+
}
78+
x; // 2
79+
}
80+
81+
function f6() {
82+
let x: 0 | 1 | 2 | 3 = 0;
83+
try {
84+
x = 1;
85+
}
86+
catch (e) {
87+
x = 2;
88+
return;
89+
}
90+
finally {
91+
x; // 0 | 1 | 2
92+
}
93+
x; // 1
94+
}
95+
96+
function f7() {
97+
let x: 0 | 1 | 2 | 3 = 0;
98+
try {
99+
x = 1;
100+
return;
101+
}
102+
catch (e) {
103+
x = 2;
104+
return;
105+
}
106+
finally {
107+
x; // 0 | 1 | 2
108+
}
109+
x; // Unreachable
110+
~~
111+
!!! error TS7027: Unreachable code detected.
112+
}
113+
114+
// Repro from #35644
115+
116+
const main = () => {
117+
let hoge: string | undefined = undefined;
118+
try {
119+
hoge = 'hoge!';
120+
return;
121+
}
122+
catch {
123+
return;
124+
}
125+
finally {
126+
if (hoge) {
127+
hoge.length;
128+
}
129+
return;
130+
}
131+
}
132+

tests/baselines/reference/tryCatchFinallyControlFlow.js

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,71 @@ function f4() {
5959
}
6060
x; // 1 | 2
6161
}
62+
63+
function f5() {
64+
let x: 0 | 1 | 2 | 3 = 0;
65+
try {
66+
x = 1;
67+
return;
68+
}
69+
catch (e) {
70+
x = 2;
71+
}
72+
finally {
73+
x; // 0 | 1 | 2
74+
}
75+
x; // 2
76+
}
77+
78+
function f6() {
79+
let x: 0 | 1 | 2 | 3 = 0;
80+
try {
81+
x = 1;
82+
}
83+
catch (e) {
84+
x = 2;
85+
return;
86+
}
87+
finally {
88+
x; // 0 | 1 | 2
89+
}
90+
x; // 1
91+
}
92+
93+
function f7() {
94+
let x: 0 | 1 | 2 | 3 = 0;
95+
try {
96+
x = 1;
97+
return;
98+
}
99+
catch (e) {
100+
x = 2;
101+
return;
102+
}
103+
finally {
104+
x; // 0 | 1 | 2
105+
}
106+
x; // Unreachable
107+
}
108+
109+
// Repro from #35644
110+
111+
const main = () => {
112+
let hoge: string | undefined = undefined;
113+
try {
114+
hoge = 'hoge!';
115+
return;
116+
}
117+
catch {
118+
return;
119+
}
120+
finally {
121+
if (hoge) {
122+
hoge.length;
123+
}
124+
return;
125+
}
126+
}
62127

63128

64129
//// [tryCatchFinallyControlFlow.js]
@@ -119,3 +184,63 @@ function f4() {
119184
}
120185
x; // 1 | 2
121186
}
187+
function f5() {
188+
var x = 0;
189+
try {
190+
x = 1;
191+
return;
192+
}
193+
catch (e) {
194+
x = 2;
195+
}
196+
finally {
197+
x; // 0 | 1 | 2
198+
}
199+
x; // 2
200+
}
201+
function f6() {
202+
var x = 0;
203+
try {
204+
x = 1;
205+
}
206+
catch (e) {
207+
x = 2;
208+
return;
209+
}
210+
finally {
211+
x; // 0 | 1 | 2
212+
}
213+
x; // 1
214+
}
215+
function f7() {
216+
var x = 0;
217+
try {
218+
x = 1;
219+
return;
220+
}
221+
catch (e) {
222+
x = 2;
223+
return;
224+
}
225+
finally {
226+
x; // 0 | 1 | 2
227+
}
228+
x; // Unreachable
229+
}
230+
// Repro from #35644
231+
var main = function () {
232+
var hoge = undefined;
233+
try {
234+
hoge = 'hoge!';
235+
return;
236+
}
237+
catch (_a) {
238+
return;
239+
}
240+
finally {
241+
if (hoge) {
242+
hoge.length;
243+
}
244+
return;
245+
}
246+
};

0 commit comments

Comments
 (0)