Skip to content

Conversation

@MelbourneDeveloper
Copy link
Owner

@MelbourneDeveloper MelbourneDeveloper commented Oct 15, 2025

TLDR

Changed the circular dependency test from logging a warning (t.Log) to failing hard (t.Fatalf) when circular dependencies aren't detected, following the project's "NO PLACEHOLDERS" principle. Also fixed invalid Osprey syntax in the test code that was preventing the circular dependency detector from running.

What Was Added?

  • Enhanced error logging in TestFailsCompilationCircularDependency to always show the actual error message received
  • Proper test failure mechanism when circular dependencies aren't detected

What Was Changed / Deleted?

Changed Files:

compiler/tests/integration/compilation_test.go:

  • Changed t.Log() to t.Fatalf() when circular dependency detection fails (line 432)
  • Fixed invalid Osprey effect handler syntax:
    • !StateA, StateB![StateA, StateB] (added missing brackets for effect list)
    • with handler StateAhandle StateA ... in (corrected handler syntax)
    • getFromB()getFromB (removed invalid parentheses from handler operation names)
    • setInA(x)setInA x (corrected handler parameter syntax)
  • Added explicit error message logging to verify circular dependency detection works

vscode-extension/package.json:

  • Removed deprecated activationEvents section (6 lines) - VS Code now automatically activates based on contributes section

How Do The Automated Tests Prove It Works?

Test: TestFailsCompilationCircularDependency

  1. Compiles code with circular effect handlers:

    fn circularEffectA() -> int ![StateA, StateB] = {
        let bValue = perform StateB.getFromA()  // StateB handler calls...
        perform StateA.setInA(bValue + 1)
        perform StateA.getFromB()
    }
    
    fn circularEffectB() -> int ![StateA, StateB] = {
        let aValue = perform StateA.getFromA()  // StateA handler calls...
        perform StateB.setInB(aValue + 1)
        perform StateB.getFromA()
    }
    
    fn main() -> Unit = {
        handle StateA
            getFromB => circularEffectB()  // ← Circular: A calls B
        in handle StateB
            getFromA => circularEffectA()  // ← Circular: B calls A
        in {
            let result = circularEffectA()
        }
    }
    
  2. Verifies compilation fails (line 423-424)

  3. Checks error message contains "circular" or "recursion" (line 431-433)

    • BEFORE: Used t.Log() - test passed even when detection didn't work
    • AFTER: Uses t.Fatalf() - test fails hard if detection doesn't work
  4. Result: Test now properly validates that the compiler's circular dependency detection (implemented in internal/codegen/effects_generation.go:537-586) catches this pattern and reports:

    Circular effect dependency detected - effects cannot have circular references
    that would cause infinite recursion
    

All existing tests pass, confirming no regressions introduced.

Summarise Changes To The Spec Here

No spec changes required.

The circular dependency detection was already fully implemented and documented in /workspace/compiler/spec/0017-AlgebraicEffects.md. This PR only fixes the test to properly exercise and validate the existing implementation.

@MelbourneDeveloper MelbourneDeveloper merged commit 5e6edcd into main Oct 15, 2025
1 check passed
@MelbourneDeveloper MelbourneDeveloper deleted the fixcirculardeps branch October 15, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants