Skip to content

Delete the unused draft FFI host function implementation - #37362

Open
robobun wants to merge 11 commits into
claude/split/install-clifrom
farm/df8b29f2/split-ffi
Open

Delete the unused draft FFI host function implementation#37362
robobun wants to merge 11 commits into
claude/split/install-clifrom
farm/df8b29f2/split-ffi

Conversation

@robobun

@robobun robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

What this does

ffi/host_fns.rs was an earlier draft of generate_symbols / generate_symbol_for_function / Function::print_source_code that ffi/mod.rs still declared and re-exported, but the live implementations are the ones in ffi_body.rs, which is what every caller uses. This deletes the draft and the Function/Step/Compiled struct copies in mod.rs that only it referenced.

Split 5 of 8 from #32023, which alii asked to be broken up. Pure code motion and deletion; no intended behavior change. Stacked on claude/split/install-cli (merge order: foundations → install-cli → these, in any order among themselves).

Verification

  • Compiles in isolation against the base (cargo check), and the union of all eight splits is byte-identical to Deduplicate jsc bindings, runtime api, test matchers, and shell builtins #32023's head f29ce8fe4, which CI ran at 285/286 green (the one red was the fleet-wide napi.test.ts GC-timing flake on windows x64-baseline).
  • Locally on the parent head: ffi.test.js passes. The deleted file had no callers (grep for its exports resolves only to the ffi_body definitions).
  • No new tests: this adds no behavior, so the existing suites above are the regression coverage.

alii and others added 11 commits June 9, 2026 13:19
Resolved src/io/posix_event_loop.rs: #32594 added PollMemoryPressure to
is_registered(), which this branch moved into impl_file_poll_flag_methods;
carried the flag into the macro body.

Restored ArrayHashMap::get_index_mut: gained a caller in
react_compiler/collections.rs via #32504.
Resolved src/bun_core/string/mod.rs: #33909 switched return types to
crate::CrateResult<()> in write_pre_quoted_string and quote_for_json,
which this branch restructures into the const-generic
write_pre_quoted_string_inner; carried CrateResult<()> into both the
adapter and _inner signatures.

Resolved src/router/lib.rs and src/sourcemap/Chunk.rs: #33909 touched
return types inside blocks this branch deletes (orphaned router test
scaffolding, uncalled print_source_map_contents); kept the deletions
after re-verifying zero callers on current main.

Restored Node::find_last in src/collections/pool.rs: gained a test
caller via #33311.
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 5:05 AM PT - Aug 11th, 2026

@alii, your commit 5b3e978 has some failures in Build #91737 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 37362

That installs a local version of the PR into your bun-37362 executable, so you can run:

bun-37362 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — pure dead-code deletion of a duplicate draft implementation.

Checked that every deleted item (Function/Step/Compiled, generate_symbols, generate_symbol_for_function, print_source_code, print_callback_source_code, Drop for Function, FFICallbackFunctionWrapper_destroy extern) has a live equivalent in ffi_body.rs, and that no code outside ffi/ referenced the removed mod.rs re-exports or types.
The deleted LIB_DIR_Z static has zero references anywhere in the tree.
The non-ffi host_fns grep hits (Response.rs, server/) are unrelated identifiers.

Extended reasoning...

Overview

This PR deletes src/runtime/ffi/host_fns.rs (469 lines) and removes ~120 lines of draft struct definitions from src/runtime/ffi/mod.rs. The deleted code was an earlier draft of the FFI Function/Step/Compiled types and the generate_symbols / print_source_code helpers. The live implementations of all of these are in ffi_body.rs, which is what mod.rs actually wires up (pub use ffi_body::FFI) and what all callers use. This is split 5/8 of #32023, broken up at maintainer request.

Security risks

None. This is a pure deletion with no new code paths, no changes to input handling, and no changes to the live FFI implementation in ffi_body.rs.

Level of scrutiny

Low-to-medium. Dead code deletion is mechanical, but FFI is a sensitive subsystem, so I verified the "dead" claim rather than trusting it:

  • Grepped for every deleted public item. All generate_symbols / generate_symbol_for_function call sites are inside ffi_body.rs and resolve to the definitions at ffi_body.rs:1707/:1846, not the deleted re-export.
  • Grepped for ffi::(Function|Step|Compiled|generate_symbol|LIB_DIR_Z) across src/ — no hits. Nothing outside the module referenced the draft types.
  • Confirmed ffi_body.rs has its own Function (line 1890), Step (2524), Compiled (2533), Drop for Function (1920, byte-identical cleanup logic), needs_handle_scope/needs_napi_env/ffi_header/print_source_code/print_callback_source_code, and its own FFICallbackFunctionWrapper_destroy extern declaration.
  • LIB_DIR_Z has zero references repo-wide.
  • The removed use lines in mod.rs (c_char, c_void, NonNull, ZBox, JSGlobalObject, bun_tcc_sys as TCC) were only used by the deleted structs.
  • The apparent host_fns hits in Response.rs and server/ are unrelated (_jsc_host_fns, exported_host_fns).

The two Function types coexisted without conflict because host_fns.rs imported super::Function (the draft in mod.rs), while ffi_body.rs defines and uses its own — they were genuinely parallel, and only the ffi_body one is reachable from the FFI class that JS actually sees.

Other factors

The PR is part of a maintainer-requested split of an already-reviewed larger PR, with the union verified byte-identical to the CI-green parent. The remaining mod.rs is a straightforward re-export hub with an updated doc comment. No test changes are expected for a pure deletion; ffi.test.js covers the surviving path.

@robobun
robobun force-pushed the claude/split/install-cli branch from 2f5dd36 to 9ae3ff5 Compare August 11, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants