Skip to content

feat(ffi)!: C names by Rust's case, one facade crate, prefix from its lib name - #599

Draft
kixelated wants to merge 25 commits into
mainfrom
quest/m1/ffi-export/names
Draft

kixelated wants to merge 25 commits into
mainfrom
quest/m1/ffi-export/names

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m1/ffi-export/names.md: rust2-ffi names every exported item by a collision-free rule and takes the crate set and prefix from Cargo, so ffi.toml keeps only exclude.

Summary

  • C names keep Rust's case (decided 2026-11-04): moq_session_Stats is the type, moq_Session_stats the method. A field's getter and setter add _get and _set (SWIG's convention), so moq-tokio's Backoff::initial field is moq_tokio_Backoff_initial_get/_set beside the method moq_tokio_Backoff_initial, and the reject fixture's Limit::max setter stays apart from set_max. A tag or constant is the name in upper case (MOQ_HANG_CATALOG_AUDIOCODEC_OPUS). A clash, which only unconventional names can cause, fails the export at both spans. Every generator derives its names from the C ones, so the field accessors are x_get()/x_set(v) in C++, a x_get property and x_set(v) in Python, xGet/xSet(v) in Kotlin, Swift, and Dart, and XGet()/XSet(v) in Go.
  • One facade crate. The exporter exports one crate's pub API, following its pub use re-exports. Items from other crates are named by their path in the facade (moq's tokio::Client is moq_tokio_Client). [crates.*] and package are gone, and the CLI takes the facade's --manifest-path.
  • Prefix from the facade's Cargo lib name ([lib] name = "moq" gives moq_). The fixtures set api, reject, and dep, so diagnostics and exclude keys spell paths by the lib crate's name (api::Size, not api_fixture::Size).
  • ffi.toml reads only exclude, and prefix, package, [crates.*], and rename are removed (ffi/src/config.rs). ffi/moq/ffi.toml is rewritten as facade paths.
  • just ffi standin now writes a facade of pub uses: moq-net and moq-pattern at the root, kio's waiter and clock items beside them, and hang, mux, and tokio modules.
  • Language-specific fixes the new names needed:
    • Go: a package function named as a type of its package, which Go's single namespace cannot keep apart as C's case does, is New and that name when it returns that type (or a Result or Option of it, as New{T} for T::new already ignores fallibility), and takes a trailing _ otherwise. moq's kio::clock::interval(period) is kio.NewClockInterval, kio::clock::wall() (which returns Wall) is kio.NewClockWall, and mux::codec::{aac,mp3,opus}::config(init), which return hang::catalog::AudioConfig, are mux.Codec{Aac,Mp3,Opus}Config_. The api fixture covers both branches (polar::point returns Option<polar::Point>, so NewPolarPoint; the new span(start, end) returns u32, so Span_).
    • Dart: constants are upper case (LIMIT).
    • Variant names: Go's variant names keep the variant's case.
  • Go: a package per Rust crate (decided 2026-11-07, see ## Decisions). go/ holds go/<package>/ for each crate whose items the facade exports, and go/rt/, the runtime they share. A crate's package is its name without the prefix and its _ (moq_mux is mux, serde_json is serdejson), so moq's export is net, pattern, kio, hang, mux, tokio, url, serdejson, and rt, and callers write mux.CodecOpusConfig_ and hang.CatalogAudioConfig. Names inside a package drop the facade segments outside the crate (moq::mux::codec::opus::Config is mux.CodecOpusConfig).
    • Placement (ffi/src/collect.rs, rehome): each item's IR now carries a Home, its crate and how many leading path segments lie outside it. An instance of a generic type, a function naming one, and a method whose instance names a later crate's type belong to the last of the crates involved in rustc's postorder_cnums order, which puts each crate after its dependencies, so every Go import points earlier in that order and the packages cannot form a cycle. moq's mux::catalog::Producer<mux::catalog::hang::Extra> stays in mux. moq-net's Error::from_transport::<E> at moq-tokio's QUIC errors moves to tokio (tokio.ErrorFromTransportSessionError), as a package function taking the receiver first, since Go keeps a type's methods in its own package.
    • Import path: the module of the nearest go.mod above --out, plus the path to go/. With none, the export writes go/go.mod declaring module <prefix>, which a caller requires and replaces with that directory, as just ffi moq and api_go now do. No config key: the derivation always has an answer.
    • Runtime split: cgo gives each package C types of its own, which no exported API may carry, so packages reach each other only through Go types. runtime/rt.go is the shared rt package: Bytes and its lending, ErrClosed, the handle type Handle_, the //exported call and release, and the contract check, plus generated Bound, ranges, TupleN, and error structs of no crate's type (rt.StringError). runtime/cgo.go is each crate package's copy of what converts its own C types (the arena, buffers, durations, addresses, await_). A record's, enum's, object's, or callback's conversions are generated in whichever package lifts or lowers it.
    • Cross-package hatches: an object's handle is the exported field Handle_ *rt.Handle_, so another package can lift one, and a trait exports <Trait>Vtable_(), so another package can lower an implementation without naming another package's C symbols. Callers use neither.
  • Foreign types: url::Url and serde_json::Value keep their parse-from-text mapping unless the facade re-exports that exact type. The old check was crate-wide, so re-exporting any other item of the crate dropped the mapping.
  • Crate identity, from this PR's local review: the IR marks a foreign object (ir::Object::foreign), and the glue depends on its crate as <crate>_foreign, apart from the facade's <root>_export. A facade whose lib is named url or serde_json and maps that crate's type now builds; before, its two glue dependencies shared one key. Census counts objects by the mark rather than by path root. No check identifies a type by a path the facade could spell: a foreign object shows at the facade's root under its role name (Url, Json), so abi names it as any other item and a facade's own serde_json::Value and serde_json::Json keep their names; the standard library's crates are known by what they define (Sized, Box, HashMap), and core's socket addresses and core's and alloc's drained iterators by those lang items too, so a facade whose lib is named core keeps its own SocketAddrV4. Tests: foreign_lib_name, foreign_role_names, and standard_names. serde's traits are still found by crate name, since that only adds a candidate trait solving then checks.
  • Merged main's moq-dev/rust2#607: an exclude may name what a cfg leaves out of this build, such as moq's tokio::unix::Error on Windows. With facade paths, the check walks the path through the facade's modules, and a missing segment counts as left out when a cfg stripped it from that module or from one the module glob-re-exports (ffi/src/driver.rs, configured_out). Its renames side is gone with rename. Test: configured_out.
  • Benchmarks: ffi/moq/bench/export.* in all seven languages use the new names (moq_broadcast_Info, moq::Timescale::MICRO(), Go's PayloadGet(), and the payload_get/payloadGet property).
  • Docs and tests: moq's tests in C, C++, Kotlin, Python, Swift, Go, and Dart, the api fixture's tests, and ffi/README.md's naming and config sections are updated to the new names.
  • Quests: deletes names.md and its links. It adds the follow-up quest/m1/ffi-export/modules.md to the epic's Required (see below).

Choices made without asking

  • ConnectTargetError alias: the stand-in re-exports moq-tokio's connect::Error as tokio::ConnectTargetError. Kotlin, Swift, Python, Go, and Dart join a type's path segments in Pascal case, so ConnectError and connect::Error would otherwise both be TokioConnectError. C is unaffected. Removing the alias is the new modules.md quest, which this PR adds directly instead of through /quest-plan.
  • Go package names drop the facade's prefix and the _ from the crate's name, matching the decision's mux.CodecOpusConfig_ example; the export refuses two crates with one package name, or one named as rt, C, main, a Go keyword, or a standard package the generated items use.
  • No go.mod above --out makes the export write go/go.mod (module <prefix>), rather than refuse as quest/m4/go/runtime-names.md recommends for rust2-go: rust2-ffi exports every language at once, so a refusal would fail a C-only export.
  • span in the api fixture: area(size) no longer clashes in Go, since Area is now dep.Area, so the new span(start, end) -> u32, beside the Span record, keeps the trailing-_ branch covered (api.Span_).
  • Dart and C++ constants are now upper case, as in C (api.LIMIT, moq::Timescale::MILLI()), so a constant stays apart from a method of the same name.

Public API

Breaking for Go callers: the generated Go is now go/<package>/ per crate plus go/rt/, instead of go/<prefix>.go, so imports and qualifiers change (moq.MuxCodecOpusConfig_ is mux.CodecOpusConfig_, moq.Bytes is rt.Bytes, moq.ErrClosed is rt.ErrClosed). To migrate, require the export's module (its go.mod, or the one above --out) and import each crate's package. An object gains the Handle_ field and a trait the <Trait>Vtable_ function, both for the generated packages. The IR gains Home on every item and Api::crates. Other languages and the C ABI are unchanged, but for the api fixture's new span function.

Decisions

Decided 2026-11-06

  • Keep modules.md in this PR rather than planning it separately.
  • Accept upper-case constants in Dart and C++ (api.LIMIT, moq::Timescale::MILLI()).
  • Go: a package function whose name matches a type's. ✅ New where the function returns the type it clashes with, a trailing _ otherwise (rejected: a Path_name split such as MuxCodecOpus_config, which needs a second rule once Go gets a package per module; an Fn/Call prefix; a trailing _ everywhere). Each clash is within one Rust module, so the rule still holds under modules.md's Go package per module (clock.NewInterval(p), opus.Config_(init)), which its Plan now notes. The decision's example ClockWall_() for clock::wall() became NewClockWall(), since kio::clock::wall returns clock::Wall.

Decided 2026-11-07

  • Design checkpoint after round 3: ✅ continue (rejected: keep the flat names and only identify foreign types by module; identify by module now and plan packages later). Its Go part, a package per Rust module, turned out impossible under Go's import-cycle ban, and was settled again below.
  • How Go namespaces moq: ✅ one Go package per Rust crate, since crates cannot form import cycles while modules can (moq-net's, hang's and mux's, and tokio's do), so callers write mux.CodecOpusConfig_, as uniffi-bindgen-go lays out crates; within a crate, names keep the settled flat form with the New and trailing _ rule (rejected: one Go package, the earlier recommendation; alias packages per module; one package per import cycle; one package per module, which Go's cycle ban rules out). The packages' import path is derived from the go.mod above --out, with no config key.
  • No name-based crate checks: ✅ identify what a type is by the crate or module that defines it, never by its spelled path (rejected: option 1 of the checkpoint, the Json name applied only where foreign is set). Done for the foreign role names and core's and alloc's types.

Models

  • Size: L
  • Implementer: Claude Code, Claude Opus 5.5, high effort

Checks

  • nix develop --command just ci (lint, quest, ffi check and test) passes on 3889c07, after merging origin/main at efa6300. api_go builds the api fixture's five packages (api, dep, url, serdejson, rt) through cgo, vets them, and runs the transcript, futures, closing, bytes, and contract cases.
  • just ffi moq passes on a715023: moq's export vets as nine packages, and moq/moq.go imports net, pattern, kio, hang, mux, tokio, url, serdejson, and rt, and runs. moq/bench/export.go vets against them; the bench itself did not run.
  • foreign_role_names, standard_names, foreign_lib_name, and lookalike pass with crates of one name in separate Go packages.

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, xhigh effort
  • Session: 01a1b3a8-d966-79c2-98f0-448cc99bf967, round 3
  • Head: e764a18, named in full once a round reports no findings
  • Base: origin/main at 324a7b7
  • Score: 9/10, The dependency collision is fixed, but ABI naming still misclassifies a facade-owned Value type when its library is named serde_json.
  • Findings, open until the next round settles them:
    • Medium, ffi/src/abi.rs:142: Apply the Json role name only to foreign objects
  • The last round's findings, at 43a2c7b:
    • Fixed: medium, ffi/src/collect.rs:276: Separate dependency identities instead of requiring a facade rename; ffi/src/glue.rs:105 and ffi/src/glue.rs:266 now distinguish foreign dependencies and type paths from the facade, and ffi/tests/fixtures.rs:1185 covers successful export and compilation of the original url collision.

Round 3's finding is fixed in 5dd3a41 (the role name moved into the IR's shown path). The next local review starts with quest review --new after the Go design change.

Follow-ups

  • quest/m1/ffi-export/modules.md: languages other than C keep facade module paths apart, so the stand-in needs no alias.
  • quest/m1/ffi-export/refusals.md, now unblocked: remove exclude by making each refused item export or fail.
  • A fixture case for the foreign-type fix: re-export another url item from the api fixture and keep url::Url's mapping. It's not added here because it changes the api fixture's generated surface in every language.

Local review restarted with quest review --new at round 1 on 3420e2d. The stored session from rounds 1 to 3 (through f8835e3, clean) recorded each round's findings as a count, which quest 0.1.0 reads as a list, so resuming it failed with the reviewer's reply does not settle each of the last round's undefined finding(s) once.

(Written by Claude Opus 5.5 with high effort)

🤖 Generated with Claude Code

kixelated and others added 8 commits November 4, 2026 11:12
…m its lib name

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at stay apart

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tself

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title C names and crates with no config feat(ffi)!: C names by Rust's case, one facade crate, prefix from its lib name Nov 6, 2026
@kixelated
kixelated marked this pull request as ready for review November 6, 2026 21:09
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7af48357-e72b-47fb-88a0-5a3685a45606
Commits

Reviewing files that changed from the base of the PR and between b3df54b and f8835e3.

Files selected for processing: 53

  • ffi/README.md
  • ffi/fixtures/api/Cargo.toml
  • ffi/fixtures/api/examples/transcript.rs
  • ffi/fixtures/api/ffi.toml
  • ffi/fixtures/api/src/lib.rs
  • ffi/fixtures/dep/Cargo.toml
  • ffi/fixtures/dep/src/lib.rs
  • ffi/fixtures/reject/Cargo.toml
  • ffi/fixtures/reject/expected.txt
  • ffi/fixtures/reject/ffi.toml
  • ffi/fixtures/reject/src/lib.rs
  • ffi/justfile
  • ffi/moq/bench/export.c
  • ffi/moq/bench/export.cpp
  • ffi/moq/bench/export.dart
  • ffi/moq/bench/export.go
  • ffi/moq/bench/export.kt
  • ffi/moq/bench/export.py
  • ffi/moq/bench/export.swift
  • ffi/moq/ffi.toml
  • ffi/moq/moq.c
  • ffi/moq/moq.cpp
  • ffi/moq/moq.dart
  • ffi/moq/moq.go
  • ffi/moq/moq.kt
  • ffi/moq/moq.py
  • ffi/moq/moq.swift
  • ffi/src/abi.rs
  • ffi/src/c.rs
  • ffi/src/census.rs
  • ffi/src/cli.rs
  • ffi/src/collect.rs
  • ffi/src/config.rs
  • ffi/src/cpp.rs
  • ffi/src/dart.rs
  • ffi/src/driver.rs
  • ffi/src/glue.rs
  • ffi/src/go.rs
  • ffi/src/kotlin.rs
  • ffi/src/python.rs
  • ffi/src/swift.rs
  • ffi/tests/api.c
  • ffi/tests/api.cpp
  • ffi/tests/api.dart
  • ffi/tests/api.go
  • ffi/tests/api.kt
  • ffi/tests/api.py
  • ffi/tests/api.swift
  • ffi/tests/fixtures.rs
  • quest/m1/ffi-export/README.md
  • quest/m1/ffi-export/modules.md
  • quest/m1/ffi-export/names.md
  • quest/m1/ffi-export/refusals.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed head: 45cd701

One finding: the ABI/accessor renames leave the existing moq benchmarks using names that are no longer generated. Please migrate those consumers alongside the tests.

Verification: GitHub-only static review of the changed generators, facade collection, fixtures, and benchmark callers. I did not run builds or tests. The check runs inspected for this head were skipped or neutral, so they do not verify just ffi bench.

(Written by review (OpenAI))

Comment thread ffi/src/abi.rs Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as draft November 6, 2026 22:24
kixelated and others added 8 commits November 6, 2026 15:25
…lashes with

A package function named as a type of its module is New and that name when it
returns that type, or an Option of it, and takes a trailing _ otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… area function

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… refuse a facade named as a foreign type's crate

A facade whose lib is named core, alloc, or std exported nothing of its own, since
the walk took any crate of those names for the standard library. One named url or
serde_json that maps that crate's type wrote glue whose two dependencies shared a
name; it now fails at collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…crate keeps its own dependency

A facade whose lib is named url or serde_json and maps that crate's type wrote glue
whose dependencies on the facade and on the crate shared one name. The IR now marks
a foreign object, the glue depends on its crate as <crate>_foreign, and the census
counts objects by the mark instead of by their root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated and others added 3 commits November 6, 2026 20:38
…ver by a spelled path

A foreign type shows at the facade's root under its role name (`Url`, `Json`), so the
ABI names it as any other item and a facade's own `serde_json::Value` keeps its name.
core and alloc are known by the lang items they define, so a facade named `core` keeps
its own `SocketAddrV4` and iterators.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: after the round-3 checkpoint, the maintainer chose to continue and to give Go a package per Rust module. They also ruled that the exporter identifies what a type is by its defining crate or module, never by a path the facade could spell.

  • Done in 5dd3a41: a foreign object's role name (Url, Json) now lives in the IR's shown path under the facade's root, so abi has no serde_json::Value special case. core's and alloc's types are now known by their lang items instead of crate names. foreign_role_names and standard_names cover both, and each failed before the fix.
  • Not built: a Go package per module. Go forbids import cycles, and moq's modules name each other's types in three cycles (the root with 16 moq-net modules, 12 of hang and mux, and the 7 of tokio). The options and a recommendation are under ## Waiting in the PR body.

The next local review starts with quest review --new once that settles.

(Written by Claude Opus 5.5 with high effort)

kixelated and others added 5 commits November 6, 2026 22:55
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…o package

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: the maintainer settled the Go question on 2026-11-07: one Go package per Rust crate. Crates can't form import cycles, and modules can (moq-net's, hang's and mux's, and tokio's do), so callers write mux.CodecOpusConfig_, as uniffi-bindgen-go does. Within a crate, names keep the flat form with the New / trailing _ rule.

  • Built in cfecaa0..3889c07: go/<package>/ per crate plus a shared go/rt/. Each instance, and each function or method naming one, belongs to the last crate involved in rustc's crate order, so imports only point earlier and can't cycle. moq-net's Error::from_transport::<E> at moq-tokio's QUIC errors becomes tokio.ErrorFromTransportSessionError.
  • The import path comes from the go.mod above --out. With none, the export writes go/go.mod declaring module <prefix>. No config key.
  • cgo gives each package C types of its own, so packages reach each other only through Go: an object's Handle_ field, a trait's <Trait>Vtable_(), and rt's byte and handle hatches.
  • just ci passes, and just ffi moq runs moq/moq.go against nine packages.

The next local review starts with quest review --new.

(Written by Claude Opus 5.5 with high effort)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant