build: host-only builtins move to one whole-TU-gated builtins_host.c (#741) - #812
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the #741 refactor to keep EIGENSCRIPT_FREESTANDING out of src/builtins.c by extracting all host-only builtins (filesystem, subprocess, terminal, POSIX regex, /dev/urandom, etc.) into a new whole-TU-gated translation unit (src/builtins_host.c). This aligns freestanding gating with the existing “linkable no-op registrar” pattern (e.g. ext_store.c) and makes host-dependency leaks show up as missing builtin_* symbols rather than scattered libc imports.
Changes:
- Added
src/builtins_host.ccontaining all host-only builtins, with a freestanding build compiling to a no-opregister_host_builtins()plus a resolver stub. - Updated
src/builtins.cto remove per-builtin#if !EIGENSCRIPT_FREESTANDINGblocks and to callregister_host_builtins(env)inside the builtin registration band; promotedreplay_blocks,g_argc, andg_argvto shared symbols. - Updated build + freestanding tooling + docs to compile/link the new TU consistently and document the new gating pattern.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/build.sh | Adds src/builtins_host.c to the WASM build source list. |
| tools/freestanding_smoke.sh | Includes src/builtins_host.c in the hosted freestanding-profile smoke build. |
| tools/freestanding_check.sh | Adds builtins_host to the per-TU freestanding symbol gate compilation set. |
| tools/embed_stack_soak.sh | Includes src/builtins_host.c in the freestanding-profile embedded-stack soak build. |
| src/builtins.c | Removes host-only builtin bodies/registrations; calls register_host_builtins; shares replay_blocks and argv globals. |
| src/builtins_internal.h | Declares register_host_builtins, replay_blocks, and the shared argv snapshot symbols. |
| src/builtins_host.c | New whole-TU-gated host-only builtin implementation + registrar and freestanding stubs. |
| Makefile | Adds src/builtins_host.c to the runtime SOURCES list to prevent drift. |
| docs/FREESTANDING.md | Documents the new whole-TU gating pattern for host-only builtins. |
| CHANGELOG.md | Adds an Unreleased entry describing the extraction and its motivation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…741) EIGENSCRIPT_FREESTANDING was carved into builtins.c inline — 52 #ifdef blocks gating ~1,500 lines, host-only builtins interleaved with portable ones. Everything needing a real OS (filesystem + the module resolver's host arm, exec_capture/proc_*, terminal raw mode, POSIX regex, streams, random_hex, build_corpus, read_file_util) now lives in builtins_host.c behind a single gate (the ext_store.c pattern: under the profile the TU is a no-op register_host_builtins plus the resolver stub). builtins.c keeps 5 conditional sites, all genuine dual-behavior carve-outs. A builtin that grows a host dependency now fails the freestanding symbol gate by NAME (undefined builtin_*) and the structural fix is "define it in the host TU" — verified by planted faults in both directions. Closes #741 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1863fdd to
91d52c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/builtins_host.c:604
- In build_corpus's "Top identifiers" debug output, the printed "uses" count can mismatch the printed name.
top_ids[i]is alwaysFIRST_IDENT + i, soidents[top_ids[i] - FIRST_IDENT].countindexesidents[i], but in frequency modetop_names[i]can come from a differentidents[best_idx]entry. This makes the diagnostic output incorrect/confusing.
Consider computing the count based on top_names[i] when printing (cheap since this loop prints at most 10 entries), rather than indexing idents via top_ids.
for (int i = 0; i < 10 && i < actual_top; i++) {
fprintf(stderr, " %3d %-20s %d uses\n", top_ids[i], top_names[i],
idents[top_ids[i] - FIRST_IDENT].count);
What does this PR do?
Closes #741 — the
builtins.chalf, which is the bulk of the finding. The 52 inlineEIGENSCRIPT_FREESTANDINGblocks (~1,500 gated lines) become one whole-TU-gated file,src/builtins_host.c, following the two patterns the issue pointed at:ext_store.c's TU gate with a linkable no-op registrar, and a single registration seam.builtins.ckeeps 5 conditional sites, all genuine dual-behavior carve-outs rather than whole host-only functions (glibcmallinfo2introspection ×2, thegetpidseeding term,try_parse's stderr fd-plumbing ×2).Closes #741
Changes
src/builtins_host.c(new, ~1,760 lines): every builtin needing a real OS — filesystem (mkdir/ls/getcwd/exe_path/chdir/mktemp/rm/file_exists/is_dir/rename/remove_file/read_bytes/read_bytes_buf/read_text/read_line/write_text/write_bytes/load_file), the module resolver's host arm (resolve_eigenscript_file_from+try_resolve_path/try_eigs_modules_walk), subprocess (exec_capture, theproc_*family), terminal raw mode (raw_key+ the termios statics, which were previously ungated), POSIX regex (match/match_all/regex_replace), streams,random_hex,build_corpus, andread_file_util. UnderEIGENSCRIPT_FREESTANDINGthe TU compiles to the no-opregister_host_builtinsplus the resolver stub (return 0— nothing resolves without a filesystem), exactly the behavior the inline gates produced.src/builtins.c: moved blocks deleted;register_builtinscallsregister_host_builtins(env)unconditionally inside the[0, g_builtin_binding_count)builtin band.replay_blocks(proc_* builtins are invisible to trace/replay — EIGS_REPLAY re-runs live subprocess side effects #148 boundary check) promoted to non-static — it's shared with the channel builtins that stay.g_argc/g_argvpromoted forexe_path's argv[0] fallback.src/builtins_internal.h: declarations for the three shared symbols + the registrar.MakefileSOURCES (whichbuild.shreads — anti-drift),tools/freestanding_check.sh,tools/freestanding_smoke.sh,tools/embed_stack_soak.sh,web/build.sh. (WASM compiles the host arm as before — emscripten's libc provided these all along.)What this buys
The gate failure mode changes from stray libc import to named builtin: a builtin registered from portable code whose definition lives (or belongs) in the host TU shows up in
freestanding_check.shstage 1 as an undefinedbuiltin_*symbol, and the structural fix is "define it inbuiltins_host.c" instead of "find and wrap the right lines in#ifdef".Registration-order note
Host builtins now register in one contiguous run instead of interleaved, so
--apioutput order shifts (membership identical — suite [131] asserts membership, not order; nothing pins the order).Testing
make, full suite): 3364/3364, exit 0.detect_leaks=1: 3368/3368, exit 0, zero LeakSanitizer/AddressSanitizer lines (floor 0 held).tools/freestanding_check.sh: both stages green, allowlist and HAL-roots untouched.tools/freestanding_smoke.sh: all pass (core language runs, carved surfaces fail loudly).tools/embed_stack_soak.sh: OK (104 steps in the 64 KiB stack).builtin_probe_host_leak. Reverted.fopen→ stage 1 FAILs namingfopen/fclose. Reverted.Follow-up
lint.c's 6 blocks (~950 gated lines: the E003 binding base,eigs_api_dump,check_undefined_names, and the dual-armeigs_lint_file) are a different shape — lint subsystems, not builtin groups — and get their own issue referencing this pattern.Checklist
make testpasses locally — 3364/3364 release, 3368/3368 ASan+UBSan leak-cleandocs/BUILTINS.md— n/a, no new builtins (pure move)docs/STDLIB.md— n/a🤖 Generated with Claude Code