From c06d2964bc0504e4b61d2634e81969e92fb5ea4d Mon Sep 17 00:00:00 2001 From: Cedric Conday Date: Tue, 30 Jun 2026 14:53:51 +0000 Subject: [PATCH] Fix reachability of code after try with a branching finally (#63583) The reachability analysis caches results for shared flow nodes in a process-global cache (flowNodeReachable). When a ReduceLabel is active (try/finally), it temporarily rewrites the target label's antecedents, making the reachability of nodes reached during that walk dependent on the reduced context. The ReduceLabel handler cleared only the single lastFlowNode cache, not the shared cache, so a branch inside a finally block (a shared merge node) had its reachability cached under one reduce context and reused under another, corrupting end-of-function and unreachable-code analysis. Suppress the shared reachability cache while a label reduction is active, depth-counted for nested try/finally. --- src/compiler/checker.ts | 11 +- ...witchReturnWithBranchingFinally.errors.txt | 78 +++++++ ...owSwitchReturnWithBranchingFinally.symbols | 107 ++++++++++ ...FlowSwitchReturnWithBranchingFinally.types | 194 ++++++++++++++++++ ...rolFlowSwitchReturnWithBranchingFinally.ts | 75 +++++++ 5 files changed, 464 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.errors.txt create mode 100644 tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.symbols create mode 100644 tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.types create mode 100644 tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0567712f11da3..bec0ffd01fa84 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2364,6 +2364,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { var sharedFlowTypes: FlowType[] = []; var flowNodeReachable: (boolean | undefined)[] = []; var flowNodePostSuper: (boolean | undefined)[] = []; + var reduceLabelDepth = 0; var potentialThisCollisions: Node[] = []; var potentialNewTargetCollisions: Node[] = []; var potentialWeakMapSetCollisions: Node[] = []; @@ -28867,7 +28868,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } const flags = flow.flags; if (flags & FlowFlags.Shared) { - if (!noCacheCheck) { + // While a reduce label is active (try/finally), a shared node's reachability is + // context-dependent on the temporarily reduced label antecedents, so it must not + // be served from or written to the process-global cache (see #63583). + if (!noCacheCheck && reduceLabelDepth === 0) { const id = getFlowNodeId(flow); const reachable = flowNodeReachable[id]; return reachable !== undefined ? reachable : (flowNodeReachable[id] = isReachableFlowNodeWorker(flow, /*noCacheCheck*/ true)); @@ -28920,7 +28924,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const target = (flow as FlowReduceLabel).node.target; const saveAntecedents = target.antecedent; target.antecedent = (flow as FlowReduceLabel).node.antecedents; + // Suppress the shared reachability cache for the duration of the reduced walk: + // reachability computed under the reduced antecedents is not valid outside this + // context (and vice versa). Depth-counted to handle nested try/finally (see #63583). + reduceLabelDepth++; const result = isReachableFlowNodeWorker((flow as FlowReduceLabel).antecedent, /*noCacheCheck*/ false); + reduceLabelDepth--; target.antecedent = saveAntecedents; return result; } diff --git a/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.errors.txt b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.errors.txt new file mode 100644 index 0000000000000..aecfd47f5958e --- /dev/null +++ b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.errors.txt @@ -0,0 +1,78 @@ +controlFlowSwitchReturnWithBranchingFinally.ts(32,9): error TS7027: Unreachable code detected. + + +==== controlFlowSwitchReturnWithBranchingFinally.ts (1 errors) ==== + // https://github.com/microsoft/TypeScript/issues/63583 + // A branch inside a `finally` block must not corrupt the reachability of the + // flow after the protected `try`. Here every `switch` case returns, so the + // function fully returns and the trailing code is genuinely unreachable — + // regardless of what the `finally` block does. + + let y = false; + + // Was wrongly: "Function lacks ending return statement..." because the branch + // in `finally` (the short-circuit of `||=`) polluted the shared reachability cache. + function test1(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } + } + + // test2 documents the *correct* counterpart: with an explicit trailing return, + // that return is genuinely unreachable (the switch is exhaustive on `boolean`), + // so reporting it is correct and finally-independent. Before the fix, test1 and + // test2 contradicted each other (test1 demanded this return; test2 then rejected + // it). After the fix they are consistent: test1 needs no return, test2's is dead. + function test2(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + return 0; // correctly flagged unreachable + ~~~~~~~~~ +!!! error TS7027: Unreachable code detected. + } + finally { y ||= true; } + } + + // A plain `if` (read with a branch) in `finally` triggered the same bug. + function test3(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { if (y) { } } + } + + // Control: linear (non-branching) finally always worked. + function test4(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y = true; } + } + + // Nested try/finally with branching finally blocks (exercises depth counting). + function test5(x: boolean): number { + try { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } + } + finally { if (y) { } } + } + \ No newline at end of file diff --git a/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.symbols b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.symbols new file mode 100644 index 0000000000000..d720e323457a6 --- /dev/null +++ b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.symbols @@ -0,0 +1,107 @@ +//// [tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts] //// + +=== controlFlowSwitchReturnWithBranchingFinally.ts === +// https://github.com/microsoft/TypeScript/issues/63583 +// A branch inside a `finally` block must not corrupt the reachability of the +// flow after the protected `try`. Here every `switch` case returns, so the +// function fully returns and the trailing code is genuinely unreachable — +// regardless of what the `finally` block does. + +let y = false; +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) + +// Was wrongly: "Function lacks ending return statement..." because the branch +// in `finally` (the short-circuit of `||=`) polluted the shared reachability cache. +function test1(x: boolean): number { +>test1 : Symbol(test1, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 14)) +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 10, 15)) + + try { + switch (x) { +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 10, 15)) + + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) +} + +// test2 documents the *correct* counterpart: with an explicit trailing return, +// that return is genuinely unreachable (the switch is exhaustive on `boolean`), +// so reporting it is correct and finally-independent. Before the fix, test1 and +// test2 contradicted each other (test1 demanded this return; test2 then rejected +// it). After the fix they are consistent: test1 needs no return, test2's is dead. +function test2(x: boolean): number { +>test2 : Symbol(test2, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 18, 1)) +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 25, 15)) + + try { + switch (x) { +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 25, 15)) + + case true: return 1; + case false: return 0; + } + return 0; // correctly flagged unreachable + } + finally { y ||= true; } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) +} + +// A plain `if` (read with a branch) in `finally` triggered the same bug. +function test3(x: boolean): number { +>test3 : Symbol(test3, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 34, 1)) +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 37, 15)) + + try { + switch (x) { +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 37, 15)) + + case true: return 1; + case false: return 0; + } + } + finally { if (y) { } } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) +} + +// Control: linear (non-branching) finally always worked. +function test4(x: boolean): number { +>test4 : Symbol(test4, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 45, 1)) +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 48, 15)) + + try { + switch (x) { +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 48, 15)) + + case true: return 1; + case false: return 0; + } + } + finally { y = true; } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) +} + +// Nested try/finally with branching finally blocks (exercises depth counting). +function test5(x: boolean): number { +>test5 : Symbol(test5, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 56, 1)) +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 59, 15)) + + try { + try { + switch (x) { +>x : Symbol(x, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 59, 15)) + + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) + } + finally { if (y) { } } +>y : Symbol(y, Decl(controlFlowSwitchReturnWithBranchingFinally.ts, 6, 3)) +} + diff --git a/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.types b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.types new file mode 100644 index 0000000000000..dd33f84c887e7 --- /dev/null +++ b/tests/baselines/reference/controlFlowSwitchReturnWithBranchingFinally.types @@ -0,0 +1,194 @@ +//// [tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts] //// + +=== controlFlowSwitchReturnWithBranchingFinally.ts === +// https://github.com/microsoft/TypeScript/issues/63583 +// A branch inside a `finally` block must not corrupt the reachability of the +// flow after the protected `try`. Here every `switch` case returns, so the +// function fully returns and the trailing code is genuinely unreachable — +// regardless of what the `finally` block does. + +let y = false; +>y : boolean +> : ^^^^^^^ +>false : false +> : ^^^^^ + +// Was wrongly: "Function lacks ending return statement..." because the branch +// in `finally` (the short-circuit of `||=`) polluted the shared reachability cache. +function test1(x: boolean): number { +>test1 : (x: boolean) => number +> : ^ ^^ ^^^^^ +>x : boolean +> : ^^^^^^^ + + try { + switch (x) { +>x : boolean +> : ^^^^^^^ + + case true: return 1; +>true : true +> : ^^^^ +>1 : 1 +> : ^ + + case false: return 0; +>false : false +> : ^^^^^ +>0 : 0 +> : ^ + } + } + finally { y ||= true; } +>y ||= true : true +> : ^^^^ +>y : boolean +> : ^^^^^^^ +>true : true +> : ^^^^ +} + +// test2 documents the *correct* counterpart: with an explicit trailing return, +// that return is genuinely unreachable (the switch is exhaustive on `boolean`), +// so reporting it is correct and finally-independent. Before the fix, test1 and +// test2 contradicted each other (test1 demanded this return; test2 then rejected +// it). After the fix they are consistent: test1 needs no return, test2's is dead. +function test2(x: boolean): number { +>test2 : (x: boolean) => number +> : ^ ^^ ^^^^^ +>x : boolean +> : ^^^^^^^ + + try { + switch (x) { +>x : boolean +> : ^^^^^^^ + + case true: return 1; +>true : true +> : ^^^^ +>1 : 1 +> : ^ + + case false: return 0; +>false : false +> : ^^^^^ +>0 : 0 +> : ^ + } + return 0; // correctly flagged unreachable +>0 : 0 +> : ^ + } + finally { y ||= true; } +>y ||= true : true +> : ^^^^ +>y : boolean +> : ^^^^^^^ +>true : true +> : ^^^^ +} + +// A plain `if` (read with a branch) in `finally` triggered the same bug. +function test3(x: boolean): number { +>test3 : (x: boolean) => number +> : ^ ^^ ^^^^^ +>x : boolean +> : ^^^^^^^ + + try { + switch (x) { +>x : boolean +> : ^^^^^^^ + + case true: return 1; +>true : true +> : ^^^^ +>1 : 1 +> : ^ + + case false: return 0; +>false : false +> : ^^^^^ +>0 : 0 +> : ^ + } + } + finally { if (y) { } } +>y : boolean +> : ^^^^^^^ +} + +// Control: linear (non-branching) finally always worked. +function test4(x: boolean): number { +>test4 : (x: boolean) => number +> : ^ ^^ ^^^^^ +>x : boolean +> : ^^^^^^^ + + try { + switch (x) { +>x : boolean +> : ^^^^^^^ + + case true: return 1; +>true : true +> : ^^^^ +>1 : 1 +> : ^ + + case false: return 0; +>false : false +> : ^^^^^ +>0 : 0 +> : ^ + } + } + finally { y = true; } +>y = true : true +> : ^^^^ +>y : boolean +> : ^^^^^^^ +>true : true +> : ^^^^ +} + +// Nested try/finally with branching finally blocks (exercises depth counting). +function test5(x: boolean): number { +>test5 : (x: boolean) => number +> : ^ ^^ ^^^^^ +>x : boolean +> : ^^^^^^^ + + try { + try { + switch (x) { +>x : boolean +> : ^^^^^^^ + + case true: return 1; +>true : true +> : ^^^^ +>1 : 1 +> : ^ + + case false: return 0; +>false : false +> : ^^^^^ +>0 : 0 +> : ^ + } + } + finally { y ||= true; } +>y ||= true : true +> : ^^^^ +>y : boolean +> : ^^^^^^^ +>true : true +> : ^^^^ + } + finally { if (y) { } } +>y : boolean +> : ^^^^^^^ +} + diff --git a/tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts b/tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts new file mode 100644 index 0000000000000..0eb10be696344 --- /dev/null +++ b/tests/cases/compiler/controlFlowSwitchReturnWithBranchingFinally.ts @@ -0,0 +1,75 @@ +// @strict: true +// @allowUnreachableCode: false +// @noEmit: true + +// https://github.com/microsoft/TypeScript/issues/63583 +// A branch inside a `finally` block must not corrupt the reachability of the +// flow after the protected `try`. Here every `switch` case returns, so the +// function fully returns and the trailing code is genuinely unreachable — +// regardless of what the `finally` block does. + +let y = false; + +// Was wrongly: "Function lacks ending return statement..." because the branch +// in `finally` (the short-circuit of `||=`) polluted the shared reachability cache. +function test1(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } +} + +// test2 documents the *correct* counterpart: with an explicit trailing return, +// that return is genuinely unreachable (the switch is exhaustive on `boolean`), +// so reporting it is correct and finally-independent. Before the fix, test1 and +// test2 contradicted each other (test1 demanded this return; test2 then rejected +// it). After the fix they are consistent: test1 needs no return, test2's is dead. +function test2(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + return 0; // correctly flagged unreachable + } + finally { y ||= true; } +} + +// A plain `if` (read with a branch) in `finally` triggered the same bug. +function test3(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { if (y) { } } +} + +// Control: linear (non-branching) finally always worked. +function test4(x: boolean): number { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y = true; } +} + +// Nested try/finally with branching finally blocks (exercises depth counting). +function test5(x: boolean): number { + try { + try { + switch (x) { + case true: return 1; + case false: return 0; + } + } + finally { y ||= true; } + } + finally { if (y) { } } +}