docs(ffi): document viewSource, read.intptr, void/usize/isize and the cc include option; call CFunction without new - #38386
docs(ffi): document viewSource, read.intptr, void/usize/isize and the cc include option; call CFunction without new#38386robobun wants to merge 1 commit into
Conversation
…lude; drop new from CFunction The remaining items from #31534 that the ffi.mdx rewrite in #35246 did not cover: - FFIType tables (ffi.mdx and c-compiler.mdx): add the void row and the usize/isize aliases of u64/i64; note that void is returns-only. - CFunction is a factory, so both examples call it without new. - Add a short viewSource section describing what it prints now that only cc() generates C. - Add read.intptr to the read table. - c-compiler.mdx: document the include option of cc().
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: docs only, ready for review.
|
There was a problem hiding this comment.
LGTM — docs-only additions to the FFI pages that match the current implementation.
What was reviewed:
usize/isize→Uint64T/Int64Tandvoidas an ABI type confirmed insrc/runtime/ffi/abi_type.rs.CFunctionis declaredfunction CFunction(options)insrc/js/bun/ffi.ts, so droppingnewis correct.read.intptrexists inFFIObject.rsandffi.d.ts;viewSourceis exported fromsrc/js/bun/ffi.ts.cc()readsoptions.includeinsrc/runtime/ffi/ffi_body.rsand passes each entry toadd_sys_include_path.
Extended reasoning...
Overview
Docs-only change to two pages: docs/runtime/ffi.mdx and docs/runtime/c-compiler.mdx. Adds isize/usize/void rows to the FFIType tables, drops new from the two CFunction examples (with a sentence explaining why), adds a short viewSource section, adds read.intptr to the read table with a note distinguishing it from read.ptr, and adds the include option to the cc() reference on the C Compiler page. No runtime, type-declaration, or test changes.
Security risks
None. Documentation only; no code paths are touched.
Level of scrutiny
Low. Docs pages for an experimental module. The relevant question is whether the new prose matches the implementation, and each claim was spot-checked against source: abi_type.rs maps "usize"→Uint64T and "isize"→Int64T and defines Void; src/js/bun/ffi.ts defines function CFunction(options) (not a class) and exports viewSource; FFIObject.rs and ffi.d.ts both define read.intptr; ffi_body.rs reads options.include and feeds it to TinyCC's include path list. The PR description records that each behavioral claim was also verified by running it, and the wording is consistent with what the source shows.
Other factors
No CODEOWNERS entry covers docs/. No prior human review comments to address. The tables remain aligned and the prose follows the existing voice of the page. bun run prettier --check was reported clean by the author. The bug-hunting pass found nothing.
Closes #31534. Supersedes #31535, which carried the same docs fixes plus runtime changes that have since landed or become moot (see below).
Problem
bun:ffi#31534 lists gaps betweendocs/runtime/ffi.mdxand whatbun:ffiexports. bun:ffi: use the engine-native FFI when available #35246 rewrote the page and covered some of them:cc()is linked to its own page,intis in theFFITypetable, andnapi_env/napi_valuenow have a paragraph and are markedcc()only in the table.main:void,usize,isizeare absent from theFFITypetable (docs/runtime/ffi.mdxand the copy indocs/runtime/c-compiler.mdx), while the Callbacks example on the same page uses"usize".CFunctionexamples usenew CFunction(...);CFunctionis a plain function (src/js/bun/ffi.ts,function CFunction(options)), andpackages/bun-types/ffi.d.tsdeclares it as one.viewSourceis not mentioned anywhere indocs/.read.intptris missing from thereadtable.cc()accepts anincludeoption (ffi.d.ts,ffi_body.rsreadsoptions.include) that theccreference on the C Compiler page does not list.Fix
FFITypetables: addisizeto thei64row,usizeto theu64row, and avoidrow markedreturnsonly, plus one sentence each on what that means (usize/isizereturn abigintlikeu64/i64;voidis the default return type and is rejected inargs).newfrom bothCFunctionexamples and say why in the sentence that introduces it.viewSourcesection. Since bun:ffi: use the engine-native FFI when available #35246 onlycc()compiles C, so the section describes whatviewSourceprints today: the per-symbol C wrapper thatcc()compiles, one string per symbol, and a comment string for the callback form. This is deliberately different from the section in docs(ffi): cover cc, viewSource, Node-API types, and fix CFunction usage #31535, which describedviewSourceas showing the bindingsdlopencompiles; that is no longer true.read.intptrto thereadtable (header renamed fromFFITypetoType, sinceintptris areadhelper and not anFFIType) with a sentence on how it differs fromread.ptr.includeto theccreference, next todefine.bun:ffi(the FFI sources have not changed since the build used):"usize"/"isize"indlopen,cc(), andJSCallbackdescriptors;returns: "void"and omittedreturns;args: ["void"]throwing indlopen,linkSymbols,CFunction,JSCallback, andcc();CFunction(...)withoutnew;read.intptrreturning-5whereread.ptrreturns18446744073709552000for the same bytes; theviewSourcesnippet as written (array of one C string, callback form returns a comment); theincludesnippet as written, with a cwd-relative directory, as an array and as a string, and failing without it.bun run prettier --checkon both pages is clean.Background
bun:ffihas two binding paths since bun:ffi: use the engine-native FFI when available #35246.dlopen,linkSymbols,CFunction, andJSCallbackare implemented inside JavaScriptCore, which generates the argument marshalling itself.cc()still uses TinyCC: for each symbol it generates a small C wrapper (Function::print_source_codeinsrc/runtime/ffi/ffi_body.rs) that unpacks the JS arguments, calls the user's function, and boxes the result, then compiles it.viewSourcereturns the output of that same printer, which is why the docs now describe it in terms ofcc().read.*are raw memory readers taking a pointer and byte offset.read.ptrandread.intptrboth read 8 bytes into a JSnumber;ptrtreats them as unsigned,intptras a signedintptr_t(src/runtime/ffi/FFIObject.rs).What happened to the rest of #31535
cc()reading symbol definitions fromoptions[key]instead ofoptions.symbols[key]: fixed onmain(src/js/bun/ffi.ts,cc()).napi_env/napi_valueJS coercion wrappers and the numericFFITypereverse-lookup keys: the JS coercion layer (ffiWrappers) they patched was removed in bun:ffi: use the engine-native FFI when available #35246, and the N-API types are now rejected outsidecc(), so there is nothing left to fix.size_talias in the JSFFITypeobject andffi.d.ts, and thenew CFunctionin theffi.d.tsJSDoc example: not part of Incomplete API coverage and discrepancies inbun:ffi#31534 (the runtime already accepts the"size_t"string natively). Left out to keep this PR docs only; theffi.d.tsgaps (isize/usizeand several accepted string aliases missing from the type maps, plus that JSDoc example) are being handled separately.ccJSDoc example tweak (${hello()}): unnecessary now thatcstringreturns are plain strings.