ReviewOS

also looking at this

home-lang/home

fix(types): free for-loop and capture scopes after checking them

#939
Open mkantautas wants to merge fix/types-free-loop-scopes into main
2 files +22 -0
8 days ago

Problem

The type checker leaks the scope it creates for every for loop and every if (x) |v| capture:

fn main() {
  for x in [10, 20] {
    let next = x + 1
    print(next)
  }
}

A debug build of home check on this file prints three error(DebugAllocator): ... leaked reports. The ForStmt and IfLetStmt branches create a TypeEnvironment for the loop or capture bindings and later restore the enclosing environment with self.env = saved_env_ptr.* without freeing that scope.

Fix

Both branches call self.env.deinit() before restoring the enclosing environment, on the normal path and on the early-return error path, as the function, test, closure and match-arm scopes already do.

Tests

New checker frees for-loop and capture scopes in packages/types/tests/control_flow_checker_test.zig type-checks a for loop and an if (value) |v| capture with the testing allocator. On main it fails with leaked 6 allocations; with this change it passes. zig build test -Denable_jsc=false -Dfilter=types on macOS: 121/121. On Linux this test step does not compile on main or on this branch, because the parser's module resolver calls libc access and the test target does not link libc.

GitHub Actions does not run on this repo at the moment, so I ran the full suite on Linux x86_64 with the pinned Zig 0.17.0-dev.2163, on main (854d00f) and on this branch: zig build test --summary all records 9226 passes, 3 failures and 8 skips out of 9237 on both (the same 3 failing tests), 23 failed build steps on both, and the diagnostic snapshot harness output is identical. The built home check on the file above prints no leak reports on this branch.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.