feat: Add experimental closures support with feature flag

[BlobMaster41]

Fixes #798.
Related: #173, #563, #2054, #2753.

Changes proposed in this pull request:

⯈ Added experimental closures feature - Closures can now capture variables from their enclosing scope. This includes support for:

• Capturing parameters and local variables (both let and var)
• Capturing this directly in class methods
• Read and write access to captured variables (reference semantics)
• Multiple closures sharing the same environment
• Deeply nested closures (capturing from grandparent scopes)
• Higher-order functions (map, filter, reduce patterns)
• Factory patterns, memoization, and other functional programming idioms

⯈ Implemented as an opt-in feature flag - Closures are disabled by default to maintain backwards compatibility and expected behavior. Users must explicitly enable the feature with --enable closures. This ensures:

• No changes to existing code behavior
• No additional runtime overhead for code that doesn't use closures
• Indirect calls without closures enabled use simpler codegen

⯈ Added compile-time constant ASC_FEATURE_CLOSURES - Allows conditional compilation based on whether closures are enabled

⯈ Added comprehensive test suites:

• closure.ts - Basic closure patterns (captures, mutations, shared environments)
• closure-stress.ts - Stress tests covering many edge cases (616 lines)
• closure-class.ts - Complex class patterns with closures (1000+ lines) including:
  • State management (BankAccount, Counter)
  • Design patterns (Builder, Factory, Observer, State Machine, Iterator)
  • Inheritance with closures
  • Callback-based "async" patterns
  • Tree traversal with recursive closures

Implementation Details

The implementation follows the approach discussed in #798:

Closure Environment:

• Captured variables are stored in a heap-allocated environment structure
• Multiple closures in the same scope share a single environment (reference semantics)
• Nested closures maintain a chain of environments via parent pointers
• Environment structures are managed by the runtime (GC-compatible)

First-Class Functions:

• Function references contain both a table index and an _env pointer
• When calling through a function reference, the _env is loaded and made available
• Non-closure functions have _env = 0, avoiding overhead when closures aren't used

Core changes in src/compiler.ts:

• Pre-scan phase (prescanForClosures) identifies closures and captured variables before compilation
• Environment allocation creates heap-allocated storage for captured variables
• Closure load/store operations route through the environment
• Dynamic Function objects are created with environment pointers for closures
• Feature checks gate closure functionality at detection points
• Indirect calls only set up closure environment handling when the feature is enabled

Supporting changes:

• std/assembly/shared/feature.ts - Added Feature.Closures enum value
• src/common.ts - Added ASC_FEATURE_CLOSURES constant name
• src/program.ts - Registered compile-time constant
• src/index-wasm.ts - Exported FEATURE_CLOSURES for CLI
• src/flow.ts - Added flow tracking for captured variables

Limitations

This is an experimental implementation. Known limitations:

• Performance: Each closure creation involves a heap allocation for the environment

Usage

# Closures disabled (default) - captures produce an error
asc myfile.ts

# Enable closures explicitly
asc myfile.ts --enable closures

// Example closure usage (requires --enable closures)
function makeCounter(): () => i32 {
  let count = 0;
  return (): i32 => {
    count += 1;
    return count;
  };
}

let counter = makeCounter();
counter(); // 1
counter(); // 2
counter(); // 3

• I've read the contributing guidelines
• I've added my name and email to the NOTICE file

[BlobMaster41]

Please review.

@dcodeIO @jtenner @willemneal @MaxGraey

[CountBleck]

Was this primarily written using an LLM? (This itself doesn't make this PR bad, but this is code we'd have to maintain for quite a while after merging, and the existence of a human who fully understands what's going on here is ideal.)

[BlobMaster41]

Was this primarily written using an LLM? (This itself doesn't make this PR bad, but this is code we'd have to maintain for quite a while after merging, and the existence of a human who fully understands what's going on here is ideal.)

This code was written primarily by an human. The only part where LLM was used is in the unit test to generate an unique set of easy to complex cases with edge cases an human might not think of. You can check my org btc-vision and what we do. We do not use LLM in critical features. AI is trash at coding in general.

[BlobMaster41]

Yes I did use copilot auto feature to give a summary of the PR content if you wonder. Everything said here is accurate.

[CountBleck]

Thanks. It seemed somewhat high-quality for an LLM, and the PR description threw me off.

This is a big chance, so I'm not too confident in merging it immediately. I'll be slow to review, especially because I'm busy until next week or so.

Could you try running some of the larger, existing compiler tests with this feature on, to see if they run correctly (obviously, the WAT output will be different, so use --create and discard the changes afterward). Also, did you make sure that all locals stored in closure environments are visited by the GC? (Sprinkling in a few __collect() calls might reveal some issues. Running those aforementioned tests should also do the same.)

Anyway, some additional things from taking a cursory glance at the PR...

[CountBleck]

This is a big switch-case, but I suppose it makes sense. It might warrant some scrutiny to make sure there's nothing missing.

[dcodeIO]

Should information gathering of this kind be integrated into the resolve step perhaps, as it basically is the pre-pass?

[BlobMaster41]

This is a big switch-case, but I suppose it makes sense. It might warrant some scrutiny to make sure there's nothing missing.

To help you understand what is supported and what is NOT supported, I made a little table:

Fully supported

Node Type	Notes
All expression nodes	Identifier, Binary, Call, New, Ternary, PropertyAccess, ElementAccess, Assertion, InstanceOf, Parenthesized, UnaryPrefix, UnaryPostfix, Comma
All control flow	If, While, Do, For, ForOf, Switch (with cases), Block
Exception handling	Try/Catch/Finally, Throw
Literals	Array, Object, Template (elements/values scanned)
Variable declarations	Variable, VariableDeclaration
Function expressions	Including nested closures with proper parameter shadowing
Parameter default values	(x: i32 = captured) => x works correctly
this capture	Properly detected and stored in environment (view tests)

Explicitly not supported (errors before reaching closure analysis)

Feature	Error
Class expressions	"Not implemented: Block-scoped class declarations or expressions"
Computed property keys	"Identifier expected" (parse error)
super in closures	"'super' can only be referenced in a derived class"
Regular expressions	"Not implemented: Regular expressions"

And here are the intentionally skipped since there is no capture possible:

1. Leaf nodes: Null, True, False, Super, Constructor, Break, Continue, Empty, Omitted, Comment
2. Type nodes: TypeName, NamedType, FunctionType, TypeParameter, Parameter
3. Top-level declarations: Source, ClassDeclaration, EnumDeclaration, FunctionDeclaration, InterfaceDeclaration, NamespaceDeclaration, TypeDeclaration, Import, Export, etc.

So I changed the way it was before to safety first checks. Now the default behavior is to throw (scanNodeForCaptures/collectDeclaredVariables/prescanNodeForFunctionExpressions): unhandled node kind: ... so any unhandled node type will immediately fail rather than silently skip captures.

[BlobMaster41]

Should information gathering of this kind be integrated into the resolve step perhaps, as it basically is the pre-pass?

Basically, I strongly belive that the current architecture is correct since locals don't exist during resolve. Also, capture analysis needs flow.lookupLocal() and flow.lookupLocalInOuter() which require the parent flow chain to be established. Lastly, the environment layout needs local.type.byteSize which requires compiled types.

However, compileFunctionBody uses prescanForClosures() which is a name-based analysis (Map<string, null>) and compileFunctionExpression uses analyzeCapturedVariables() which is a local-based analysis (Map<Local, i32>).

I strongly belive I should refactor this and create a function scanForCaptures that accepts a "capture collector" interface which support multiple mode:

1. Name collection mode (used during prescan when Locals don't exist yet)
2. Local resolution mode (used when compiling function expressions)

This will be a better design. I will update the PR shortly. It a big refactor but doable.

[BlobMaster41]

Managed to refactor this section and remove about 400 lines of code. This is way better!

[dcodeIO]

(Heh, started typing this before I saw your second comment)

Fwiw, looking through the existing code again, I think the previous plan was something along these lines:

As it stands, it is already possible to resolve an outer local without doing a separate pre-scan. In compileAssignment for example, that's the target local, where a captured local lives in a different (here: outer) flow. Right now this errors as it's not implemented.

Idea there was, that at the moment the compiler sees such a local, it can mark the outer local as captured, create or update an append-only closure environment of the respective outer function, and emit an env load or store instead.

When seeing such a local again, i.e. it is marked captured, the compiler can emit the env load or store directly.

This way, the discovery can happen as part of compilation without a separate pre-scan, perhaps even without recompiling anything and instead modifying the signature of closure-producing functions plus prepending a prologue to allocate the respective closure environment - or whatever fits best.

Also, one assumption there is that compilation of the inner function happens and completes mid-compilation of the outer function, producing the information the outer functions needs to be finalized before it is finalized - so it can either do something clever to upgrade to a closure producing function mid-compilation, or be fixed up without a full recompile. Well possible that this assumption still holds.

So, in summary, closure implementation was planned in a way that avoids doing pre-passes, with some support functionality to build it this way already present. Would be cool if there's way to reuse some of that!

[BlobMaster41]

(Heh, started typing this before I saw your second comment)

Fwiw, looking through the existing code again, I think the previous plan was something along these lines:

As it stands, it is already possible to resolve an outer local without doing a separate pre-scan. In compileAssignment for example, that's the target local, where a captured local lives in a different (here: outer) flow. Right now this errors as it's not implemented.

Idea there was, that at the moment the compiler sees such a local, it can mark the outer local as captured, create or update an append-only closure environment of the respective outer function, and emit an env load or store instead.

When seeing such a local again, i.e. it is marked captured, the compiler can emit the env load or store directly.

This way, the discovery can happen as part of compilation without a separate pre-scan, perhaps even without recompiling anything and instead modifying the signature of closure-producing functions plus prepending a prologue to allocate the respective closure environment - or whatever fits best.

Also, one assumption there is that compilation of the inner function happens and completes mid-compilation of the outer function, producing the information the outer functions needs to be finalized before it is finalized - so it can either do something clever to upgrade to a closure producing function mid-compilation, or be fixed up without a full recompile. Well possible that this assumption still holds.

So, in summary, closure implementation was planned in a way that avoids doing pre-passes, with some support functionality to build it this way already present. Would be cool if there's way to reuse some of that!

So if I understand correctly, this is a more "discover as you go" architecture. Let me give it a try. I will probably be unable to finish this today but let me give it a try!

[BlobMaster41]

So I guess the tricky part would be about code that already compiled before we discovered the capture?

For example:

function outer() {
	let x = 1;        // Compiles: local.set $x
	x = x + 1;        // Compiles: local.get $x, local.set $x
	let f = () => x;  // NOW we discover x is captured!
	x = x + 1;        // Should use: env.load, env.store
}

When we hit the inner function, the outer function has already emitted local.get/set for x. But for correctness, all accesses to captured variables (even in the declaring function) must go through the environment.

So I guess prepend a prologue that copies locals to env, but this only works if outer function doesn't modify the captured variable after creating the closure.

Should we do a more like hybrid approch where we could use the existing support (flow.outer, lookupLocalInOuter, declaredByFlow) lets us detect outer accesses? Let me try this...

[BlobMaster41]

So the algorithm flow would be something like this?

flowchart TD
    A[compileFunctionBody instance] --> B{bodyContainsFunctionExpressions?}
    B -->|yes| C[instance.mayHaveClosures = true]
    B -->|no| D[numLocalsBefore = localsByIndex.length]
    C --> D
    
    D --> E[compileStatements body]
    
    E --> F[compileExpression identifier]
    F --> G{local.isCaptured?}
    G -->|no| H[local.wasAccessedAsLocal = true]
    G -->|yes| I[skip]
    H --> J
    I --> J
    
    E --> J[compileFunctionExpression]
    J --> K[analyzeCapturedVariables]
    
    K --> L[for each capture]
    L --> M[local.isCaptured = true]
    M --> N{local.wasAccessedAsLocal?}
    N -->|yes| O[local.envOwner.needsCaptureRecompile = true]
    N -->|no| P[allocate envSlotIndex]
    O --> P
    
    P --> Q{instance.needsCaptureRecompile?}
    Q -->|yes| R[reset locals to numLocalsBefore]
    R --> S[clear wasAccessedAsLocal flags]
    S --> T[instance.needsCaptureRecompile = false]
    T --> A
    
    Q -->|no| U[emit environment allocation if needed]
    U --> V[done]

Loading

Some edge case with this solution that would still need recompilation:

Case	Recompile?	Notes
let x=1; let f=()=>x; x=2	YES	x accessed before closure
let f=()=>x; let x=1; x=2	NO	x declared after closure
let x=1; let f=()=>x;	YES	x initialized before closure
Nested closures	Each level handles independently
Multiple closures	capturedLocals accumulates all

Is this a cleaner approch?

[dcodeIO]

Would say that possible solutions there could be to a) emit fixup code that lifts the local to the environment at the time it's needed, b) to walk already generated code backwards and replace local.get/set of the local in question with environment accesses or c) to schedule a recompile of affected functions. Might well be that c) is the way to go if it significantly reduces complexity.

Apart from that, one interesting case is compile time branch elimination

function add<T>(a: T, b: T): T {
  let c: T;
  if (isString<T>()) {
    let concat = () => a + b;
    c = concat();
  } else {
    c = a + b;
  }
  return c;
}

where pre-scanning would detect a closure, even though for certain T there is none. Iirc that was part of the motivation for a demand-driven design.

[BlobMaster41]

Thanks. It seemed somewhat high-quality for an LLM, and the PR description threw me off.

This is a big chance, so I'm not too confident in merging it immediately. I'll be slow to review, especially because I'm busy until next week or so.

Could you try running some of the larger, existing compiler tests with this feature on, to see if they run correctly (obviously, the WAT output will be different, so use --create and discard the changes afterward). Also, did you make sure that all locals stored in closure environments are visited by the GC? (Sprinkling in a few __collect() calls might reveal some issues. Running those aforementioned tests should also do the same.)

Anyway, some additional things from taking a cursory glance at the PR...

Hey, sorry for the delay! I will review all the comments now.

[BlobMaster41]

I need to refactor the code again a bit since npm run bootstrap:debug has errors with null types due to lazy initialisation. I did not see this before.

[dcodeIO]

Looks like most of these can be fixed by modifying the pattern

if (foo.bar && foo.bar.baz) { ... }

to

let bar = foo.bar;
if (bar && bar.baz) { ... }

Has something to do with side-effects, where if .bar is a getter, it could modify state so the next .bar access is no longer non-null. One of the places where TS is unsound.

[BlobMaster41]

Looks like most of these can be fixed by modifying the pattern

if (foo.bar && foo.bar.baz) { ... }

to

let bar = foo.bar;
if (bar && bar.baz) { ... }

Has something to do with side-effects, where if .bar is a getter, it could modify state so the next .bar access is no longer non-null. One of the places where TS is unsound.

Yea, I got rid of the Map<string, null> and used a set instead. Im using Map<Local, i32> as well now for capturedLocals instead of generic maps since I have to assign it to a variable before to avoir the error. I mean, its more type strict so its good I guess.

I prefered the implementation before because it was shorter but yea, my changes are "safer" for null checking now I guess.