ReviewOS

home-lang/home

fix(types): Home checker never checks return, match, block, assert, trait or impl statements

#778
Closed chris opened this 9 days ago ·
9 days ago

Problem

TypeChecker.checkStatement handles ~16 statement kinds and routes everything else to else => {} — including ReturnStmt, MatchStmt, BlockStmt, AssertStmt, TraitDecl, ImplDecl and ExtendDecl. A function declared -> i32 can therefore return a String, and anything inside a nested {} / unsafe {} block is unchecked. The function-body loop carries no return-type context. A second checker in codegen does implement checkReturnStmt, but native_codegen.typeCheck() has no callers.

Evidence (origin/main @ d4b219507)

Scope

  • Handle every statement kind in checkStatement (no silent else => {}).
  • Track the enclosing function's declared return type and check each return against it.
  • Fold the useful parts of packages/codegen/src/type_checker.zig into packages/types and delete the dead copy.

Acceptance criteria

  • home check rejects a wrong-typed return.
  • Type errors inside { ... } and unsafe { ... } blocks are reported.
  • A non-bool assert condition is rejected.
  • An impl method whose signature differs from its trait declaration is rejected.
  • Each case has a negative regression test.

Part of #814.

9 days ago

Checkpoint pushed to origin/main: c734d2c4f (fix(types): check returns blocks and assertions (#778)).

This batch removes three statement kinds from the checker's silent fallthrough:

  • tracks the enclosing function's declared return type and checks every ReturnStmt, including bare returns and returns nested in blocks;
  • recursively checks BlockStmt, including parser-produced unsafe { ... } blocks;
  • requires AssertStmt conditions to be boolean and still checks the optional message expression.

Added dedicated parser-to-checker regression suites for wrong/correct returns, nested unsafe-block diagnostics, non-boolean/boolean assertions, and invalid assert-message expressions. Both suites are wired into the umbrella build test filter.

Verification under the repository's single-process 3,840 MB guard:

zig build test -Dfilter=types_ -j1 --summary all
Build Summary: 5/5 steps succeeded
run-bounded: peak tree footprint 136 MB; host low-water 47%

git diff --check and AST checks also pass. The issue remains open: MatchStmt, TraitDecl, ImplDecl, and ExtendDecl still need first-class handling, and the dead duplicate codegen checker still needs consolidation/removal.

9 days ago

Second checkpoint pushed to origin/main: 1978095ec (fix(types): validate match statements and patterns (#778)).

MatchStmt now receives first-class checking: scrutinee inference, pattern/type validation, per-arm pattern-binding scopes, boolean guard checks, arm-body checks, and exhaustiveness diagnostics. The new coverage proves invalid guards/bodies, wrong-type patterns, non-exhaustive boolean matches, and valid identifier bindings.

The regression tests also exposed two ownership defects, fixed in the same logical path rather than suppressed: function-local type environments are now deinitialized on every exit path, and match-pattern ASTs are recursively destroyed. Identifier-vs-range parser backtracking now frees its speculative expression as well.

Guarded verification:

zig build test -Dfilter=types_ -j1 --summary all
10/10 tests passed; zero leaks; peak 613 MB

zig build test -Dfilter=parser -j1 --summary all
1058/1058 tests passed; peak 448 MB

Authorship and origin/main were verified at the full SHA. Remaining #778 scope is trait/impl/extend checking plus removal of the dead duplicate codegen checker.

9 days ago

Third checkpoint pushed to origin/main: a821d1ac7 (fix(types): validate trait implementations (#778)).

The authoritative checker now registers trait declarations before the checking pass, diagnoses undefined super-traits and undefined impl traits, validates required methods and method signatures (name, arity, asyncness, parameter types, and return types), rejects extra impl methods, checks impl method bodies, and checks extension method bodies. Focused regressions cover the requested mismatched impl signature and an invalid extension body.

The new tests exposed previously unreachable AST ownership leaks. TraitDecl, ImplDecl, and ExtendDecl now receive recursive teardown, including methods, parameters, type expressions, generic parameters, associated types, where clauses, and function bodies.

Guarded verification:

zig build test -Dfilter=types_ -j1 --summary all
12/12 tests passed; zero leaks; peak 598 MB

zig build test -Dfilter=parser -j1 --summary all
all selected parser artifacts passed; direct parser 145/145; peak 287 MB

The full SHA and sole author/committer identity were verified on origin/main. Final #778 consolidation remains: remove the uncalled duplicate checker and its uncalled NativeCodegen.typeCheck() entry point, then replace the remaining silent statement fallback with an exhaustive classification.

9 days ago

Completed and pushed directly to origin/main in four reviewable commits:

  • c734d2c4f — check returns, blocks, and assertions; add focused checker artifacts.
  • 1978095ec — validate match patterns, bindings, guards, bodies, and exhaustiveness; fix function-scope and match-pattern ownership.
  • a821d1ac7 — register/check traits, validate impl signatures and required/extra methods, check impl/extend bodies, and complete trait AST teardown.
  • 4f72dd59d — remove the uncalled 747-line codegen checker and NativeCodegen.typeCheck(), document the single semantic authority, and make checkStatement exhaustive instead of silently swallowing future variants.

Acceptance audit:

  • wrong return type rejected: covered;
  • nested block / unsafe diagnostics reported: covered;
  • non-boolean assert rejected: covered;
  • impl method signature mismatch rejected: covered;
  • negative tests added and wired into the umbrella build: covered;
  • every statement union variant explicitly classified; no else => {} remains in checkStatement;
  • useful return/block behavior consolidated into packages/types; dead duplicate removed.

Final guarded evidence:

types checker: 12/12 passed, zero leaks (587 MB peak)
codegen:       12/12 passed (180 MB peak)
parser sweep:  selected artifacts green; direct parser 145/145

Earlier match/parser ownership verification also passed 1,058/1,058 selected parser-family tests. All heavy runs used the 3,840 MB ceiling and global lock. Each commit has 3–5 files, and all four remote commits have sole author and committer glennmichael123 <gtorregosa@gmail.com>.

Sign in to comment on this issue.