Skip to content

builtins: route assert failures through g_has_error instead of exit(1) - #220

Merged
InauguralPhysicist merged 1 commit into
mainfrom
fix/assert-bypass-teardown
Jun 17, 2026
Merged

builtins: route assert failures through g_has_error instead of exit(1)#220
InauguralPhysicist merged 1 commit into
mainfrom
fix/assert-bypass-teardown

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Summary

  • builtin_assert called exit(1) on failure, skipping main's teardown (env_decref, gc_collect_at_exit, chunk_free). Standalone leaker test_assert_fail.eigs leaked the global env and every registered builtin — ~25 KB / 198 objects.
  • Swap exit(1) for g_has_error = 1 + g_error_msg snprintf + make_null(). CHECK_ERROR in vm_run unwinds to main, which runs the normal teardown and returns non-zero because g_has_error is set.
  • Stderr output (ASSERT FAIL: <msg>) and exit code (1) preserved.

Side effect

  • assert failures are now catchable by try/catch, matching throw. Previously exit(1) was uncatchable.

Numbers

  • Standalone leaker audit: 13 → 12.
  • Suite: 1882/1882.

Test plan

  • make asan && tests/test_assert_fail.eigs is leak-clean, exit 1, same stderr.
  • make asan && bash tests/run_all_tests.sh — 1882/1882, harness leak tally unchanged at 10.
  • Standalone-leaker audit drops by exactly one (test_assert_fail.eigs).

🤖 Generated with Claude Code

builtin_assert called exit(1) on failure, bypassing main's teardown
(env_decref, gc_collect_at_exit, chunk_free). Standalone leaker
test_assert_fail.eigs leaked the global env and every registered
builtin (~25 KB, 198 objects).

Setting g_has_error + g_error_msg and returning null lets CHECK_ERROR
unwind vm_run to main, which runs the standard teardown and exits
non-zero from g_has_error. Same stderr output, same exit code, no leak.

Side effect: assert failures are now catchable by try/catch, matching
throw. Previously exit(1) was uncatchable.

Standalone leaker count: 13 -> 12. Suite 1882/1882.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@InauguralPhysicist
InauguralPhysicist merged commit 14be586 into main Jun 17, 2026
11 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the fix/assert-bypass-teardown branch June 17, 2026 09:05
InauguralPhysicist added a commit that referenced this pull request Jun 17, 2026
- CHANGELOG [Unreleased]: document #220 behavior change (assert
  failures route through g_has_error and are catchable by try/catch;
  exit code + stderr unchanged for uncaught failures).
- docs/BUILTINS.md: assert row no longer says "Exit with message";
  now describes the catchable runtime error.
- CLAUDE.md: harness leak tally reflects the actual current count
  (10, down from 13 after PRs #212/#213/#214/#220) and the suite
  size is bumped to ~1882 to match.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
InauguralPhysicist added a commit that referenced this pull request Jun 18, 2026
All six observer predicates (converged/stable/oscillating/improving/
diverging/equilibrium) now read the dH window instead of the last step
(#204#209), plus catchable assert (#220). See CHANGELOG [0.16.0].

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant