From 52a228d0f5024ed714002e33ba53eecf9dcd3e5b Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 07:50:59 -0500 Subject: [PATCH] Keep declarations a removed branch shares with another --- .../src/frontend/shape-inference/overlay.ts | 23 +++++++++++-------- .../npm/cases/shape-overlay-facts/main.ts | 6 +++-- .../node_modules/shape-facts/index.d.ts | 1 + .../node_modules/shape-facts/index.js | 19 +++++++++++++-- tests/harness/shape-overlay.test.ts | 5 ++-- 5 files changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/compiler/src/frontend/shape-inference/overlay.ts b/packages/compiler/src/frontend/shape-inference/overlay.ts index 7747fac20..edaad7b80 100644 --- a/packages/compiler/src/frontend/shape-inference/overlay.ts +++ b/packages/compiler/src/frontend/shape-inference/overlay.ts @@ -502,15 +502,18 @@ export function planShapeOverlay( // Dead branches: outermost first; edits inside a replaced range drop. const dead = (deadByFile.get(sf) ?? []).sort((a, b) => a.pos - b.pos); const ranges: [number, number][] = []; - const hoisted = new Map(); + const removed = new Set(); for (const branch of dead) { const start = ts.tokenStartOfNode(branch); if (ranges.some(([lo, hi]) => start >= lo && start < hi)) continue; - const names = hoistedNames(branch); - if (names === null) continue; - hoisted.set(start, names); + if (hoistedNames(branch, removed) === null) continue; + removed.add(branch); ranges.push([start, branch.end]); } + // A var that only removed branches declare stays declared in each. + const hoisted = new Map(); + for (const branch of removed) + hoisted.set(ts.tokenStartOfNode(branch), hoistedNames(branch, removed)!); if (ranges.length > 0) { const kept = edits.filter((e) => !ranges.some(([lo, hi]) => e.pos > lo && e.pos < hi)); edits.length = 0; @@ -664,11 +667,11 @@ function neutralizeComment(comment: string): string { } /** The `var` names a dead branch declares that its function (or file) - * declares nowhere else: replacing the branch must keep them bound, as - * undefined. Null when the branch declares a function in a block, whose - * binding outside the block depends on the file's strictness; such a - * branch is kept. */ -function hoistedNames(branch: ts.Node): readonly string[] | null { + * declares nowhere else outside the `removed` branches: replacing the + * branch must keep them bound, as undefined. Null when the branch declares + * a function in a block, whose binding outside the block depends on the + * file's strictness; such a branch is kept. */ +function hoistedNames(branch: ts.Node, removed: ReadonlySet): readonly string[] | null { let scope: ts.Node | undefined = branch.parent; while (scope !== undefined && !ts.isSourceFile(scope) && !functionScope(scope)) scope = scope.parent; @@ -702,7 +705,7 @@ function hoistedNames(branch: ts.Node): readonly string[] | null { const body = ts.isSourceFile(scope) ? scope : (scope as { body?: ts.Node }).body; if (body !== undefined) ts.walkPreorder(body, (n) => { - if (n === branch) return "skip"; + if (n === branch || removed.has(n)) return "skip"; if (n !== body && ts.isFunctionDeclaration(n) && n.name !== undefined) elsewhere.add(n.name.text); if (n !== body && (functionScope(n) || ts.isClassDeclaration(n) || ts.isClassExpression(n))) diff --git a/tests/fixtures/npm/cases/shape-overlay-facts/main.ts b/tests/fixtures/npm/cases/shape-overlay-facts/main.ts index 3ed1cf5de..2c83bcb85 100644 --- a/tests/fixtures/npm/cases/shape-overlay-facts/main.ts +++ b/tests/fixtures/npm/cases/shape-overlay-facts/main.ts @@ -1,6 +1,7 @@ // The shape overlay's facts: results that include undefined, literals that -// hold comment-like text, and a var declared in a branch no call takes. -import { banner, hoisted, lookup, lookupOrReturn } from "shape-facts"; +// hold comment-like text, a var declared in a branch no call takes, and +// a var declared in two such branches. +import { banner, hoisted, lookup, lookupOrReturn, shared } from "shape-facts"; console.log(lookup("b")); console.log(lookup("z")); @@ -8,3 +9,4 @@ console.log(lookupOrReturn("a")); console.log(lookupOrReturn("y")); console.log(banner(7)); console.log(hoisted("note:")); +console.log(shared("level:")); diff --git a/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.d.ts b/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.d.ts index b1865415d..12ff3f2d8 100644 --- a/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.d.ts +++ b/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.d.ts @@ -2,3 +2,4 @@ export declare function lookup(key: string): string; export declare function lookupOrReturn(key: string): string; export declare function banner(n: number): string; export declare function hoisted(label: string): string; +export declare function shared(label: string): string; diff --git a/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.js b/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.js index 505af75a0..8cfa73620 100644 --- a/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.js +++ b/tests/fixtures/npm/cases/shape-overlay-facts/node_modules/shape-facts/index.js @@ -1,6 +1,7 @@ // Functions whose results include the undefined of a fall-through or a -// bare return, comment-like text in literals, and a branch no call takes -// that declares a var read outside it. +// bare return, comment-like text in literals, a branch no call takes +// that declares a var read outside it, and two such branches that declare +// the same var. const entries = [ { key: "a", value: 1 }, @@ -53,3 +54,17 @@ function noteOf(extra) { export function hoisted(label) { return label + " " + noteOf(undefined); } + +function levelOf(warn, fail) { + if (warn) { + var level = "warn"; + } + if (fail) { + var level = "fail"; + } + return typeof level + " " + String(level); +} + +export function shared(label) { + return label + " " + levelOf(undefined, undefined); +} diff --git a/tests/harness/shape-overlay.test.ts b/tests/harness/shape-overlay.test.ts index 54bcfc1ce..6c843c7cb 100644 --- a/tests/harness/shape-overlay.test.ts +++ b/tests/harness/shape-overlay.test.ts @@ -63,7 +63,8 @@ test("shapeOverlay: false compiles without the overlay", { timeout: 900_000 }, a // shape-facts: results that include the undefined of a fall-through or a bare // return keep the caller's falsy branch, comment-like text in literals keeps -// its value, and a replaced branch keeps its var declared. +// its value, and a replaced branch keeps its var declared, also when two +// replaced branches declare the same var. const factsEntry = join(repoRoot, "tests/fixtures/npm/cases/shape-overlay-facts/main.ts"); test( @@ -78,7 +79,7 @@ test( throw new Error(result.diagnostics.map((d) => `${d.code}: ${d.message}`).join("\n")); const report = (result.warnings ?? []).find((w) => w.code === "SC6103"); expect(report?.message).toMatch(/of 1 package \(shape-facts\)/); - expect(report?.message).toMatch(/replaces 1 branch it proved unreachable/); + expect(report?.message).toMatch(/replaces 3 branches it proved unreachable/); const native = await execFileAsync(result.binaryPath, [], { cwd: dirname(factsEntry), encoding: "utf8",