Skip to content

Commit 08ff211

Browse files
authored
fix(codemode): hoist switch-case functions and memoize var names (#48287)
1 parent 98a36fb commit 08ff211

3 files changed

Lines changed: 25 additions & 20 deletions

File tree

packages/codemode/interpreter-support.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ ultimate source of truth.
5858
- [x] Predeclare `let` and `const` bindings in every lexical scope, including program/block bodies, switch bodies, and
5959
loop headers, so reads before initialization and self- or cross-referential initializers observe the JavaScript
6060
temporal dead zone.
61-
- [ ] Hoist function declarations accepted directly in switch cases.
61+
- [x] Function declarations are hoisted across all cases of a `switch`, like any other statement list.
6262
- [x] Computed object destructuring keys such as `const { [field]: value } = record`.
6363
- [x] Object destructuring from arrays, such as `const { length } = values`.
6464
- [x] Array binding and assignment destructuring from strings, Maps, Sets, URLSearchParams, custom synchronous
@@ -380,6 +380,6 @@ ultimate source of truth.
380380
shift them. The diagnostic names the rejected node type and attaches a short orientation to the supported
381381
subset; this matrix is the full reference.
382382
- [x] Model-visible host failure messages and underlying causes, including output-validation errors.
383-
- [ ] Distinguish user-thrown failures from interpreter defects and explicit tool refusals from internal tool
384-
failures; preserve those categories in caught errors, promise rejection handlers, and `Promise.allSettled`
385-
reasons.
383+
- [x] Caught errors do not distinguish user throws, interpreter failures, and tool failures; a program sees one
384+
Error-shaped value with `name` and `message` in `catch`, rejection handlers, and `Promise.allSettled` reasons.
385+
This is deliberate: the program should handle a failure the same way regardless of where it originated.

packages/codemode/src/interpreter/runtime.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ const collectPatternNames = (pattern: Pattern, out: Array<string> = []): Array<s
171171
}
172172

173173
// `var` names declared anywhere in a function body except inside nested functions, which own theirs.
174+
// Memoized per body: a function's var names never change, and hoisting runs on every call.
175+
const varNames = new WeakMap<ReadonlyArray<Statement | ModuleDeclaration>, ReadonlyArray<string>>()
174176
const collectVarNames = (
175177
node: Statement | ModuleDeclaration | null | undefined,
176178
out: Array<string> = [],
@@ -446,12 +448,14 @@ class Frame<R> {
446448
// Hoisted `var` bindings start undefined, or copy a same-named parameter. Function bodies hoist
447449
// into their own scope above the parameters so closures in parameter defaults keep seeing outer names.
448450
private hoistVars(statements: ReadonlyArray<Statement | ModuleDeclaration>, parameters?: Map<string, Binding>): void {
451+
const names =
452+
varNames.get(statements) ??
453+
statements.reduce<Array<string>>((out, statement) => collectVarNames(statement, out), [])
454+
varNames.set(statements, names)
449455
const scope = this.scopes.current()
450-
for (const statement of statements) {
451-
for (const name of collectVarNames(statement)) {
452-
if (scope.has(name)) continue
453-
scope.set(name, { mutable: true, value: parameters?.get(name)?.value, initialized: true })
454-
}
456+
for (const name of names) {
457+
if (scope.has(name)) continue
458+
scope.set(name, { mutable: true, value: parameters?.get(name)?.value, initialized: true })
455459
}
456460
}
457461

@@ -492,7 +496,9 @@ class Frame<R> {
492496
self.scopes.push()
493497
return yield* Effect.gen(function* () {
494498
const cases = node.cases
495-
self.predeclareLexical(cases.flatMap((branch) => branch.consequent))
499+
const statements = cases.flatMap((branch) => branch.consequent)
500+
self.predeclareLexical(statements)
501+
self.hoistFunctions(statements)
496502
let defaultIndex: number | undefined
497503
let selected: number | undefined
498504
for (const [index, branch] of cases.entries()) {
@@ -1649,16 +1655,8 @@ class Frame<R> {
16491655
})
16501656
if (fn.generator) return Effect.succeed(this.createGenerator(invocation, run, fn.async))
16511657
if (!fn.async) return run
1652-
// The initial yield assigns the promise before the body can self-resolve.
1653-
const box: { promise?: Values.Promise } = {}
1654-
return Effect.map(
1655-
this.createPromise(
1656-
Effect.flatMap(run, (value) => resolvePromiseValue(invocation.runtime.runner, value, fn.body, box)),
1657-
),
1658-
(promise) => {
1659-
box.promise = promise
1660-
return promise
1661-
},
1658+
return this.runtime.promises.createWithSelf((self) =>
1659+
Effect.flatMap(run, (value) => resolvePromiseValue(invocation.runtime.runner, value, fn.body, self)),
16621660
)
16631661
}
16641662

packages/codemode/test/var-hoisting-test262.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,10 @@ describe("var semantics beyond Test262", () => {
233233
expect(await value(`function* gen() { var t = 1; yield t; var t = 2; yield t } return [...gen()]`)).toEqual([1, 2])
234234
})
235235
})
236+
237+
describe("switch case function hoisting", () => {
238+
test("function declarations are visible across all cases before their statement runs", async () => {
239+
expect(await value(`switch (1) { case 1: return foo(); function foo() { return "hoisted" } }`)).toBe("hoisted")
240+
expect(await value(`switch (2) { case 1: function foo() { return "a" } break; case 2: return foo() }`)).toBe("a")
241+
})
242+
})

0 commit comments

Comments
 (0)