Skip to content
This repository was archived by the owner on Apr 29, 2026. It is now read-only.

fix repl use of aux#353

Merged
navicore merged 1 commit intomainfrom
fix-repl-aux
Feb 10, 2026
Merged

fix repl use of aux#353
navicore merged 1 commit intomainfrom
fix-repl-aux

Conversation

@navicore
Copy link
Copy Markdown
Owner

No description provided.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 10, 2026

PR Review: Fix REPL use of aux stack

Summary

This PR fixes a critical bug where the REPL was not properly initializing the auxiliary stack slot counts needed for code generation. The fix aligns the REPL's code path with the main compiler by extracting and setting aux_max_depths from the typechecker.


Code Quality ✅

Positive:

  • The fix is minimal and follows the existing pattern used in crates/compiler/src/lib.rs:298-309 and lib.rs:432-435
  • Variable naming is consistent with the rest of the codebase (aux_max_depths)
  • The placement is correct - the depths are extracted after type checking and set before codegen
  • The code properly integrates with the existing Issue RFC: Auxiliary stash stacks for complex stack juggling #350 infrastructure for aux stack support

Style conformance:

  • Follows the project's conventions for single-responsibility changes
  • No unnecessary abstractions or over-engineering
  • Clear and straightforward implementation

Correctness ✅

The implementation correctly:

  1. Extracts aux_max_depths from the typechecker (line 262)
  2. Sets them on the codegen instance before program compilation (line 264)
  3. Maintains the same order of operations as the main compiler

Verification: The fix mirrors the working implementation in crates/compiler/src/lib.rs, ensuring consistency across compilation paths.


Test Coverage ⚠️

Missing test for REPL:
While the main compiler has unit tests for aux stack functionality (test_aux_max_depths_tracked in crates/compiler/src/typechecker.rs:4717), there's no specific test for the REPL path.

Recommendation: Consider adding a REPL-specific test to prevent regression. The new example file examples/language/aux-stack.seq provides excellent test cases that could be adapted for REPL testing. For example:

#[test]
fn test_repl_aux_stack() {
    let source = ": test ( Int -- Int ) >aux aux> ;";
    let result = analyze_with_config(source, &CompilerConfig::default());
    assert!(result.errors.is_empty());
    assert!(result.llvm_ir.is_some());
}

Documentation ✅

Excellent example file:
The new examples/language/aux-stack.seq is exceptionally well-documented:

  • Clear explanation of aux stack semantics (word-local, LIFO, must be empty on return)
  • Progressive examples from simple to complex
  • Demonstrates real-world use cases (formatting, pattern matching, multiple stashes)
  • Includes both "with aux" and "without aux" comparisons
  • Well-structured with clear section headers

This example serves as both documentation and educational material for the aux stack feature.


Performance ✅

No performance concerns:

  • take_aux_max_depths() is already called in the main compiler path
  • The operation is O(1) - just moving a HashMap from typechecker to codegen
  • No additional allocations or computational overhead

Security ✅

No security concerns identified. The aux stack feature is:

  • Compiler-enforced (must be empty on word return)
  • Type-safe
  • Word-local (no cross-word contamination)

Overall Assessment

Verdict: Approved ✅

This is a clean, focused bug fix that:

  • Solves a real issue (REPL couldn't compile aux stack code)
  • Follows established patterns
  • Includes excellent documentation via the example file
  • Has no security or performance concerns

Minor suggestion: Add a REPL-specific test case to prevent future regressions.


References

@navicore navicore merged commit 0f24100 into main Feb 10, 2026
2 checks passed
@navicore navicore deleted the fix-repl-aux branch February 10, 2026 03:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant