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!

[CountBleck]

Did you write this line? Regardless, maybe stick an assert(false) in there?

[BlobMaster41]

Yes, I did write this, I did not add scanning for While, Do, For, For of, switch, try, throw, etc. I will add it and update the PR. Even if AS do not support try catch, I will add these so when the implementation of try catch is done we do not have to modify the code to add it.

I didnt add an error here because a lot of node types like Break, Continue, Empty, True, False, Null, Super, Omitted don't contain variable references so adding an error there would error the compile all the time.

I will add all the node types to the switch case and add the assert.

[BlobMaster41]

After refactoring the method like mentioned in my other message, I managed to get rid of this entire switch case. We no longer need any of this anymore! Great!

[CountBleck]

Nitpick: extra $

[BlobMaster41]

Fixed.

[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.