diff --git a/examples/echo/echo.nim b/examples/echo/echo.nim index 7313859..cb689f1 100644 --- a/examples/echo/echo.nim +++ b/examples/echo/echo.nim @@ -6,9 +6,7 @@ import ffi, chronos, strutils type Echo = object prefix: string -# `-d:ffiEchoAbiC` builds the `abi = c` variant (`_CWire` structs on the wire); -# the default is the CBOR ABI. The same source drives both the `c_bindings/` -# (CBOR) and `c_abi_bindings/` example outputs. +# `-d:ffiEchoAbiC` builds the `abi = c` variant; default is the CBOR ABI. when defined(ffiEchoAbiC): declareLibrary("echo", Echo, defaultABIFormat = "c") else: diff --git a/examples/timer/timer.nim b/examples/timer/timer.nim index 0ce0924..eb52568 100644 --- a/examples/timer/timer.nim +++ b/examples/timer/timer.nim @@ -2,17 +2,11 @@ import ffi, chronos, options type Maybe[T] = Option[T] -# The library's main state type. The FFI context owns one instance. -# Named `MyTimer` (not `Timer`) so the C-exported symbols are -# `my_timer_create` / `my_timer_destroy` / ... — `timer_create` would -# collide with POSIX ``'s `int timer_create(clockid_t, ...)` which -# `` transitively drags in on Linux. +# Named `MyTimer` (not `Timer`) so C symbols like `my_timer_create` don't collide with POSIX ``'s `timer_create`. type MyTimer = object name: string # set at creation time, read back in each response -# `defaultABIFormat` selects the wire format every {.ffi.} / {.ffiEvent.} / ... -# in this library inherits; "cbor" is the default and can be overridden per -# annotation with an `"abi = ..."` spec. +# `defaultABIFormat` is the wire format every annotation inherits (override per-annotation with "abi = ..."). declareLibrary("my_timer", MyTimer, defaultABIFormat = "cbor") type TimerConfig {.ffi.} = object @@ -37,27 +31,19 @@ type ComplexResponse {.ffi.} = object itemCount: int hasNote: bool -# --- Library-initiated event ---------------------------------------------- -# Demonstrates the {.ffiEvent.} macro: a typed event the library can fire -# from any {.ffi.} handler, dispatched to the foreign side's registered -# callback as CBOR. Per-target codegens emit a typed handler-struct + -# dispatcher so the foreign caller decodes nothing by hand. +# {.ffiEvent.}: a typed event any {.ffi.} handler can fire to the foreign callback. type EchoEvent {.ffi.} = object message: string echoCount: int proc onEchoFired*(evt: EchoEvent) {.ffiEvent: "on_echo_fired".} -# --- Constructor ----------------------------------------------------------- -# Called once from Rust. Creates the FFIContext + MyTimer. -# Uses chronos (await sleepAsync) so the body is async. +# Constructor: creates the FFIContext + MyTimer; async via chronos. proc myTimerCreate*(config: TimerConfig): Future[Result[MyTimer, string]] {.ffiCtor.} = await sleepAsync(1.milliseconds) # proves chronos is live on the FFI thread return ok(MyTimer(name: config.name)) -# --- Async method ---------------------------------------------------------- -# Waits `delayMs` milliseconds (non-blocking, on the chronos event loop) -# then echoes the message back with a request counter. +# Async method: sleeps `delayMs` then echoes the message back. proc myTimerEcho*( timer: MyTimer, req: EchoRequest ): Future[Result[EchoResponse, string]] {.ffi.} = @@ -65,9 +51,7 @@ proc myTimerEcho*( onEchoFired(EchoEvent(message: req.message, echoCount: 1)) return ok(EchoResponse(echoed: req.message, timerName: timer.name)) -# --- Sync method ----------------------------------------------------------- -# No await — the macro detects this and fires the callback inline, -# without going through the request channel. +# Sync method: no await, so the macro fires the callback inline. proc myTimerVersion*(timer: MyTimer): Future[Result[string, string]] {.ffi.} = return ok("nim-timer v0.1.0") @@ -82,12 +66,7 @@ proc myTimerComplex*( return ok(ComplexResponse(summary: summary, itemCount: count, hasNote: req.note.isSome)) -# --- Multiple complex parameters ------------------------------------------- -# Demonstrates how a {.ffi.} proc handles several object-typed parameters at -# once. Each parameter is its own {.ffi.} type, so it lands in the generated -# foreign-side bindings as a first-class struct/class, and the per-proc Req -# envelope (MyTimerScheduleReq on the wire) carries all three under field -# names that match the Nim params. +# Multiple object-typed params: each is its own {.ffi.} type, all carried in one per-proc Req envelope. type JobSpec {.ffi.} = object name: string payload: seq[string] @@ -112,10 +91,7 @@ type ScheduleResult {.ffi.} = object proc myTimerSchedule*( timer: MyTimer, job: JobSpec, retry: RetryPolicy, schedule: ScheduleConfig ): Future[Result[ScheduleResult, string]] {.ffi.} = - ## Composes three independent object-typed parameters (`job`, `retry`, - ## `schedule`) into a single scheduling decision. The macro packs them into - ## one CBOR-encoded request envelope on the wire and unpacks them back into - ## the named locals before this body runs. + ## Three object-typed params (`job`, `retry`, `schedule`) packed into one CBOR envelope. await sleepAsync(1.milliseconds) if job.name.len == 0: return err("job name must not be empty") @@ -137,22 +113,8 @@ proc myTimerSchedule*( ) proc my_timer_destroy*(timer: MyTimer) {.ffiDtor.} = - ## Tears down the FFI context created by my_timer_create. - ## Blocks until the FFI thread and watchdog thread have joined. + ## Tears down the FFI context; blocks until FFI + watchdog threads join. discard -# genBindings() must be the LAST top-level call in the FFI root file — -# after every {.ffi.}, {.ffiCtor.} and {.ffiDtor.} pragma. Each pragma -# fires at compile time and registers its proc into the compile-time -# ffiProcRegistry / ffiTypeRegistry; genBindings() then reads those -# registries to emit the language bindings. If genBindings() runs before -# a pragma, that proc is silently absent from the generated bindings. -# -# Multi-file libraries: keep all .ffi./.ffiCtor./.ffiDtor. pragmas in -# imported sub-modules and call genBindings() once at the bottom of the -# top-level file that imports them — Nim resolves imports before the -# importing file's body runs, so the registries are fully populated by -# the time genBindings() executes. -# -# genBindings() is a compile-time no-op unless -d:ffiGenBindings is set. +# Must be the LAST top-level call, after every pragma registered its proc (no-op unless -d:ffiGenBindings). genBindings() diff --git a/ffi/alloc.nim b/ffi/alloc.nim index 07951bf..3bbb593 100644 --- a/ffi/alloc.nim +++ b/ffi/alloc.nim @@ -1,24 +1,13 @@ ## Cross-thread allocation helpers backed by libc `malloc`/`free`. -## -## We deliberately avoid Nim's `allocShared`/`deallocShared` here. Under -## `--mm:orc` they delegate to the per-thread `allocator` MemRegion stored -## in TLS; freeing such a buffer from a different thread later walks -## `chunk.owner` back to that MemRegion. If the original thread has exited -## by then (e.g. a `std::async` worker that produced the FFI request and -## was destroyed before the FFI thread ran `deleteRequest`), `chunk.owner` -## dangles into reclaimed TLS and `addToSharedFreeList` segfaults — TSan on -## ARM reproduces this from `TimerE2E.ThreadedHammer`. `malloc`/`free` are -## process-global and thread-lifetime-independent, so freeing on a different -## thread is safe. +## Avoids Nim `allocShared` whose TLS-owned MemRegion segfaults when freed from a +## thread other than the one that allocated (and may have since exited); libc is process-global. import system/ansi_c -## Can be shared safely between threads type SharedSeq*[T] = tuple[data: ptr UncheckedArray[T], len: int] proc alloc*(str: cstring): cstring = - ## Allocates a fresh null-terminated copy of `str` via `c_malloc`. The - ## returned pointer must be released with `dealloc(cstring)`. + ## Fresh null-terminated `c_malloc` copy of `str`; free with `dealloc(cstring)`. if str.isNil(): var ret = cast[cstring](c_malloc(1)) ret[0] = '\0' @@ -29,8 +18,6 @@ proc alloc*(str: cstring): cstring = return ret proc alloc*(str: string): cstring = - ## Allocates a fresh null-terminated copy of `str` via `c_malloc`. The - ## returned pointer must be released with `dealloc(cstring)`. var ret = cast[cstring](c_malloc(csize_t(str.len + 1))) let s = cast[seq[char]](str) for i in 0 ..< str.len: @@ -39,20 +26,15 @@ proc alloc*(str: string): cstring = return ret proc dealloc*(p: cstring) {.inline.} = - ## Frees a buffer obtained from one of the `alloc(...)` overloads above. - ## Nil-safe. + ## Frees an `alloc(...)` buffer. Nil-safe. if not p.isNil(): c_free(cast[pointer](p)) proc allocBox*(size: int): pointer = - ## `c_malloc` block for a cross-thread callback box (allocated on the foreign - ## caller thread, freed on the FFI thread). Uses libc for the same - ## thread-lifetime safety reason as the rest of this module. Free with - ## `freeBox`. + ## `c_malloc` block for a cross-thread callback box; free with `freeBox`. c_malloc(csize_t(size)) proc freeBox*(p: pointer) = - ## Releases a block from `allocBox`. Nil-safe. if not p.isNil(): c_free(p) @@ -70,8 +52,6 @@ proc deallocSharedSeq*[T](s: var SharedSeq[T]) = s.len = 0 proc toSeq*[T](s: SharedSeq[T]): seq[T] = - ## Creates a seq[T] from a SharedSeq[T]. No explicit dealloc is required - ## as req[T] is a GC managed type. var ret = newSeq[T]() for i in 0 ..< s.len: ret.add(s.data[i]) diff --git a/ffi/cbor_serial.nim b/ffi/cbor_serial.nim index 54b1f8a..24b83d0 100644 --- a/ffi/cbor_serial.nim +++ b/ffi/cbor_serial.nim @@ -1,31 +1,6 @@ -## Thin wrapper around `cbor_serialization` (vacp2p/nim-cbor-serialization) that -## adapts the library's exception-based API to the `Result[T, string]` shape the -## FFI plumbing expects, and adds the few transport-only details the FFI layer -## needs on top: -## -## - `cborEncodeShared` writes into a `c_malloc` buffer so the FFI thread -## can take ownership of the bytes without a second copy. `c_malloc` -## (not `allocShared`) because the buffer must be freeable from the FFI -## thread after the producing thread may have exited — see the note in -## `ffi/ffi_thread_request.nim`. -## - `CborNullByte` is the canonical "successful but no value" wire sentinel. -## -## `cborEncode` / `cborDecode` are the public API the macros and tests use. -## -## Type contract for `.ffi.` payloads: -## -## - Plain `object` types flow as value copies — fields are serialized and -## the foreign side reconstructs an independent value. -## - `ref T` is *also* a value copy: `cbor_serialization`'s default `ref T` -## writer dereferences and encodes the pointee, so the receiving side -## allocates a fresh `ref` local to its own GC heap. No object identity -## is preserved across the boundary — the two sides own independent -## copies after decode. -## - Raw `pointer` / `ptr T` are rejected at macro-expansion time (see -## `rejectRawPtrType` in `internal/ffi_macro.nim`). The only address that -## legitimately crosses the boundary is the opaque ctx handle returned by -## `.ffiCtor.`, which is validated against `FFIContextPool` on every -## re-entry. Arbitrary user pointers would lack that validation. +## `cbor_serialization` wrapper adapting its exception API to `Result[T, string]` for the FFI layer. +## `.ffi.` payloads (plain `object` and `ref T`) cross as value copies; raw `pointer`/`ptr T` are +## rejected at macro-expansion time (see `rejectRawPtrType`). import system/ansi_c import cbor_serialization, cbor_serialization/std/options, results @@ -33,20 +8,14 @@ import cbor_serialization, cbor_serialization/std/options, results export cbor_serialization, options, results const CborNullByte*: byte = 0xf6'u8 - ## CBOR encoding of `null` — used as the wire sentinel for empty OK payloads. + ## CBOR `null` — wire sentinel for empty OK payloads. proc cborEncode*[T](x: T): seq[byte] = - ## CBOR-encode any cbor_serialization-supported type (plus `pointer` / `ptr T` - ## via our custom writers) into a fresh `seq[byte]`. return Cbor.encode(x) proc cborEncodeShared*[T](x: T): tuple[data: ptr UncheckedArray[byte], len: int] = - ## Encodes `x` into a `c_malloc` buffer. - ## - ## The returned `data` is owned by the caller and must be freed exactly - ## once via `cborFreeShared`. The - ## `FFIThreadRequest deleteRequest` path frees adopted buffers - ## automatically. Empty payloads return `(nil, 0)` without allocating. + ## Encodes `x` into a caller-owned `c_malloc` buffer (free via `cborFreeShared`). + ## Empty payloads return `(nil, 0)` without allocating. let bytes = Cbor.encode(x) if bytes.len == 0: return (nil, 0) @@ -55,16 +24,13 @@ proc cborEncodeShared*[T](x: T): tuple[data: ptr UncheckedArray[byte], len: int] return (buf, bytes.len) proc cborFreeShared*(data: var ptr UncheckedArray[byte]) = - ## Releases a buffer previously returned by `cborEncodeShared` and nils - ## the caller's pointer so a stale reference can't be reused after free. - ## Safe to call with `nil` (the `(nil, 0)` empty-payload contract). + ## Frees a `cborEncodeShared` buffer and nils the pointer. Nil-safe. if not data.isNil(): c_free(data) data = nil proc cborDecode*[T](data: openArray[byte], _: typedesc[T]): Result[T, string] = - ## Decode `data` into a `T`, converting any cbor_serialization exception - ## into a `Result.err` carrying the exception message. + ## Decode `data` into a `T`, mapping any exception to `Result.err`. try: let v = Cbor.decode(data, T) return ok(v) @@ -74,8 +40,7 @@ proc cborDecode*[T](data: openArray[byte], _: typedesc[T]): Result[T, string] = proc cborDecodePtr*[T]( data: ptr UncheckedArray[byte], dataLen: int, _: typedesc[T] ): Result[T, string] = - ## Convenience for ptr+len buffers (used by the macro to avoid binding an - ## openArray to a `let`). + ## Convenience for ptr+len buffers. if dataLen <= 0: return cborDecode(default(seq[byte]), T) cborDecode(toOpenArray(data, 0, dataLen - 1), T) diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index 5331ed1..71075bb 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -1,15 +1,11 @@ -## C99 binding generator. The library's ABI format picks the shape: `cbor` -## (default) emits three headers (prelude + cbor codecs + `.h`) exchanging -## CBOR via vendored TinyCBOR; `c` (`abi = c`) emits one `.h` whose structs -## are the C ABI directly. C lacks generics, so each distinct `seq[T]`/`Option[T]` -## is monomorphised into its own struct + codec triple (e.g. `seq[uint32]` yields -## an `EchoSeq_U32` struct plus `echo_enc_`/`echo_dec_`/`echo_free_EchoSeq_U32`). +## C99 binding generator. `abi = cbor` (default) emits three CBOR headers; +## `abi = c` emits one header whose structs are the C ABI directly. Lacking +## generics, each distinct `seq[T]`/`Option[T]` is monomorphised per type. import std/[os, strutils, tables, sets] import ./meta, ./string_helpers, ./c_cpp_common, ./types_ir -## Fixed 64-bit wire type for any Nim `ptr T`/`pointer`, so payload size is -## host-arch independent (mirrors CppPtrType). +## Fixed 64-bit wire type for any Nim `ptr T`/`pointer` (mirrors CppPtrType). const CPtrType* = "uint64_t" const @@ -47,8 +43,7 @@ func leafSuffix(cType: string): string = else: "" func cToken(cType: string): string = - ## PascalCase token for monomorphised names: leaf suffix capitalised, else the - ## (already unique) composite C name verbatim. + ## PascalCase token for monomorphised names. let suffix = leafSuffix(cType) if suffix.len > 0: return capitalizeFirstLetter(suffix) @@ -244,10 +239,8 @@ proc emitStructType(reg: var CTypeReg, t: FFITypeMeta) = reg.owns[t.name] = owns proc ensureCType(reg: var CTypeReg, t: FFIType): tuple[cType: string, owns: bool] = - ## Lowers the type intermediate representation (see types_ir.nim) to a C type, - ## monomorphising each distinct `seq[T]`/`Option[T]` on first sight. `owns` - ## marks a type carrying heap payload the caller must release via its generated - ## free function. + ## Lowers an `FFIType` to a C type, monomorphising each `seq[T]`/`Option[T]` + ## on first sight. `owns` marks a type the caller must free. case t.kind of ftPtr: return (CPtrType, false) @@ -285,8 +278,7 @@ proc ensureCType(reg: var CTypeReg, nimType: string): tuple[cType: string, owns: return ensureCType(reg, parseFFIType(nimType)) proc reqTypeMeta(p: FFIProcMeta): FFITypeMeta = - ## Synthesises the per-proc Req struct so it flows through the same - ## monomorphisation path as user types; pointer/handle params ride as uint64. + ## Synthesises the per-proc Req struct; pointer/handle params ride as uint64. var fields: seq[FFIFieldMeta] = @[] for ep in p.extraParams: let typeName = if ep.ridesAsPtr(): "pointer" else: ep.typeName @@ -641,8 +633,7 @@ proc emitMethod( lines.add(" if (dec != 0) {") lines.add(" box->fn(-1, NULL, err ? err : \"decode failed\", box->user_data);") lines.add(" free(err);") - # Reclaim any fields a partial decode allocated (out is zeroed, so free skips - # what was never written). + # Reclaim fields a partial decode allocated (out is zeroed). if retFree.len > 0: lines.add(" " & retFree & "(&out);") lines.add(" free(box);") @@ -722,9 +713,8 @@ proc monomorphiseAll( procs, methods: seq[FFIProcMeta], events: seq[FFIEventMeta], ): tuple[reqTypes, respTypes: seq[string]] = - ## Runs every user type, Req envelope, return type and event payload through - ## ensureCType, emitting structs/codecs into `reg` in dependency order. - ## Returns the Req and response C type names the buffer adapters need. + ## Runs every type, Req, return type and event payload through ensureCType, + ## returning the Req and response C type names the buffer adapters need. for t in types: discard ensureCType(reg, t.name) var reqTypes: seq[string] = @[] @@ -741,13 +731,11 @@ proc monomorphiseAll( return (reqTypes, respTypes) func generateCPreludeHeader*(): string = - ## The library-agnostic `nim_ffi_prelude.h`: owned string/byte types + libc/ - ## TinyCBOR includes, emitted verbatim. + ## The library-agnostic `nim_ffi_prelude.h`, emitted verbatim. return HeaderPreludeTpl & "\n" func generateCCborHeader*(): string = - ## The library-agnostic `nim_ffi_cbor.h`: leaf CBOR codecs and buffer drivers, - ## emitted verbatim. + ## The library-agnostic `nim_ffi_cbor.h`, emitted verbatim. return CborHelpersTpl & "\n" proc generateCLibHeader*( @@ -857,10 +845,7 @@ proc generateCCMakeLists*(libName, nimSrcRelPath: string): string = let src = nimSrcRelPath.replace("\\", "/") return CMakeListsTpl.multiReplace(("{{LIB}}", libName), ("{{SRC}}", src)) -# `abi = c` binding: the `_CWire` structs are the C ABI (no CBOR). Layout -# mirrors `wireValueType`/`wireFieldsFor` byte-for-byte: `string`→`const char*`, -# `seq[T]`→`* _items` + `ptrdiff_t _len`, `Option[T]`→`*` -# (NULL = none), nested type→its `_CWire` struct, `ptr`/`pointer`→`void*`. +# `abi = c` binding: structs are the C ABI directly (no CBOR), matching the Nim-side wire layout byte-for-byte. const AbiCPtrType = "void*" const AbiCMakeListsTpl = staticRead("templates/c/CMakeLists_abi.txt.tpl") @@ -1261,8 +1246,7 @@ proc generateCAbiCMakeLists*(libName, nimSrcRelPath: string): string = return AbiCMakeListsTpl.multiReplace(("{{LIB}}", libName), ("{{SRC}}", src)) func libWireFormat(procs: seq[FFIProcMeta], types: seq[FFITypeMeta]): ABIFormat = - ## The single wire format the C header targets; a library can't mix `abi = c` - ## and `abi = cbor` in one header. + ## The single wire format the C header targets (no mixing in one header). var seen: set[ABIFormat] = {} for p in procs: if p.kind != FFIKind.DTOR: @@ -1286,8 +1270,7 @@ proc generateCBindings*( nimSrcRelPath: string, events: seq[FFIEventMeta] = @[], ) = - ## Emits the C binding for `libName`, picking the `abi = c` or CBOR shape from - ## the library's ABI format. + ## Emits the C binding for `libName`, picking the `abi = c` or CBOR shape. createDir(outputDir) case libWireFormat(procs, types) of ABIFormat.C: diff --git a/ffi/codegen/c_cpp_common.nim b/ffi/codegen/c_cpp_common.nim index fcabea0..5ca3ce0 100644 --- a/ffi/codegen/c_cpp_common.nim +++ b/ffi/codegen/c_cpp_common.nim @@ -1,22 +1,17 @@ -## Helpers shared by the language-specific binding generators (cpp.nim, c.nim). -## Kept here so the per-proc envelope naming, lib-prefix stripping and -## proc-classification logic live in one place rather than being copy-pasted -## into each backend. +## Helpers shared by the C/C++ binding generators (cpp.nim, c.nim). import std/strutils import ./meta, ./string_helpers proc stripLibPrefix*(procName, libName: string): string = - ## Drops the `_` prefix from an exported C symbol, e.g. - ## `stripLibPrefix("timer_echo", "timer")` → `"echo"`. + ## Drops the `_` prefix from an exported C symbol. let prefix = libName & "_" if procName.startsWith(prefix): return procName[prefix.len .. ^1] return procName proc reqStructName*(p: FFIProcMeta): string = - ## Mirrors the Nim macro: `Req`, or `...CtorReq` for a - ## constructor. The per-proc envelope every backend encodes onto the wire. + ## Per-proc wire envelope name: `Req` (`...CtorReq` for ctors). let camel = snakeToPascalCase(p.procName) if p.kind == FFIKind.CTOR: camel & "CtorReq" @@ -29,9 +24,7 @@ type ClassifiedProcs* = object dtorProcName*: string proc classifyProcs*(procs: seq[FFIProcMeta]): ClassifiedProcs = - ## Splits the registry into constructors, instance methods and (the first) - ## destructor symbol — the split every backend needs before emitting a - ## high-level context wrapper. + ## Splits the registry into constructors, methods and the first destructor. var c: ClassifiedProcs for p in procs: case p.kind @@ -45,8 +38,7 @@ proc classifyProcs*(procs: seq[FFIProcMeta]): ClassifiedProcs = c proc libTypeName*(ctors: seq[FFIProcMeta], libName: string): string = - ## The user's library type name (e.g. `MyTimer`), taken from the first ctor - ## or derived from `libName` when the library declares none. + ## The library type name, from the first ctor or derived from `libName`. if ctors.len > 0: return ctors[0].libTypeName capitalizeFirstLetter(libName) diff --git a/ffi/codegen/cddl.nim b/ffi/codegen/cddl.nim index 5fa2914..c2e34f2 100644 --- a/ffi/codegen/cddl.nim +++ b/ffi/codegen/cddl.nim @@ -1,7 +1,5 @@ -## CDDL (RFC 8610) schema generator for the nim-ffi framework. -## Mirrors the CBOR wire format produced by ffi/cbor_serial.nim: every -## user-declared {.ffi.} type becomes a CDDL rule, every {.ffi.} / {.ffiCtor.} -## proc gets a request envelope rule plus a response shape rule. +## CDDL (RFC 8610) schema generator mirroring the CBOR wire format from +## ffi/cbor_serial.nim: types become rules, procs get request/response rules. import std/[os, strutils, unicode] import ./meta @@ -25,22 +23,18 @@ proc toCamelCase(s: string): string = return res proc nimTypeToCddl*(typeName: string): string = - ## Maps a Nim type name (as recorded in the compile-time registries) to its - ## CDDL equivalent. Unknown PascalCase names are passed through as references - ## to other CDDL rules in the same document. + ## Nim type name → CDDL equivalent; unknown names pass through as rule refs. let t = typeName.strip() let seqI = innerOf(t, "seq[") if seqI.len > 0: let inner = seqI.strip() if inner == "byte" or inner == "uint8": - # `seq[byte]` rides the wire as a CBOR byte string, matching the Nim - # cbor_serialization writer — reflect that in the schema. + # seq[byte] rides the wire as a CBOR byte string. return "bytes" return "[* " & nimTypeToCddl(inner) & "]" let arrI = innerOf(t, "array[") if arrI.len > 0: - # CDDL has no fixed-length array literal as ergonomic as Nim's array; emit - # an unbounded array whose element type is the user-declared element type. + # Emit an unbounded array of the element type (CDDL lacks a fixed-length literal). let commaIdx = arrI.find(',') let elemT = if commaIdx >= 0: @@ -63,7 +57,6 @@ proc nimTypeToCddl*(typeName: string): string = of "float32": "float32" of "pointer": "uint" else: t - # reference to another rule in this CDDL document proc reqStructName(p: FFIProcMeta): string = ## Mirrors the Nim macro: {Ctor}Req. @@ -101,14 +94,13 @@ proc emitReqFields(p: FFIProcMeta): string = emitMap(fields) proc responseRule(p: FFIProcMeta): string = - ## CDDL shape of the success payload returned by the FFI callback. - ## Error payloads stay as raw UTF-8 and are intentionally absent from the schema. + ## CDDL shape of the success payload; error payloads are raw UTF-8, absent here. case p.kind of FFIKind.CTOR: - # The ctor returns the FFI context address as a CBOR-encoded decimal string. + # Ctor returns the FFI context address as a CBOR decimal string. "tstr" of FFIKind.DTOR: - # The dtor has no meaningful payload — handleRes sends a CBOR null sentinel. + # Dtor payload is a CBOR null sentinel. "nil" of FFIKind.FFI: if p.returnRidesAsPtr(): diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index dbf22dc..b75d76c 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -1,18 +1,11 @@ -## C++ binding generator for the nim-ffi framework. -## Generates a header-only C++ binding and CMakeLists.txt. Requests/responses -## travel as CBOR (encoded with vendored TinyCBOR on the C++ side, matching -## the Nim-side cbor_serial codec on the wire — both ends speak RFC 8949). +## C++ binding generator: header-only binding + CMakeLists, CBOR over the wire. import std/[os, strutils] import ./meta, ./string_helpers, ./c_cpp_common, ./types_ir -## Wire-format C++ type used for any Nim `ptr T` / `pointer`. Fixed 64-bit so -## the CBOR payload size is stable regardless of host architecture. +## Fixed 64-bit wire type for any Nim `ptr T` / `pointer`. const CppPtrType* = "uint64_t" -## Static template blocks live as real C++ / CMake files under templates/cpp/ -## and are slurped into the binary at compile time. Edits to those files are -## reflected in the generated bindings without touching this codegen. const HeaderPreludeTpl = staticRead("templates/cpp/header_prelude.hpp.tpl") ResultTpl = staticRead("templates/cpp/result.hpp.tpl") @@ -56,14 +49,9 @@ proc nimTypeToCpp*(typeName: string): string = proc emitStructCborCodec( lines: var seq[string], structName: string, fields: seq[(string, string)] ) = - ## Appends per-struct TinyCBOR encode_cbor + decode_cbor free functions for - ## `structName`. `fields` is a sequence of (field-name, ignored C++ type) - ## pairs — the type is unused at the codec layer because the generic - ## encode_cbor / decode_cbor overloads in cbor_helpers.hpp.tpl dispatch on - ## the struct member's type. We emit a CBOR map with text-string keys to - ## match the wire format produced by Nim's cbor_serialization. + ## Appends per-struct TinyCBOR encode_cbor + decode_cbor functions emitting a + ## text-keyed CBOR map. The C++ type in `fields` is unused (overloads dispatch). let n = fields.len - # ── encode ──────────────────────────────────────────────────────────────── if n == 0: lines.add( "inline CborError encode_cbor(CborEncoder& e, const $1&) {" % [structName] @@ -84,7 +72,6 @@ proc emitStructCborCodec( ) lines.add(" return cbor_encoder_close_container(&e, &m);") lines.add("}") - # ── decode ──────────────────────────────────────────────────────────────── if n == 0: lines.add("inline CborError decode_cbor(CborValue& it, $1&) {" % [structName]) lines.add(" if (!cbor_value_is_map(&it)) return CborErrorImproperValue;") @@ -106,44 +93,15 @@ proc emitStructCborCodec( lines.add("}") proc cppBracedInit(structName: string, fieldNames: seq[string]): string = - ## Produces a C++ braced-init expression for a per-proc Req struct. - ## Used to construct the request value before CBOR-encoding it for the wire, - ## as in `const auto req = TimerEchoReq{message, count};` in the generated - ## header. The field order must match the struct's declaration order, which - ## in turn mirrors the user's Nim FFI signature. - ## - ## Examples: - ## cppBracedInit("TimerEchoReq", @["message", "count"]) - ## → "TimerEchoReq{message, count}" - ## cppBracedInit("TimerVersionReq", @[]) - ## → "TimerVersionReq{}" - ## cppBracedInit("TimerCreateCtorReq", @["config"]) - ## → "TimerCreateCtorReq{config}" - ## - ## Empty `fieldNames` collapses cleanly because `join` on an empty seq - ## returns "", so the result is the well-formed empty-init `Name{}`. + ## C++ braced-init for a Req struct, e.g. `TimerEchoReq{message, count}`. return structName & "{" & fieldNames.join(", ") & "}" proc emitEventDispatcher( lines: var seq[string], ctxTypeName, libName: string, events: seq[FFIEventMeta] ) = - ## Public listener-registration API in the generated context class: - ## - ## - `addOnListener(std::function) -> ListenerHandle` - ## per declared `{.ffiEvent.}`. Internally registers under the wire - ## event name; the per-listener trampoline decodes the CBOR - ## envelope's `payload` field as `T` and invokes the user handler. - ## Callers subscribe to each event separately. - ## - `removeEventListener(ListenerHandle) -> bool` drops a listener by - ## handle. After it returns true, no further callbacks for that id - ## are in flight on the FFI side (the Nim-side registry lock plus - ## snapshot copy guarantees this). - ## - ## Ownership: each listener's callable is held by a - ## `std::unique_ptr` in `listeners_`, keyed by id; the - ## raw pointer is handed to the dylib as `user_data`. The map entry - ## (and therefore the callable) survives at a stable heap address - ## until `removeEventListener` removes it. + ## Emits the public per-event `addOnListener` / `removeEventListener` API. + ## Callables are owned by `listeners_` (unique_ptr keyed by id); the raw + ## pointer is the dylib's `user_data`, stable until removal. if events.len == 0: return lines.add( @@ -151,7 +109,6 @@ proc emitEventDispatcher( ) lines.add(" struct ListenerHandle { std::uint64_t id = 0; };") lines.add("") - # Per-event typed registration helpers. for ev in events: let methodName = "addOn" & capitalizeFirstLetter(ev.nimProcName).substr(2) & "Listener" @@ -174,7 +131,6 @@ proc emitEventDispatcher( lines.add(" return ListenerHandle{id};") lines.add(" }") lines.add("") - # Remove by handle. lines.add(" bool removeEventListener(ListenerHandle handle) {") lines.add(" if (handle.id == 0) return false;") lines.add( @@ -186,14 +142,8 @@ proc emitEventDispatcher( lines.add("") proc emitEventTrampoline(lines: var seq[string], events: seq[FFIEventMeta]) = - ## Private listener machinery for the public API emitted by - ## `emitEventDispatcher`: - ## - ## - `ListenerBase` is a polymorphic base so the context's - ## `listeners_` map can own typed listeners under a single value type. - ## - `TypedListener` holds the user's `std::function` - ## and is the target of `typedTrampoline`, which CBOR-decodes the - ## envelope's `payload` field as `T` and invokes the handler. + ## Private listener machinery for `emitEventDispatcher`: polymorphic + ## `ListenerBase`, `TypedListener` and the `typedTrampoline` decoder. if events.len == 0: return lines.add(" struct ListenerBase {") @@ -208,7 +158,6 @@ proc emitEventTrampoline(lines: var seq[string], events: seq[FFIEventMeta]) = ) lines.add(" };") lines.add("") - # Typed trampoline — one instantiation per payload type, all sharing a body. lines.add(" template ") lines.add( " static void typedTrampoline(int ret, const char* msg, std::size_t len, void* ud) {" @@ -241,24 +190,13 @@ proc generateCppHeader*( lines.add(HeaderPreludeTpl) if events.len > 0: - # Only pulled in when the library declares `{.ffiEvent.}` procs — - # `` backs the `listeners_` map. lines.add("#include ") - # Result is the exception-free return channel used by every generated - # entry point. It must precede the CBOR helpers and sync-call helper below, - # which now hand their failures back as Result rather than throwing. lines.add(ResultTpl) - # CBOR primitive / container helpers must precede the per-struct codecs - # below, because each emitted `encode_cbor`/`decode_cbor(T)` calls the - # generic overloads for the struct's fields (std::string, std::vector, - # std::optional, primitives). The struct codecs are non-template `inline` - # functions, so name lookup happens at parse time — the overloads must be - # in scope before the struct codecs are parsed. + # Generic CBOR overloads must precede the non-template struct codecs that call them (parse-time name lookup). lines.add(CborHelpersTpl) - # ── Types ────────────────────────────────────────────────────────────────── if types.len > 0: lines.add("// ============================================================") lines.add("// User-declared FFI types") @@ -275,7 +213,6 @@ proc generateCppHeader*( emitStructCborCodec(lines, t.name, fields) lines.add("") - # ── Per-proc Req structs (CBOR transport units) ─────────────────────────── lines.add("// ============================================================") lines.add("// Per-proc request envelopes (CBOR encoded on the wire)") lines.add("// ============================================================") @@ -304,7 +241,6 @@ proc generateCppHeader*( emitStructCborCodec(lines, reqName, fields) lines.add("") - # ── C FFI declarations ───────────────────────────────────────────────────── lines.add("// ============================================================") lines.add("// C FFI declarations") lines.add("// ============================================================") @@ -328,8 +264,7 @@ proc generateCppHeader*( ) of FFIKind.DTOR: lines.add("int $1(void* ctx);" % [p.procName]) - # `declareLibrary` always exports the listener-registration ABI. Declare - # it here so the typed event-handler wiring below can call into it. + # Listener-registration ABI is always exported. lines.add( "uint64_t $1_add_event_listener(void* ctx, const char* event_name, FFICallback callback, void* user_data);" % [libName] @@ -342,7 +277,6 @@ proc generateCppHeader*( lines.add(SyncCallHelperTpl) - # ── High-level C++ context class ────────────────────────────────────────── let classified = classifyProcs(procs) let ctors = classified.ctors let methods = classified.methods @@ -355,7 +289,6 @@ proc generateCppHeader*( lines.add("class $1 {" % [ctxTypeName]) lines.add("public:") - # ── Constructors ──────────────────────────────────────────────────────── for ctor in ctors: let reqName = reqStructName(ctor) var ctorParams: seq[string] = @[] @@ -377,18 +310,7 @@ proc generateCppHeader*( let reqInit = cppBracedInit(reqName, epNames) - # Same `ffi_*_` underscore convention as instance methods so that a ctor - # parameter cannot collide with the local Req envelope name. - # - # The ctor's C symbol returns `void*` (the ctx pointer) synchronously, but - # `ffi_call_` expects an int-returning lambda — and we want the callback - # path anyway since it carries the CBOR-encoded ctx address. Discard the - # synchronous return and yield 0 from the lambda; the address comes back - # through the callback's CBOR text-string payload. - # `create` returns std::unique_ptr rather than a Ctx by value: the - # context owns library threads, so we forbid copy/move on the class - # itself (see ContextRuleOf5Tpl) and hand out ownership through a - # smart pointer that callers can move, store in containers, etc. + # `create` yields the ctx via the callback's CBOR address (sync void* return discarded), owned as a unique_ptr since the class forbids copy/move. let createRet = "Result>" % [ctxTypeName] lines.add(" static $1 create($2) {" % [createRet, ctorParamsWithTimeout]) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) @@ -412,9 +334,7 @@ proc generateCppHeader*( " if (ffi_addr_.isErr()) return $1::err(ffi_addr_.error());" % [createRet] ) lines.add(" const auto& addr_str = ffi_addr_.value();") - # Parse the ctx address without exceptions: std::stoull would throw on a - # non-numeric payload, so use std::from_chars and surface the failure as - # an err() Result instead. + # from_chars (not stoull) so a bad payload is an err() Result, not a throw. lines.add(" std::uint64_t addr = 0;") lines.add(" const char* addr_begin = addr_str.data();") lines.add(" const char* addr_end = addr_begin + addr_str.size();") @@ -425,7 +345,7 @@ proc generateCppHeader*( [createRet] ) lines.add(" }") - # Use `new` directly (not std::make_unique) so the ctor can stay private. + # `new` (not make_unique) so the ctor can stay private. lines.add( " return $1::ok(std::unique_ptr<$2>(new $2(reinterpret_cast(static_cast(addr)), timeout)));" % [createRet, ctxTypeName] @@ -454,15 +374,12 @@ proc generateCppHeader*( lines.add(" }") lines.add("") - # ── Rule of 5 ────────────────────────────────────────────────────────── lines.add( ContextRuleOf5Tpl.multiReplace(("{{CTX}}", ctxTypeName), ("{{LIB}}", libName)) ) - # ── Typed event handlers (public section) ─────────────────────────────── emitEventDispatcher(lines, ctxTypeName, libName, events) - # ── Instance methods ──────────────────────────────────────────────────── for m in methods: let methodName = stripLibPrefix(m.procName, libName) let retCppType = @@ -488,8 +405,6 @@ proc generateCppHeader*( let reqInit = cppBracedInit(reqName, methParamNames) let methRet = "Result<$1>" % [retCppType] - # Use a single-underscore-suffixed local for the Req envelope so it can't - # shadow a method parameter whose name happens to be `req` (or similar). lines.add(" $1 $2($3) const {" % [methRet, methodName, methParamsStr]) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) lines.add(" auto ffi_enc_ = encodeCborFFI(ffi_req_);") @@ -509,10 +424,7 @@ proc generateCppHeader*( lines.add(" return decodeCborFFI<$1>(ffi_raw_.value());" % [retCppType]) lines.add(" }") lines.add("") - # The async wrapper calls the sync method via `this->methodName(...)` so - # a method param that happens to share the method's name doesn't shadow - # the call target (e.g. `schedule(job, retry, schedule)` would otherwise - # parse as invoking the `schedule` parameter). + # `this->methodName(...)` so a same-named param can't shadow the call target. if methParamsStr.len > 0: lines.add( " std::future<$1> $2Async($3) const {" % [methRet, methodName, methParamsStr] @@ -532,20 +444,11 @@ proc generateCppHeader*( lines.add("") lines.add("private:") - # Listener machinery (`ListenerBase`, `TypedListener`, plus the - # static trampolines) must appear before the `listeners_` data member - # declaration — C++ requires the value type of a member to be complete - # at point of declaration. The public add*/remove methods above also - # reference these types, but member function bodies see the full class - # scope regardless of declaration order, so emitting here is sufficient - # for both. + # Listener machinery must precede the `listeners_` member (its value type must be complete at declaration). emitEventTrampoline(lines, events) lines.add(" void* ptr_;") lines.add(" std::chrono::milliseconds timeout_;") if events.len > 0: - # One owning entry per live listener, keyed by id. Destroyed after - # the destructor body runs `_destroy(ptr_)`, by which point the - # FFI side has joined its threads so no callback is mid-flight. lines.add( " std::unordered_map> listeners_;" ) diff --git a/ffi/codegen/meta.nim b/ffi/codegen/meta.nim index 4622a79..63c8a84 100644 --- a/ffi/codegen/meta.nim +++ b/ffi/codegen/meta.nim @@ -1,20 +1,20 @@ -## Compile-time metadata types for FFI binding generation. -## Populated by the {.ffiCtor.} and {.ffi.} macros and consumed by codegen. +## Compile-time metadata types for FFI binding generation, populated by the +## {.ffiCtor.}/{.ffi.} macros and consumed by codegen. import std/strutils type ABIFormat* {.pure.} = enum - ## Wire format for an FFI payload. Only `Cbor` is wired end-to-end; `C` - ## (`abi = c` C-struct) has a type codec but no proc-dispatch path yet. + ## FFI payload wire format. `Cbor` is wired end-to-end; `C` has a type codec + ## but no proc-dispatch path yet. Cbor = "cbor" C = "c" FFIParamMeta* = object - name*: string # Nim param name, e.g. "req" - typeName*: string # Nim type name, e.g. "EchoRequest" - isPtr*: bool # true if the type is `ptr T` - isHandle*: bool # true if the type is an {.ffiHandle.} type (wire form uint64) + name*: string + typeName*: string + isPtr*: bool + isHandle*: bool # {.ffiHandle.} type, wire form uint64 FFIKind* {.pure.} = enum FFI @@ -22,70 +22,61 @@ type DTOR FFIProcMeta* = object - procName*: string # e.g. "timer_echo" - libName*: string # library name, e.g. "timer" + procName*: string + libName*: string kind*: FFIKind - libTypeName*: string # e.g. "Timer" + libTypeName*: string extraParams*: seq[FFIParamMeta] # all params except the lib param - returnTypeName*: string # e.g. "EchoResponse", "string", "pointer" - returnIsPtr*: bool # true if return type is ptr T - returnIsHandle*: bool # true if return type is an {.ffiHandle.} type - abiFormat*: ABIFormat # wire format for this interaction (default Cbor) + returnTypeName*: string + returnIsPtr*: bool + returnIsHandle*: bool + abiFormat*: ABIFormat scalarFastPath*: bool - ## True for an `abi = c` proc whose whole signature is scalar (see - ## `isScalarOnly`): dispatches through the CBOR-free scalar fast path and - ## is skipped by the foreign-binding generators (their codegen is a - ## follow-up). + ## `abi = c` proc with an all-scalar signature: uses the CBOR-free fast + ## path and is skipped by the foreign-binding generators. FFIFieldMeta* = object - name*: string # e.g. "delayMs" - typeName*: string # e.g. "int" + name*: string + typeName*: string FFITypeMeta* = object name*: string fields*: seq[FFIFieldMeta] - abiFormat*: ABIFormat # wire format for this type (default Cbor) + abiFormat*: ABIFormat FFIEventMeta* = object - ## Library-initiated event from `{.ffiEvent: "wire_name".}`. `wireName` is - ## the verbatim CBOR `eventType` string the foreign side dispatches on. + ## Library-initiated event from `{.ffiEvent: "wire_name".}`; `wireName` is + ## the verbatim CBOR `eventType` the foreign side dispatches on. wireName*: string nimProcName*: string libName*: string payloadTypeName*: string - abiFormat*: ABIFormat # wire format for this event (default Cbor) + abiFormat*: ABIFormat -# Compile-time registries populated by the macros var ffiProcRegistry* {.compileTime.}: seq[FFIProcMeta] var ffiTypeRegistry* {.compileTime.}: seq[FFITypeMeta] var ffiEventRegistry* {.compileTime.}: seq[FFIEventMeta] var currentLibName* {.compileTime.}: string -# Set by `declareLibrary`; the FFI annotations require it (name/type/default ABI). +# Set by `declareLibrary`; the FFI annotations require it. var libraryDeclared* {.compileTime.}: bool = false -# Set by `genBindings()`. Any FFI annotation expanded after it registers into the -# codegen registries too late to be emitted, so the annotation macros check this -# and fail loudly instead of silently dropping the proc/type from the bindings. +# Set by `genBindings()`. Annotations expanded after it register too late to be emitted, so the macros check this and fail loudly instead of dropping silently. var genBindingsEmitted* {.compileTime.}: bool = false # Library-wide default ABI, inherited by each annotation unless it overrides. var currentDefaultABIFormat* {.compileTime.}: ABIFormat = ABIFormat.Cbor proc abiCodegenImplemented*(fmt: ABIFormat): bool = - ## Whether `fmt` has a working proc-dispatch path. Both `Cbor` and `C` are - ## wired: `Cbor` rides the generic overloads, `C` rides the `_CWire` - ## companions (a CBOR-free foreign surface with CBOR transport internally). + ## Whether `fmt` has a working proc-dispatch path (both Cbor and C do). fmt in {ABIFormat.Cbor, ABIFormat.C} proc overrideKey*(override: string): string = - ## Lowercased key of a `key = value` pragma override (the text before `=`), - ## used to route it to its parser. `"abi = c"` → `"abi"`. + ## Lowercased key of a `key = value` pragma override, e.g. `"abi = c"` → `"abi"`. override.split('=')[0].strip().toLowerAscii() proc parseABIFormatName*(name: string): tuple[ok: bool, fmt: ABIFormat] = - ## Bare format name (`"c"`/`"cbor"`, case-insensitive) → `ABIFormat`; - ## `ok` is false otherwise. + ## Bare format name ("c"/"cbor", case-insensitive) → ABIFormat; else ok=false. case name.strip().toLowerAscii() of "cbor": (true, ABIFormat.Cbor) @@ -95,8 +86,7 @@ proc parseABIFormatName*(name: string): tuple[ok: bool, fmt: ABIFormat] = (false, ABIFormat.Cbor) proc parseAbiSpec*(override: string): tuple[ok: bool, fmt: ABIFormat, err: string] = - ## Parse an `"abi = "` override (whitespace/case tolerant). On bad - ## grammar or format, returns `ok = false` with a human-readable `err`. + ## Parse an `"abi = "` override; on bad grammar returns ok=false + err. let parts = override.split('=') if parts.len != 2: return ( @@ -129,30 +119,21 @@ proc isFFIHandleTypeName*(name: string): bool {.compileTime.} = name in ffiHandleTypeNames proc ridesAsPtr*(ep: FFIParamMeta): bool = - ## True if the param crosses the wire as an opaque uint64 — a raw `ptr` or an - ## `{.ffiHandle.}` id. Both share the codegen pointer type. + ## True if the param crosses the wire as an opaque uint64 (raw ptr or handle). ep.isPtr or ep.isHandle proc returnRidesAsPtr*(p: FFIProcMeta): bool = - ## True if the return crosses the wire as an opaque uint64 (raw `ptr` or handle). + ## True if the return crosses the wire as an opaque uint64 (raw ptr or handle). p.returnIsPtr or p.returnIsHandle -# Target language(s) for binding generation; override with -d:targetLang=cpp. -# Accepts a comma-separated list (e.g. -d:targetLang=rust,cpp,c) to emit -# several languages from a single compile. +# Target language(s), override with -d:targetLang=cpp; comma-separated list allowed. const targetLang* {.strdefine.} = "rust" -# Output directory override for generated bindings; set with -# -d:ffiOutputDir=path/to/dir. Empty (the default) derives `_bindings/` -# next to the compiled source. +# Output dir override (-d:ffiOutputDir); empty derives `_bindings/` by src. const ffiOutputDir* {.strdefine.} = "" -# Nim source path override (relative to outputDir) embedded in generated build -# files; set with -d:ffiSrcPath=../relative/path.nim. Empty (the default) -# derives it from the compiled source relative to the output dir. +# Nim src path override relative to outputDir (-d:ffiSrcPath); empty derives it. const ffiSrcPath* {.strdefine.} = "" -# When set to true, scalar-only `abi = c` procs (which have no foreign-binding codegen -# yet) are silently omitted from the generated bindings instead of failing the -# build. Off by default so the drop is loud; see genBindings(). +# When true, scalar-only `abi = c` procs are silently omitted rather than failing the build. Off by default so the drop is loud; see genBindings(). const ffiAllowScalarSkip* {.booldefine.} = false diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 671ecc0..f07d655 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -1,12 +1,10 @@ -## Rust binding generator for the nim-ffi framework. -## Generates a complete Rust crate that uses CBOR (ciborium) on the wire. +## Rust binding generator: emits a complete Rust crate using CBOR (ciborium). import std/[os, strutils] import ./meta, ./string_helpers, ./types_ir -## Wire-format Rust type used for any Nim `ptr T` / `pointer`. Fixed 64-bit so -## the CBOR payload size is stable regardless of host architecture (mirrors -## CppPtrType in cpp.nim). +## Wire-format Rust type for any Nim `ptr T`/`pointer`; fixed 64-bit for a +## host-independent CBOR payload size (mirrors CppPtrType). const RustPtrType* = "u64" func rustScalar(s: ScalarKind): string = @@ -44,8 +42,7 @@ proc nimTypeToRust*(typeName: string): string = renderNative(rustMap, parseFFIType(typeName)) proc deriveLibName*(procs: seq[FFIProcMeta]): string = - ## Extracts the common prefix before the first `_` from proc names. - ## e.g. ["timer_create", "timer_echo"] → "timer" + ## Common prefix before the first `_` in proc names, e.g. "timer_create" → "timer". if currentLibName.len > 0: return currentLibName if procs.len == 0: @@ -57,8 +54,7 @@ proc deriveLibName*(procs: seq[FFIProcMeta]): string = return "unknown" proc stripLibPrefix*(procName: string, libName: string): string = - ## Strips the library prefix from a proc name. - ## e.g. "timer_echo", "timer" → "echo" + ## Strips the library prefix, e.g. ("timer_echo", "timer") → "echo". let prefix = libName & "_" if procName.startsWith(prefix): return procName[prefix.len .. ^1] @@ -73,14 +69,7 @@ proc reqStructName(p: FFIProcMeta): string = camel & "Req" proc generateCargoToml*(libName: string): string = - # `flume` is the unified callback channel (PR #23 Rust review, item 8): one - # primitive that supports both `recv_timeout` (blocking trampoline) and - # `recv_async` (async trampoline). Default-features disabled to avoid - # pulling its async-std/futures shims. - # `tokio` is needed only for `tokio::time::timeout` around the async - # `recv_async`. Feature-gating tokio (item 11) is a follow-up commit. - # `[dev-dependencies]` lets the bundled `examples/` use `#[tokio::main]` - # without pulling those features into the library's runtime profile. + # flume: callback channel (recv_timeout + recv_async), default-features off. tokio: only the async timeout. return """[package] name = "$1" @@ -99,8 +88,8 @@ tokio = { version = "1", features = ["rt-multi-thread", "macros", "sync", "time" [libName] proc generateBuildRs*(libName: string, nimSrcRelPath: string): string = - ## Generates build.rs that compiles the Nim library. - ## nimSrcRelPath is relative to the output (crate) directory. + ## Generates build.rs that compiles the Nim library; nimSrcRelPath is relative + ## to the crate directory. let escapedSrc = nimSrcRelPath.replace("\\", "\\\\") return """use std::path::PathBuf; @@ -162,8 +151,8 @@ pub use api::*; """ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = - ## Generates ffi.rs with extern "C" declarations. Each Nim FFI proc takes a - ## single CBOR buffer (ptr+len) for its request payload. + ## Generates ffi.rs with extern "C" declarations; each proc takes one CBOR + ## buffer (ptr+len) as its request payload. var lines: seq[string] = @[] lines.add("use std::os::raw::{c_char, c_int, c_void};") lines.add("") @@ -175,18 +164,15 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = lines.add(");") lines.add("") - # Collect unique lib names for #[link(...)] var libNames: seq[string] = @[] for p in procs: if p.libName notin libNames: libNames.add(p.libName) - # Derive lib name from proc names if not set var linkLibName = "" if libNames.len > 0 and libNames[0].len > 0: linkLibName = libNames[0] else: - # derive from first proc name if procs.len > 0: let parts = procs[0].procName.split('_') if parts.len > 0: @@ -199,7 +185,7 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = var params: seq[string] = @[] case p.kind of FFIKind.FFI: - # Method/destructor-style: ctx comes first + # Method-style: ctx first. params.add("ctx: *mut c_void") params.add("callback: FFICallback") params.add("user_data: *mut c_void") @@ -207,7 +193,7 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = params.add("req_cbor_len: usize") lines.add(" pub fn $1($2) -> c_int;" % [p.procName, params.join(", ")]) of FFIKind.CTOR: - # Constructor: no ctx; returns the freshly-allocated handle + # Ctor: no ctx; returns the freshly-allocated handle. params.add("req_cbor: *const u8") params.add("req_cbor_len: usize") params.add("callback: FFICallback") @@ -217,8 +203,7 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = params.add("ctx: *mut c_void") lines.add(" pub fn $1($2) -> c_int;" % [p.procName, params.join(", ")]) - # Listener-registration ABI — emitted on the Nim side by `declareLibrary`, - # always present in the dylib. + # Listener-registration ABI, always present in the dylib. lines.add( " pub fn $1_add_event_listener(ctx: *mut c_void, event_name: *const c_char, callback: FFICallback, user_data: *mut c_void) -> u64;" % [linkLibName] @@ -232,8 +217,7 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = return lines.join("\n") & "\n" proc generateTypesRs*(types: seq[FFITypeMeta], procs: seq[FFIProcMeta]): string = - ## Generates types.rs with Rust structs for all user-declared FFI types and - ## for each per-proc Req struct (matching the Nim macro's generated types). + ## Generates types.rs: Rust structs for user FFI types and each per-proc Req. var lines: seq[string] = @[] lines.add("use serde::{Deserialize, Serialize};") lines.add("") @@ -244,15 +228,14 @@ proc generateTypesRs*(types: seq[FFITypeMeta], procs: seq[FFIProcMeta]): string for f in t.fields: let snakeName = camelToSnakeCase(f.name) let rustType = nimTypeToRust(f.typeName) - # Add serde rename if camelCase name differs from snake_case + # serde rename when camelCase differs from snake_case. if snakeName != f.name: lines.add(" #[serde(rename = \"$1\")]" % [f.name]) lines.add(" pub $1: $2," % [snakeName, rustType]) lines.add("}") lines.add("") - # Per-proc Req structs — these wrap the typed parameters and are the unit of - # CBOR encoding sent across the FFI boundary. + # Per-proc Req structs: the unit of CBOR encoding sent across the boundary. for p in procs: if p.kind == FFIKind.DTOR: continue @@ -280,13 +263,7 @@ proc generateTypesRs*(types: seq[FFITypeMeta], procs: seq[FFIProcMeta]): string proc generateApiRs*( procs: seq[FFIProcMeta], libName: string, events: seq[FFIEventMeta] = @[] ): string = - ## Generates api.rs with both a blocking and a tokio-async high-level API. - ## - ## Blocking: ctx.echo(req) — thread-blocks via Condvar - ## Async: ctx.echo_async(req).await — non-blocking via oneshot channel; - ## the FFI callback fires from the Nim/chronos thread and wakes - ## the awaiting task without ever blocking a thread. - ## + ## Generates api.rs with a blocking and a tokio-async high-level API. ## Requests/responses are CBOR (ciborium); errors are raw UTF-8 strings. var lines: seq[string] = @[] @@ -311,7 +288,6 @@ proc generateApiRs*( let ctxTypeName = libTypeName & "Ctx" - # ── Imports ──────────────────────────────────────────────────────────────── lines.add("use std::os::raw::{c_char, c_int, c_void};") lines.add("use std::slice;") lines.add("use std::time::Duration;") @@ -321,7 +297,6 @@ proc generateApiRs*( lines.add("use super::types::*;") lines.add("") - # ── CBOR helpers ─────────────────────────────────────────────────────────── lines.add("fn encode_cbor(value: &T) -> Result, String> {") lines.add(" let mut buf = Vec::new();") lines.add( @@ -335,16 +310,7 @@ proc generateApiRs*( lines.add("}") lines.add("") - # ── Unified FFI trampoline (PR #23 Rust review, items 1, 2, 4, 8, 9) ─────── - # One callback shape used by both the blocking and async wrappers. The - # `user_data` pointer owns a single `Box, - # String>>>`; the callback reconstructs it, sends the payload, and drops - # the box (releasing the sender). The receiver side then either - # `recv_timeout` (sync) or `recv_async` under `tokio::time::timeout` - # (async). A late callback that fires after the caller has already timed - # out sends into a closed receiver, which is harmless: the Err is - # discarded and the box drops cleanly. No Arc/Condvar; no Box leak; no - # late-fire UAF; no double trampoline. + # FFI trampoline: user_data owns a Box; a late callback sends into a closed receiver, which is harmless. lines.add("type FFIResult = Result, String>;") lines.add("type FFISender = flume::Sender;") lines.add("") @@ -472,11 +438,7 @@ proc generateApiRs*( lines.add("}") lines.add("") - # ── Per-listener handler boxes + extern "C" trampolines ───────────────── - # Each registered listener owns a `Box<…Handler>` that is kept alive in - # `$1::listeners` (keyed by listener id). The raw pointer to the inner - # handler is handed to the dylib as `user_data` for the per-event - # trampoline below. + # Per-listener handler boxes + extern "C" trampolines: the Box is kept alive in `listeners`, its raw pointer is the per-event `user_data`. if events.len > 0: for ev in events: let handlerStruct = capitalizeFirstLetter(ev.nimProcName) & "Handler" @@ -508,23 +470,18 @@ proc generateApiRs*( lines.add("pub struct ListenerHandle { pub id: u64 }") lines.add("") - # ── Context struct ───────────────────────────────────────────────────────── lines.add("/// High-level context for `$1`." % [libTypeName]) lines.add("pub struct $1 {" % [ctxTypeName]) lines.add(" ptr: *mut c_void,") lines.add(" timeout: Duration,") if events.len > 0: - # Keeps each registered handler box alive while its listener id is - # live on the Nim side. Removing an entry from the map drops the - # Box and frees the user's closure; the Nim-side registry has - # already guaranteed no callback for that id is in flight by the - # time `_remove_event_listener` returns. + # Keeps each handler box alive while its listener id is live on the Nim side. lines.add( " listeners: std::sync::Mutex>>," ) lines.add("}") lines.add("") - # SAFETY block applies to both impls below (PR #23 Rust review, item 7). + # SAFETY block applies to both impls below. lines.add( "// SAFETY: The `ptr` field points to an FFIContext owned by the Nim runtime." ) @@ -546,10 +503,7 @@ proc generateApiRs*( lines.add("unsafe impl Sync for $1 {}" % [ctxTypeName]) lines.add("") - # ── Drop: tears down the Nim runtime when the ctx goes out of scope ────── - # Without this, forgetting the ctx leaks the entire Nim runtime (FFI thread, - # watchdog, chronos, lib state). Mirrors the C++ binding's `~$1()` dtor. - # PR #23 review (Rust), Critical item 3. + # Drop tears down the Nim runtime when the ctx goes out of scope; without it, forgetting the ctx leaks the entire runtime (FFI thread, watchdog, chronos). if dtorProcName.len > 0: lines.add("impl Drop for $1 {" % [ctxTypeName]) lines.add(" fn drop(&mut self) {") @@ -557,16 +511,13 @@ proc generateApiRs*( lines.add(" unsafe { ffi::$1(self.ptr); }" % [dtorProcName]) lines.add(" self.ptr = std::ptr::null_mut();") lines.add(" }") - # `listeners` is dropped automatically after this body returns. By - # that point the dylib has joined its threads, so no callback is mid- - # flight against any of the raw pointers we handed it. + # `listeners` drops after this body; the dylib has joined its threads by then, so no callback is mid-flight against the raw pointers we handed it. lines.add(" }") lines.add("}") lines.add("") lines.add("impl $1 {" % [ctxTypeName]) - # ── Constructors ─────────────────────────────────────────────────────────── for ctor in ctors: let reqName = reqStructName(ctor) var paramsList: seq[string] = @[] @@ -580,9 +531,7 @@ proc generateApiRs*( nimTypeToRust(ep.typeName) paramsList.add("$1: $2" % [snake, rustType]) fieldInits.add(snake) - # Both `create` and `new_async` accept an explicit `timeout: Duration`; the - # value flows into `self.timeout` so subsequent method calls inherit it. - # (PR #23 Rust review, item 5: don't hardcode 30s for the async ctor.) + # `create` and `new_async` take an explicit `timeout: Duration` that flows into `self.timeout` so subsequent method calls inherit it. let ctorParamsStr = if paramsList.len > 0: paramsList.join(", ") & ", timeout: Duration" @@ -595,14 +544,10 @@ proc generateApiRs*( else: reqName & " {}" - # -- blocking create -- lines.add(" pub fn create($1) -> Result {" % [ctorParamsStr]) lines.add(" let req = $1;" % [reqLit]) lines.add(" let req_bytes = encode_cbor(&req)?;") - # Ctor C ABI returns *mut c_void synchronously AND fires the callback; - # the callback carries the success/error payload, so discard the - # synchronous return value and yield RET_OK to make the trampoline wait - # on the callback. + # Ctor also fires the callback carrying the payload, so discard the synchronous *mut c_void and yield RET_OK to wait on the callback. lines.add(" let raw_bytes = ffi_call_sync(timeout, |cb, ud| unsafe {") lines.add( " let _ = ffi::$1(req_bytes.as_ptr(), req_bytes.len(), cb, ud);" % @@ -610,7 +555,7 @@ proc generateApiRs*( ) lines.add(" 0") lines.add(" })?;") - # The ctor success payload is a CBOR text string holding the ctx address. + # Ctor success payload is a CBOR text string holding the ctx address. lines.add(" let addr_str: String = decode_cbor(&raw_bytes)?;") lines.add( " let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?;" @@ -624,14 +569,12 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # -- async new_async -- lines.add( " pub async fn new_async($1) -> Result {" % [ctorParamsStr] ) lines.add(" let req = $1;" % [reqLit]) lines.add(" let req_bytes = encode_cbor(&req)?;") - # See `create` above: discard the ctor's *mut c_void synchronous return - # and rely on the callback to deliver the ctx address. + # See `create`: discard the ctor's synchronous return; the callback delivers the ctx address. lines.add(" let raw_bytes = ffi_call_async(timeout, move |cb, ud| unsafe {") lines.add( " let _ = ffi::$1(req_bytes.as_ptr(), req_bytes.len(), cb, ud);" % @@ -652,14 +595,8 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # ── Listener-registration API ───────────────────────────────────────── if events.len > 0: - # Private helper shared by every public `add_*_listener`: the - # FFI call + map insertion is identical across the typed event - # variants, so it lives in one place. The caller owns the box - # (typed as the concrete handler struct so the raw pointer matches - # the trampoline's expected type) and only erases it to - # `dyn Any + Send` when handing ownership over. + # Shared by every public `add_*_listener`: caller owns the concrete-typed box, erased to `dyn Any + Send` only on hand-off. lines.add(" fn add_listener_inner(") lines.add(" &self,") lines.add(" event_name: *const c_char,") @@ -706,8 +643,7 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # Remove by handle. Drops the Box (and the user's closure) after the - # C ABI confirms the listener has been unregistered. + # Remove by handle; drops the Box after the C ABI confirms unregistration. lines.add(" /// Remove a previously-registered listener by handle. Returns true") lines.add(" /// if the listener existed and was removed; false otherwise.") lines.add( @@ -724,7 +660,6 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # ── Methods ──────────────────────────────────────────────────────────────── for m in methods: let methodName = stripLibPrefix(m.procName, libName) let retRustType = nimTypeToRust(m.returnTypeName) @@ -755,7 +690,6 @@ proc generateApiRs*( let retTypeForApi = if m.returnRidesAsPtr(): RustPtrType else: retRustType - # -- blocking method -- lines.add( " pub fn $1(&self$2) -> Result<$3, String> {" % [methodName, paramsStr, retTypeForApi] @@ -772,9 +706,7 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # -- async method -- - # ptr is cast to usize (Copy + Send) so the move closure is Send, - # keeping the returned future Send for multi-threaded tokio runtimes. + # async method: ptr cast to usize (Copy + Send) keeps the move closure and returned future Send for multi-threaded tokio runtimes. lines.add( " pub async fn $1_async(&self$2) -> Result<$3, String> {" % [methodName, paramsStr, retTypeForApi] diff --git a/ffi/codegen/string_helpers.nim b/ffi/codegen/string_helpers.nim index 1834a01..a44eef5 100644 --- a/ffi/codegen/string_helpers.nim +++ b/ffi/codegen/string_helpers.nim @@ -1,21 +1,16 @@ -## Identifier-casing helpers shared by the codegen modules and the FFI macro. -## All three operate on `Rune` via `std/unicode` so non-ASCII identifiers -## (rare in FFI symbols but possible in field names) round-trip correctly. +## Unicode-aware identifier-casing helpers shared by codegen and the FFI macro. import std/[strutils, unicode] proc toLower*(s: string): string = - ## Unicode-aware lowercase for an entire string. Wraps `std/unicode`'s - ## per-Rune `toLower` so callers don't have to iterate manually. + ## Unicode-aware lowercase for an entire string. var buf = "" for r in runes(s): buf.add($r.toLower()) return buf proc camelToSnakeCase*(s: string): string = - ## Converts camelCase to snake_case. Inserts `_` before each uppercase rune - ## that's not the first character and lowercases everything. - ## e.g. "delayMs" → "delay_ms", "timerName" → "timer_name" + ## camelCase → snake_case, e.g. "delayMs" → "delay_ms". var snake = "" var first = true for r in runes(s): @@ -26,8 +21,7 @@ proc camelToSnakeCase*(s: string): string = return snake func capitalizeFirstLetter*(s: string): string = - ## Returns `s` with its first rune uppercased; the rest is left unchanged. - ## e.g. "abc" → "Abc", "" → "", "Abc" → "Abc" + ## Returns `s` with its first rune uppercased, rest unchanged. if s.len == 0: return s var runesSeq = toRunes(s) @@ -35,9 +29,7 @@ func capitalizeFirstLetter*(s: string): string = return $runesSeq proc snakeToPascalCase*(s: string): string = - ## Converts snake_case identifiers to PascalCase: split on `_`, uppercase - ## the first rune of each part, concatenate. - ## e.g. "testlib_create" → "TestlibCreate", "hello_world" → "HelloWorld" + ## snake_case → PascalCase, e.g. "hello_world" → "HelloWorld". let parts = s.split('_') var pascal = "" for p in parts: diff --git a/ffi/codegen/types_ir.nim b/ffi/codegen/types_ir.nim index 6454c8c..feb3cb5 100644 --- a/ffi/codegen/types_ir.nim +++ b/ffi/codegen/types_ir.nim @@ -1,6 +1,5 @@ -## Structured type model shared by the C / C++ / Rust binding generators: one -## parser (`parseFFIType`) for the Nim type strings each backend used to slice -## by hand, plus `renderNative` to walk the result into a backend's type string. +## Structured type model shared by the C / C++ / Rust binding generators: +## `parseFFIType` parses a Nim type string, `renderNative` walks it per backend. import std/[strutils, options] @@ -39,8 +38,7 @@ type discard NativeTypeMap* = object - ## A backend's answer to "what do you call this kind?". `seqOf`/`optOf` - ## wrap an already-rendered element; `structName` maps a user type name. + ## Per-backend type names; `structName` nil ⇒ user type name passes through. scalar*: proc(s: ScalarKind): string {.noSideEffect, nimcall.} str*: string bytes*: string @@ -48,17 +46,14 @@ type seqOf*: proc(elem: string): string {.noSideEffect, nimcall.} optOf*: proc(elem: string): string {.noSideEffect, nimcall.} structName*: proc(name: string): string {.noSideEffect, nimcall.} - ## nil ⇒ the user type name passes through unchanged func genericInnerType*(typeName, prefix: string): string = - ## Inner type of a single-parameter generic `Prefix[Inner]`, e.g. - ## `genericInnerType("seq[int]", "seq[")` → `"int"`; "" if not that shape. + ## Inner type of `Prefix[Inner]`, e.g. ("seq[int]", "seq[") → "int"; "" if no match. if typeName.startsWith(prefix) and typeName.endsWith("]"): return typeName[prefix.len .. ^2] return "" func scalarKind(t: string): Option[ScalarKind] = - ## Single source of truth for the scalar leaf set every backend shares. case t of "bool": some(skBool) @@ -86,9 +81,8 @@ func scalarKind(t: string): Option[ScalarKind] = none(ScalarKind) func parseFFIType*(typeName: string): FFIType = - ## Single source of truth for turning a Nim type string into the shared - ## intermediate representation: - ## ptr/pointer, seq[byte]→bytes, seq/Option/Maybe, scalars, string, else struct. + ## Nim type string → shared `FFIType`: ptr/pointer, seq[byte]→bytes, seq/Option/Maybe, + ## scalars, string, else struct. let t = typeName.strip() if t.startsWith("ptr ") or t == "pointer": return FFIType(kind: ftPtr) @@ -114,7 +108,7 @@ func parseFFIType*(typeName: string): FFIType = FFIType(kind: ftStruct, name: t) func renderNative*(m: NativeTypeMap, t: FFIType): string = - ## Recursively walks `t` into a native type string for the backend `m`. + ## Recursively walks `t` into a native type string for backend `m`. case t.kind of ftScalar: m.scalar(t.scalar) diff --git a/ffi/event_thread.nim b/ffi/event_thread.nim index 21ccdb9..af8376d 100644 --- a/ffi/event_thread.nim +++ b/ffi/event_thread.nim @@ -1,13 +1,6 @@ -## Event-thread body and FFI-thread liveness monitoring. -## -## Included from `ffi_context.nim` — inherits its imports, FFIContext type, -## and the heartbeat-timing constants. Lives alongside `ffi_thread.nim` -## so each thread's machinery is readable on its own. -## -## Responsibilities: -## - Drain queued events into listener callbacks. -## - Watch `ctx.ffiHeartbeat` and emit `NotRespondingEvent` / `RespondingEvent` -## on FFI-thread stall and recovery transitions. +## Event-thread body and FFI-thread liveness monitoring. Included from +## `ffi_context.nim`. Drains queued events into listeners and emits +## NotResponding/Responding on FFI-heartbeat stall/recovery. type NotRespondingEvent* = object @@ -20,8 +13,8 @@ const proc dispatchToListeners[T]( ctx: ptr FFIContext[T], eventName: string, data: pointer, dataLen: int ) = - ## Holds reg.lock for the entire snapshot + invocation so concurrent - ## add/remove on this registry blocks until dispatch returns. + ## Holds reg.lock across snapshot + invocation so concurrent add/remove blocks + ## until dispatch returns. withLock ctx[].eventRegistry.lock: let listeners = ctx[].eventRegistry.byEvent.getOrDefault(eventName) if listeners.len == 0: @@ -37,8 +30,7 @@ proc dispatchToListeners[T]( ) proc emitLivenessEvent[T, P](ctx: ptr FFIContext[T], name: string, payload: P) = - ## Encodes a liveness event and dispatches directly to listeners (bypassing - ## the queue, which may be wedged). Runs on the event thread. + ## Dispatches directly to listeners, bypassing the (possibly wedged) queue. let event = try: EventEnvelope[P](eventType: name, payload: payload).cborEncode() @@ -57,18 +49,15 @@ proc onNotResponding*(ctx: ptr FFIContext) = proc onResponding*(ctx: ptr FFIContext) = ## Fired once when the heartbeat resumes after a NotRespondingEvent. - ## Lets consumers clear any "library hung" UI state without polling. emitLivenessEvent(ctx, RespondingEventName, RespondingEvent()) proc dispatchQueuedEvent[T](ctx: ptr FFIContext[T], qe: QueuedEvent) = - ## Reads the borrowed slab payload; `commitDequeue` (not this proc) frees any - ## heap-fallback buffer once the read has returned. + ## Reads the borrowed slab payload; `commitDequeue` frees any heap fallback. ctx.dispatchToListeners($qe.name, qe.data, qe.dataLen) proc drainOneEvent[T](ctx: ptr FFIContext[T]): bool = - ## Peek → dispatch → commit for a single event. The slot stays pinned across - ## dispatch so the producer can't reuse its slab buffer mid-read; `defer` - ## commits even if a listener raises. Returns false when the queue is empty. + ## Peek → dispatch → commit; slot stays pinned across dispatch, `defer` commits + ## even if a listener raises. False when the queue is empty. let opt = ctx.eventQueue.peekEvent() if opt.isNone(): return false @@ -97,8 +86,7 @@ proc init(T: type HeartbeatMonitor, ctx: ptr FFIContext): T = ) proc check[T](hb: var HeartbeatMonitor, ctx: ptr FFIContext[T]) = - ## Fires onNotResponding / onResponding on heartbeat stall / recovery. - ## Both transitions latch — each fires at most once per stall episode. + ## Fires onNotResponding/onResponding on stall/recovery; each latches once per episode. if Moment.now() - hb.startedAt <= FFIHeartbeatStartDelay: return let cur = ctx.ffiHeartbeat.load() @@ -116,17 +104,15 @@ proc eventRun[T](ctx: ptr FFIContext[T]) {.async.} = var hb = HeartbeatMonitor.init(ctx) var notifiedStuck = false # latched forever — eventQueueStuck is sticky terminal. - # Keep draining after `running` flips false until the FFI thread has exited, so - # events emitted by an async {.ffiDtor.} teardown are still dispatched. + # Keep draining after `running` flips false until the FFI thread exits, so events from an async {.ffiDtor.} teardown are still dispatched. while ctx.running.load() or not ctx.ffiThreadExited.load(): - # Wake on enqueue or tick — whichever first. discard await ctx.eventQueueSignal.wait().withTimeout(EventThreadTickInterval) ctx.drainEventQueue() - # Liveness only applies while running; skip it during the teardown drain. + # Liveness only while running; skip during the teardown drain. if ctx.running.load(): - # Fire after drain so reg.lock is free — FFI-thread would deadlock here. + # Fire after drain so reg.lock is free (FFI thread would deadlock here). if not notifiedStuck and ctx.eventQueueStuck.load(): onNotResponding(ctx) notifiedStuck = true @@ -137,8 +123,6 @@ proc eventRun[T](ctx: ptr FFIContext[T]) {.async.} = proc eventThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = ## Drains the event queue and runs the FFI-thread heartbeat check. - ## Borrows each queued slab payload until dispatch returns, then releases - ## any heap-fallback buffer. defer: let fireRes = ctx.eventThreadExitSignal.fireSync() if fireRes.isErr(): diff --git a/ffi/ffi_context.nim b/ffi/ffi_context.nim index 7833188..7584f84 100644 --- a/ffi/ffi_context.nim +++ b/ffi/ffi_context.nim @@ -1,8 +1,4 @@ ## FFIContext type plus lifecycle (init / signal-stop / join / destroy). -## -## The per-thread bodies live in `ffi_thread.nim` and `event_thread.nim`, -## included below so the thread code can access the private FFIContext -## fields without forcing them through a public surface. {.passc: "-fPIC".} @@ -23,55 +19,42 @@ type FFIContext*[T] = object myLib*: ptr T # main library object (Waku, LibP2P, SDS, …) ffiThread: Thread[(ptr FFIContext[T])] eventThread: Thread[(ptr FFIContext[T])] - reqQueueBank: RequestQueueBank # mutex-guarded MPSC ingress from foreign threads - reqSignal: ThreadSignalPtr # wakes the FFI thread on enqueue + reqQueueBank: RequestQueueBank + reqSignal: ThreadSignalPtr stopSignal: ThreadSignalPtr threadExitSignal: ThreadSignalPtr - # bounds destroyFFIContext's wait so a blocked loop cannot hang the caller - eventQueueSignal: ThreadSignalPtr # wakes the event thread on enqueue - eventThreadExitSignal: ThreadSignalPtr # mirrors threadExitSignal for the event thread + eventQueueSignal: ThreadSignalPtr + eventThreadExitSignal: ThreadSignalPtr userData*: pointer eventRegistry*: FFIEventRegistry - handles*: FFIHandleRegistry # live {.ffiHandle.} objects, keyed by uint64 id + handles*: FFIHandleRegistry eventQueue*: EventQueue ffiHeartbeat*: Atomic[int64] - # advanced each FFI-thread loop; event thread reads for liveness - eventQueueStuck*: Atomic[bool] # sticky overflow flag + eventQueueStuck*: Atomic[bool] ffiThreadExited*: Atomic[bool] - # set once the FFI thread (including any async {.ffiDtor.} teardown) is done; - # keeps the event thread draining until then so teardown-emitted events land - running: Atomic[bool] # To control when the threads are running + # set once FFI thread (incl. async {.ffiDtor.}) is done; event thread drains until then + running: Atomic[bool] registeredRequests: ptr Table[cstring, FFIRequestProc] staleWarnInterval*: Duration - # RET_STALE_WARN cadence. An internal seam (tests tune it) — deliberately - # not exposed to the ffi dev, and there is no per-proc override. var onFFIThread* {.threadvar.}: bool - # Re-entrant dispatch guard for `sendRequestToFFIThread`. const git_version* {.strdefine.} = "n/a" const EventThreadTickInterval* = 1.seconds - FFIHeartbeatStartDelay* = 10.seconds # grace window for library startup + FFIHeartbeatStartDelay* = 10.seconds FFIHeartbeatStaleThreshold* = 1.seconds const StaleWarnIntervalMs* {.intdefine: "ffiStaleWarnIntervalMs".} = 5000 - ## `RET_STALE_WARN` cadence, fired without limit — nim-ffi never times a - ## handler out. 5s mirrors Android's ANR input timeout. Override with - ## `-d:ffiStaleWarnIntervalMs=`. + ## `RET_STALE_WARN` cadence; handlers are never timed out. const StaleWarnInterval* = StaleWarnIntervalMs.milliseconds type FFITeardownProc*[T] = proc(lib: ptr T): Future[void] {.async.} proc ffiTeardownHook*[T](): var FFITeardownProc[T] = - ## Per-library teardown slot (one `{.global.}` per `T`), assigned at module init - ## by a non-empty `{.ffiDtor.}` and awaited by the FFI thread before it exits. - ## - ## A runtime slot, not an overloaded `ffiTeardown` resolved via `mixin`: the - ## constructor force-instantiates the FFI thread body before the dtor (declared - ## later in the source) is visible, so an overload would bind the no-op default - ## and the teardown would silently never run. + ## Per-library teardown slot (one `{.global.}` per `T`), awaited by the FFI thread before exit. + ## Runtime slot not an overload: an overload would bind the no-op default before the dtor is visible. var hook {.global.}: FFITeardownProc[T] hook @@ -84,18 +67,13 @@ template closeAndNil(field: untyped) = field = nil proc deinitContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## Mirror of `initContextResources`. Threads MUST be joined first (FFI thread - ## drained); fields are nil'd after close so re-init on the same slot is safe. - ## `deinitRequestQueue` frees any request raced in after the final drain. + ## Mirror of `initContextResources`. Threads MUST be joined first; fields nil'd after close. deinitRequestQueue(ctx[].reqQueueBank) deinitEventRegistry(ctx[].eventRegistry) deinitHandleRegistry(ctx[].handles) deinitEventQueue(ctx[].eventQueue) when defined(gcRefc): - # ThreadSignalPtr.close() under refc traps in safeUnregisterAndCloseFd - # → newDispatcher → rawNewObj → signal-handler re-entry (process hangs). - # See tests/test_ffi_context.nim "destroyFFIContext refc workaround". - # Fd leak is bounded — destroy runs once per process lifetime. + # ThreadSignalPtr.close() under refc hangs via signal-handler re-entry; leak the bounded fd. discard else: closeAndNil(ctx.reqSignal) @@ -106,7 +84,6 @@ proc deinitContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] = ok() proc cleanUpResources[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## Deinit + free for heap-allocated contexts. defer: freeShared(ctx) ctx.deinitContextResources() @@ -116,8 +93,7 @@ template newSignalOrErr(field: untyped, name: string) = return err("couldn't create ThreadSignalPtr: " & name & ": " & $error) proc initContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## On failure, the deferred cleanup closes partial state; caller releases - ## the slot (freeShared or pool.releaseSlot). + ## On failure, deferred cleanup closes partial state; caller releases the slot. # Nil first so deferred cleanup can't double-close a reused pool slot. ctx.reqSignal = nil ctx.stopSignal = nil @@ -187,28 +163,21 @@ proc waitExitOrErr( ok() proc signalStop*[T](ctx: ptr FFIContext[T]): Result[void, string] = - # Skip onNotResponding on error: it takes reg.lock, which a back-pressuring - # listener may hold — would deepen the stuck state into a deadlock. + # Skip onNotResponding on error: it takes reg.lock a stuck listener may hold (deadlock risk). ctx.running.store(false) ?ctx.reqSignal.fireOrErr("reqSignal") ?ctx.stopSignal.fireOrErr("stopSignal") - # Non-fatal: event thread sees running==false on the next tick anyway. ctx.eventQueueSignal.fireOrErr("eventQueueSignal").isOkOr: error "failed to signal eventQueueSignal in signalStop", error = error ok() -## Bound on how long stopAndJoinThreads waits for a worker thread to exit before -## leaking ctx rather than hanging the caller. Configurable because an async -## `{.ffiDtor.}` runs its teardown (e.g. `switch.stop()` over many live -## connections) on the FFI thread before it exits — a graceful shutdown can -## outlast the default, and being cut short leaks the context instead of waiting. -## Override at compile time with `-d:ffiThreadExitTimeoutMs=`. +## Per-thread exit wait before stopAndJoinThreads leaks ctx rather than hanging; async +## `{.ffiDtor.}` teardown can outlast the default. Override `-d:ffiThreadExitTimeoutMs=`. const ThreadExitTimeoutMs* {.intdefine: "ffiThreadExitTimeoutMs".} = 1500 const ThreadExitTimeout* = ThreadExitTimeoutMs.milliseconds proc stopAndJoinThreads*[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## On timeout, returns err and skips remaining joins (leaves threads live). - ## Caller owns resource cleanup. Skips onNotResponding (same reason as signalStop). + ## On timeout, returns err and skips remaining joins (leaves threads live); caller cleans up. ctx.signalStop().isOkOr: return err("signalStop failed: " & $error) @@ -219,7 +188,6 @@ proc stopAndJoinThreads*[T](ctx: ptr FFIContext[T]): Result[void, string] = ok() proc clearContext[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## Stops a heap-allocated FFI context. ctx.stopAndJoinThreads().isOkOr: return err("clearContext: " & $error) ctx.cleanUpResources().isOkOr: diff --git a/ffi/ffi_context_pool.nim b/ffi/ffi_context_pool.nim index 5e2d2cb..ac8ab0c 100644 --- a/ffi/ffi_context_pool.nim +++ b/ffi/ffi_context_pool.nim @@ -3,7 +3,6 @@ import results import ./ffi_context const MaxFFIContexts* = 32 - # Only affects upfront pool memory; fds/threads consumed per acquired slot. type FFIContextPool*[T] = object ## Fixed pool. Bounds ThreadSignalPtr fds at MaxFFIContexts * 2. @@ -36,7 +35,7 @@ proc createFFIContext*[T]( proc destroyFFIContext*[T]( pool: var FFIContextPool[T], ctx: ptr FFIContext[T] ): Result[void, string] = - ## On thread-exit timeout the slot is leaked — closing live-thread resources is unsafe. + ## On thread-exit timeout the slot is leaked; closing live-thread resources is unsafe. ctx.stopAndJoinThreads().isOkOr: return err("destroyFFIContext(pool): " & $error) # Required: next acquisition would otherwise re-init a live lock (UB). @@ -47,7 +46,7 @@ proc destroyFFIContext*[T]( ok() proc isValidCtx*[T](pool: var FFIContextPool[T], ctx: pointer): bool = - ## Rejects nil / offset-invalid / dangling pointers at the API boundary. + ## Rejects nil / dangling pointers at the API boundary. if ctx.isNil(): return false for i in 0 ..< MaxFFIContexts: diff --git a/ffi/ffi_events.nim b/ffi/ffi_events.nim index cb51f81..0131875 100644 --- a/ffi/ffi_events.nim +++ b/ffi/ffi_events.nim @@ -1,6 +1,5 @@ -## Per-context event registry + bounded SPSC queue. FFI thread enqueues, -## event thread drains; payloads travel via `c_malloc` so they survive -## pool-slot reuse across thread heaps. +## Per-context event registry + bounded SPSC queue. FFI thread enqueues, event +## thread drains; payloads use c_malloc so they survive cross-thread heap reuse. {.pragma: callback, cdecl, raises: [], gcsafe.} @@ -25,15 +24,13 @@ type byEvent*: Table[string, seq[FFIEventListener]] proc initEventRegistry*(reg: var FFIEventRegistry) = - ## Must run once on the owning thread before sharing — `initLock` on a - ## live primitive is UB at the OS layer. + ## Run once on the owning thread before sharing (re-initLock is UB). reg.lock.initLock() reg.nextId = 0'u64 reg.byEvent = initTable[string, seq[FFIEventListener]]() proc deinitEventRegistry*(reg: var FFIEventRegistry) = - ## Mirror of `initEventRegistry`; same single-thread constraint. Resets GC - ## fields so pool-slot reuse on another thread sees no hidden dtor. + ## Mirror of `initEventRegistry`; resets GC fields so slot reuse sees no dtor. reg.lock.deinitLock() reg.byEvent = default(Table[string, seq[FFIEventListener]]) reg.nextId = 0'u64 @@ -59,8 +56,7 @@ proc addEventListener*( assigned proc removeEventListener*(reg: var FFIEventRegistry, id: uint64): bool {.raises: [].} = - ## Safe to call from inside a dispatch — the in-flight snapshot still - ## delivers exactly once to the removed listener. + ## Safe from inside a dispatch; the in-flight snapshot still delivers once. if id == 0'u64: return false @@ -91,8 +87,7 @@ proc removeAllEventListeners*(reg: var FFIEventRegistry) {.raises: [].} = proc snapshotListeners*( reg: var FFIEventRegistry, eventName: string ): seq[FFIEventListener] {.raises: [].} = - ## Lock held only across the copy — keeps re-entrant add/remove - ## from a handler deadlock-free. + ## Lock held only across the copy so re-entrant add/remove can't deadlock. var listeners: seq[FFIEventListener] = @[] withLock reg.lock: for l in reg.byEvent.getOrDefault(eventName): @@ -100,33 +95,23 @@ proc snapshotListeners*( listeners const EventQueueCapacity* {.intdefine.} = 1024 - ## Sustained backlog at this depth means a listener is wedged. Compile-time - ## per-library override: `-d:EventQueueCapacity=N`. + ## Sustained backlog here means a listener is wedged. Override `-d:EventQueueCapacity=N`. const MaxEventPayloadBytes* {.intdefine.} = 512 - ## Per-slot payload slab budget. Payloads up to this size copy into a - ## preallocated, reused buffer (zero steady-state allocation); larger ones - ## fall back to a one-off `c_malloc` freed on commit. Compile-time - ## per-library override: `-d:MaxEventPayloadBytes=N`. + ## Per-slot payload slab; larger payloads take a one-off c_malloc freed on + ## commit. Override `-d:MaxEventPayloadBytes=N`. const MaxEventNameBytes* {.intdefine.} = 64 - ## Per-slot name slab budget (incl. NUL). Event names are short compile-time - ## literals; anything longer takes the same heap fallback as the payload. - ## Compile-time per-library override: `-d:MaxEventNameBytes=N`. + ## Per-slot name slab (incl. NUL); longer names take the heap fallback. + ## Override `-d:MaxEventNameBytes=N`. const emptyListenerPayload*: cstring = "" - ## Non-nil zero-length buffer handed to listeners when the payload is empty - ## (a nil pointer would be UB for consumers doing `memcpy` even at len 0). - ## Also the stand-in name for a nil/empty event name. + ## Non-nil zero-length stand-in for empty payloads/names (nil would be UB for + ## consumers doing memcpy even at len 0). type QueuedEvent* = object - # `name`/`data` point into the queue's reused per-slot buffers (not freed - # per-event) unless the value didn't fit that slot's budget, in which case - # the corresponding `*HeapOwned` flag marks a one-off `c_malloc` freed on - # commit. Both buffers are `c_malloc`-backed so the event thread can read - # them after the producing FFI thread's heap is gone (same TLS hazard as - # `alloc.nim`). + # `name`/`data` point into reused per-slot buffers, or a one-off c_malloc marked by `*HeapOwned` when oversize; both c_malloc'd so they outlive the FFI thread's heap. name*: cstring nameHeapOwned*: bool data*: ptr UncheckedArray[byte] @@ -139,8 +124,8 @@ type tail*: int count*: int buf*: array[EventQueueCapacity, QueuedEvent] - slab*: array[EventQueueCapacity, ptr UncheckedArray[byte]] # payload buffers - nameSlab*: array[EventQueueCapacity, ptr UncheckedArray[byte]] # name buffers + slab*: array[EventQueueCapacity, ptr UncheckedArray[byte]] + nameSlab*: array[EventQueueCapacity, ptr UncheckedArray[byte]] proc allocSlot(nbytes: int): ptr UncheckedArray[byte] {.raises: [].} = if nbytes <= 0: @@ -158,7 +143,7 @@ proc initEventQueue*(q: var EventQueue) {.raises: [].} = q.nameSlab[i] = allocSlot(MaxEventNameBytes) proc releaseEvent*(qe: QueuedEvent) {.raises: [], gcsafe.} = - ## Frees only the heap-fallback buffers. Reused slot buffers persist. + ## Frees only heap-fallback buffers; reused slot buffers persist. if qe.nameHeapOwned and not qe.name.isNil(): c_free(cast[pointer](qe.name)) if qe.dataHeapOwned and not qe.data.isNil(): @@ -167,7 +152,7 @@ proc releaseEvent*(qe: QueuedEvent) {.raises: [], gcsafe.} = proc deinitEventQueue*(q: var EventQueue) {.raises: [].} = ## Both producer and consumer must have stopped. for i in 0 ..< EventQueueCapacity: - releaseEvent(q.buf[i]) # free any undrained heap-fallback buffers + releaseEvent(q.buf[i]) q.buf[i] = QueuedEvent() if not q.slab[i].isNil(): c_free(q.slab[i]) @@ -183,9 +168,8 @@ proc deinitEventQueue*(q: var EventQueue) {.raises: [].} = proc copyIntoSlot( slot: ptr UncheckedArray[byte], slotCap, nbytes: int, src: pointer ): tuple[buf: ptr UncheckedArray[byte], heap: bool, ok: bool] {.raises: [].} = - ## Copies `nbytes` from `src` into the reusable `slot` when they fit, else a - ## one-off `c_malloc`. `ok=false` only on allocation failure; `nbytes<=0` - ## yields `(nil, false, true)` with no copy. + ## Copies into `slot` when it fits, else a one-off c_malloc; `ok=false` only on + ## alloc failure. if nbytes <= 0: return (nil, false, true) if nbytes <= slotCap and not slot.isNil(): @@ -200,16 +184,13 @@ proc copyIntoSlot( proc tryEnqueueEvent*( q: var EventQueue, name: cstring, src: pointer, dataLen: int ): bool {.raises: [], gcsafe.} = - ## Copies `name` (NUL included) and `dataLen` payload bytes from `src` into - ## the tail slot's reused buffers, or a heap fallback when either overflows - ## its slot budget. Returns false (nothing enqueued) when the ring is full or - ## a fallback allocation fails. + ## Copies `name` (NUL included) and payload into the tail slot's reused buffers + ## or a heap fallback; false when the ring is full or a fallback alloc fails. withLock q.lock: if q.count >= EventQueueCapacity: return false let slot = q.tail - # Copy the name *including* its NUL (a non-nil cstring is terminated) so the - # stored copy stays a valid cstring; a nil/empty name uses the static stand-in. + # Include the NUL so the stored copy stays a valid cstring. let nameBytes = if name.isNil(): 0 @@ -222,7 +203,7 @@ proc tryEnqueueEvent*( let dataRes = copyIntoSlot(q.slab[slot], MaxEventPayloadBytes, dataLen, src) if not dataRes.ok: if nameRes.heap: - c_free(nameRes.buf) # unwind the name fallback we just took + c_free(nameRes.buf) return false let nameCStr = if nameRes.buf.isNil(): @@ -241,18 +222,15 @@ proc tryEnqueueEvent*( true proc peekEvent*(q: var EventQueue): Option[QueuedEvent] {.raises: [], gcsafe.} = - ## Returns the head event *without* advancing — the slot stays counted so the - ## single-producer can't reuse its slab buffer while the consumer is still - ## reading it. Pair every non-none `peekEvent` with a `commitDequeue` once - ## dispatch has returned. The returned event borrows the slab slot. + ## Returns the head without advancing (slot stays pinned so the producer can't + ## reuse it mid-read); pair each non-none peek with a `commitDequeue`. withLock q.lock: if q.count == 0: return none(QueuedEvent) return some(q.buf[q.head]) proc commitDequeue*(q: var EventQueue) {.raises: [], gcsafe.} = - ## Retires the head slot after its `peekEvent` was dispatched: frees any - ## heap-fallback payload, clears the slot, and only now frees it for reuse. + ## Retires the dispatched head slot: frees any heap fallback and frees the slot. withLock q.lock: if q.count == 0: return @@ -268,8 +246,7 @@ proc eventQueueLen*(q: var EventQueue): int {.raises: [], gcsafe.} = proc notifyListeners*( listeners: seq[FFIEventListener], retCode: cint, data: pointer, dataLen: int ) = - ## Empty payloads go through `emptyListenerPayload` so consumers doing - ## `std::string(data, len)` / `memcpy` never see a nil pointer. + ## Empty payloads use `emptyListenerPayload` so consumers never see a nil ptr. let n = max(dataLen, 0) let dataPtr = if n > 0 and not data.isNil(): @@ -288,7 +265,6 @@ proc notifyListenersErr*(listeners: seq[FFIEventListener], msg: string) = notifyListeners(listeners, RET_ERR, p, msg.len) var ffiCurrentEventRegistry* {.threadvar.}: ptr FFIEventRegistry - # Kept for tests that drive the registry directly. var ffiCurrentEventQueue* {.threadvar.}: ptr EventQueue # Installed by the FFI thread so dispatch templates need no `ctx`. @@ -297,14 +273,11 @@ var ffiCurrentEventQueueStuck* {.threadvar.}: ptr Atomic[bool] # Sticky overflow flag; FFI request entry point reads it to reject. var ffiCurrentNotifyEventEnqueued* {.threadvar.}: proc() {.gcsafe, raises: [].} - # Hook so this module doesn't depend on chronos's ThreadSignalPtr. - # Nil-safe; tick-driven tests leave it unset. + # Wake hook so this module needn't depend on chronos; nil-safe. template enqueueOrMarkStuck(eventName: string, src: pointer, dataLen: int) = - ## Copies `eventName` and `dataLen` bytes from `src` into the queue's reused - ## slot buffers. On queue-full sets the sticky stuck flag and wakes the event - ## thread (firing onNotResponding from here would risk deadlock against a - ## back-pressuring listener). + ## Enqueues into the reused slot buffers; on queue-full sets the sticky stuck + ## flag and wakes the event thread (firing onNotResponding here could deadlock). block enqueueBlock: let q = ffiCurrentEventQueue if q.isNil(): @@ -322,9 +295,7 @@ template enqueueOrMarkStuck(eventName: string, src: pointer, dataLen: int) = ffiCurrentNotifyEventEnqueued() template dispatchFFIEvent*(eventName: string, body: untyped) = - ## `body` must yield `string` / `seq[byte]`. FFI thread only: copies the - ## bytes into the tail slot (slab, or a heap fallback when oversize) and - ## enqueues; the event thread fans out. + ## `body` yields string/seq[byte]. FFI thread only: enqueues; event thread fans out. block: let evtName: string = eventName let bodyVal = body @@ -337,8 +308,8 @@ template dispatchFFIEvent*(eventName: string, body: untyped) = enqueueOrMarkStuck(evtName, src, dataLen) template dispatchFFIEventCbor*(eventName: string, eventPayload: typed) = - ## Typed CBOR variant of `dispatchFFIEvent`. The param is `eventPayload` - ## (not `payload`) to avoid clobbering `EventEnvelope.payload` substitution. + ## Typed CBOR variant; param is `eventPayload` to avoid clobbering + ## `EventEnvelope.payload` substitution. block: let evtName: string = eventName let encoded = cborEncode( diff --git a/ffi/ffi_handles.nim b/ffi/ffi_handles.nim index 0052805..22a598a 100644 --- a/ffi/ffi_handles.nim +++ b/ffi/ffi_handles.nim @@ -1,16 +1,12 @@ -## Per-context registry of live `{.ffiHandle.}` objects. The object stays here, -## in `FFIContext.handles`; only its `uint64` id crosses the boundary. Ids are -## monotonic and never recycled (0 = null), so a stale/forged id misses cleanly. -## FFI-thread-only access, so no locking. +## Per-context registry of live `{.ffiHandle.}` objects; only the `uint64` id crosses the +## boundary. Ids are monotonic, never recycled (0 = null). FFI-thread-only, so no locking. import std/tables import results import ./cbor_serial type - FFIHandleRoot* = ref object of RootObj - ## Base every `{.ffiHandle.}` type inherits from, so handle refs are storable - ## under one static type. + FFIHandleRoot* = ref object of RootObj ## Base of every `{.ffiHandle.}` type. FFIHandleEntry = object obj: FFIHandleRoot @@ -31,7 +27,6 @@ proc deinitHandleRegistry*(reg: var FFIHandleRegistry) = proc register*( reg: var FFIHandleRegistry, obj: FFIHandleRoot, typeName: string ): uint64 = - ## Stores `obj`, returns its fresh handle id (>0). reg.nextId.inc() reg.byHandle[reg.nextId] = FFIHandleEntry(obj: obj, typeName: typeName) reg.nextId @@ -51,16 +46,15 @@ proc lookup*( ok(entry.obj) proc release*(reg: var FFIHandleRegistry, handle: uint64): bool {.discardable.} = - ## Drops the entry; true iff it existed. if not reg.byHandle.hasKey(handle): return false reg.byHandle.del(handle) return true proc releaseAll*(reg: var FFIHandleRegistry) = - ## Drops every entry. Must run on the FFI thread that allocated the refs. + ## Must run on the FFI thread that allocated the refs. reg.byHandle.clear() proc encodeHandle*(id: uint64): seq[byte] = - ## Wire-form bytes for a handle id. Single ABI seam for future format changes. + ## Single ABI seam for the handle-id wire format. cborEncode(id) diff --git a/ffi/ffi_request_queue.nim b/ffi/ffi_request_queue.nim index b553f33..6e50d4f 100644 --- a/ffi/ffi_request_queue.nim +++ b/ffi/ffi_request_queue.nim @@ -1,32 +1,19 @@ -## Sharded, mutex-guarded MPSC ingress for `ptr FFIThreadRequest`: foreign -## threads enqueue without serialising against each other. -## -## Why sharded: one shared queue funnels all producers through a single cache -## line, capping submit throughput. N independent queues (one per producer) -## remove that hotspot — producers contend only when two pick the same queue. -## -## Each queue is an intrusive FIFO under its own `Lock`: race-free under TSAN, and -## the request is its own node (intrusive `next`), so enqueue never allocates nor -## touches a Nim GC heap (the cross-thread `MemRegion` hazard). -## -## FIFO holds per queue, not globally. Unbounded by design: submit never blocks -## or rejects; completion comes via each request's callback. +## Sharded, mutex-guarded MPSC ingress for `ptr FFIThreadRequest`: N intrusive +## FIFOs (one per producer) spread lock contention; the request is its own node +## so enqueue never touches a Nim GC heap. Unbounded — submit never blocks. import std/[atomics, locks] import ./ffi_thread_request const RequestQueueCount* = 16 - ## Independent ingress queues. ≥ the expected concurrent producer count keeps - ## queue collisions (hence lock contention) near zero. + ## Independent ingress queues; ≥ concurrent producer count keeps collisions low. QueuePadBytes = 192 - ## Pads each queue well past a cache line (128B on Apple silicon) so adjacent - ## queues' hot fields never false-share — false sharing would re-serialise - ## exactly what the sharding is meant to spread out. + ## Pads each queue past a cache line (128B on Apple silicon) to avoid false + ## sharing between adjacent queues. static: - # `myQueueIndex` maps threads to queues with an `and` mask, so the count must - # be a power of two — otherwise the distribution silently skews onto a subset. + # `myQueueIndex` masks with `and`, so the count must be a power of two. doAssert (RequestQueueCount and (RequestQueueCount - 1)) == 0, "RequestQueueCount must be a power of two" @@ -34,7 +21,7 @@ type RequestQueue = object lock: Lock head: ptr FFIThreadRequest ## consumer pops here (oldest) - tail: ptr FFIThreadRequest ## producers on this queue append here (newest) + tail: ptr FFIThreadRequest ## producers append here (newest) pad: array[QueuePadBytes, byte] RequestQueueBank* = object @@ -43,14 +30,13 @@ type var gRequestQueue {.threadvar.}: int var gRequestQueueAssigned {.threadvar.}: bool var gRequestQueueCounter: Atomic[int] - ## Hands each producer thread a distinct queue round-robin on first use, so - ## queues fill evenly regardless of OS thread-id distribution. + ## Round-robins producers onto distinct queues on first use so they fill evenly. proc myQueueIndex(): int {.raises: [].} = if not gRequestQueueAssigned: gRequestQueue = gRequestQueueCounter.fetchAdd(1) gRequestQueueAssigned = true - return gRequestQueue and (RequestQueueCount - 1) # RequestQueueCount is a power of two + return gRequestQueue and (RequestQueueCount - 1) proc initRequestQueue*(bank: var RequestQueueBank) {.raises: [].} = for queue in bank.queues.mitems: @@ -59,9 +45,8 @@ proc initRequestQueue*(bank: var RequestQueueBank) {.raises: [].} = queue.tail = nil proc deinitRequestQueue*(bank: var RequestQueueBank) {.raises: [].} = - ## Both producers and the consumer must have stopped. Frees any request still - ## queued on any queue — e.g. one a producer raced in after the FFI thread's - ## final drain — so a teardown race leaks nothing instead of dangling them. + ## Both producers and consumer must have stopped. Frees any still-queued request + ## (e.g. one raced in after the final drain) so a teardown race leaks nothing. for queue in bank.queues.mitems: var request = queue.head while not request.isNil(): @@ -75,9 +60,8 @@ proc deinitRequestQueue*(bank: var RequestQueueBank) {.raises: [].} = proc pushRequest*( bank: var RequestQueueBank, request: ptr FFIThreadRequest ): bool {.raises: [].} = - ## Append `request` to this producer thread's queue (takes ownership). Returns - ## true only when the queue was empty: the consumer sleeps on an empty queue, so - ## that's the one push that must wake it; a missed wake just waits the 100ms poll. + ## Append `request` to this thread's queue (takes ownership). True only when the + ## queue was empty — the one push that must wake the sleeping consumer. request[].next = nil let idx = myQueueIndex() withLock bank.queues[idx].lock: @@ -90,9 +74,8 @@ proc pushRequest*( return wasEmpty proc mergeQueues*(bank: var RequestQueueBank): ptr FFIThreadRequest {.raises: [].} = - ## Single-consumer: splice every queue into one chain, resetting them to empty. - ## Returns nil when all are empty; the caller then owns the chain and must read - ## each request's `next` before dispatching (dispatch frees the request). + ## Single-consumer: splice every queue into one chain and reset them. Caller owns + ## the chain and must read each `next` before dispatch (dispatch frees the request). var head: ptr FFIThreadRequest = nil var tail: ptr FFIThreadRequest = nil for queue in bank.queues.mitems: diff --git a/ffi/ffi_thread.nim b/ffi/ffi_thread.nim index 61076ef..aa8577f 100644 --- a/ffi/ffi_thread.nim +++ b/ffi/ffi_thread.nim @@ -1,14 +1,6 @@ -## FFI-thread body and request submission API. -## -## Included from `ffi_context.nim` — inherits its imports, FFIContext type, -## and the `onFFIThread` threadvar. Companion to `event_thread.nim`. -## -## Responsibilities: -## - Receive `FFIThreadRequest`s from foreign threads via `reqQueueBank` (a -## mutex-guarded MPSC queue) and dispatch them through the user-registered -## handler table. -## - Advance `ctx.ffiHeartbeat` each loop iteration so the event thread can -## detect a wedged FFI thread. +## FFI-thread body and request submission API. Included from `ffi_context.nim`. +## Dispatches `FFIThreadRequest`s from `reqQueueBank` and advances +## `ctx.ffiHeartbeat` so the event thread can spot a wedged FFI thread. proc sendRequestToFFIThread*( ctx: ptr FFIContext, ffiRequest: ptr FFIThreadRequest @@ -18,25 +10,16 @@ proc sendRequestToFFIThread*( return err("event queue stuck - library cannot accept new requests") if onFFIThread: - # A handler re-dispatching onto its own FFI thread would enqueue work the - # blocked dispatcher can never drain; reject instead of dead-locking. + # A handler re-dispatching onto its own FFI thread would deadlock; reject. deleteRequest(ffiRequest) return err( "reentrant ffi call: a handler invoked sendRequestToFFIThread on its own context" ) - # The lock inside pushRequest covers only the O(1) enqueue; the wake stays - # outside it, so concurrent producers don't serialise. Unbounded, so enqueue - # can't fail — completion comes via the request's own callback, no accept-ack. - # - # Wake only when the push found the queue empty: while the consumer drains, a - # fireSync() syscall per submit (contended across producers) is what destroys - # scaling. A skipped wake can't strand the request — the consumer re-polls 100ms. + # Wake only when the push found the queue empty: waking per submit kills scaling, and a skipped wake just waits the consumer's 100ms poll. let shouldWake = ctx.reqQueueBank.pushRequest(ffiRequest) - # A failed wake is non-fatal: the request is queued and the poll-drain - # dispatches it within a tick anyway. Returning err would double-fire the - # caller's callback for a request that still completes. + # A failed wake is non-fatal (poll-drain still dispatches); erroring here would double-fire the callback for a request that still completes. if shouldWake: ctx.reqSignal.fireSync().isOkOr: error "failed to wake FFI thread after enqueue (request still queued)", @@ -50,20 +33,16 @@ proc awaitWithStaleWarnings( interval: Duration, reqId: string, ): Future[Result[seq[byte], string]] {.async.} = - ## Pings the caller with RET_STALE_WARN every `interval` while the handler - ## runs, then returns its real result. Never cancels it: a hard-cancel mid-call - ## into the underlying library (Waku/libp2p) can leave it partially applied, so - ## the caller is kept informed and decides for itself. The timer lives entirely - ## in this frame, so nothing references the request once the handler resolves. + ## Pings RET_STALE_WARN every `interval` while the handler runs, then returns + ## its real result. Never cancels the handler: a hard-cancel mid-call could + ## leave the underlying library partially applied. let intervalMs = interval.milliseconds if intervalMs <= 0: - # A non-positive / infinite interval opts out of progress pings entirely. return await retFut var elapsed = 0'i64 while not retFut.finished(): let timer = sleepAsync(interval) - # `race` returns the first to finish WITHOUT cancelling the loser, so the - # handler keeps running when the timer wins. + # `race` doesn't cancel the loser, so the handler keeps running. discard await race(retFut, timer) if retFut.finished(): if not timer.finished(): @@ -78,11 +57,10 @@ proc awaitWithStaleWarnings( proc processRequest[T]( request: ptr FFIThreadRequest, ctx: ptr FFIContext[T] ) {.async.} = - ## Invoked within the FFI thread to process a request coming from the FFI API consumer thread. + ## Processes one request on the FFI thread. let reqId = $request[].reqId - let reqIdCs = reqId.cstring - # keeps reqId alive; implicit string→cstring is a warning. + let reqIdCs = reqId.cstring # keeps reqId alive let retFut = if not ctx[].registeredRequests[].contains(reqIdCs): @@ -90,9 +68,7 @@ proc processRequest[T]( else: ctx[].registeredRequests[][reqIdCs](cast[pointer](request), ctx) - # CatchableError covers CancelledError from the shutdown drain. The warn loop - # and the handler share one try so that a cancel mid-loop still reaches the - # response-and-free below. + # One try over warn-loop + handler so a shutdown-drain cancel still reaches the response-and-free below. let res = try: await awaitWithStaleWarnings(retFut, request, ctx.staleWarnInterval, reqId) @@ -116,13 +92,10 @@ proc ffiNotifyEventEnqueuedHook() {.gcsafe, raises: [].} = error "failed to fire eventQueueSignal after enqueue", err = res.error proc proveAlive(ctx: ptr FFIContext) = - ## Advance the heartbeat the event thread polls to spot a wedged FFI thread. - ## Only that the counter keeps moving matters, never its value — so a plain - ## atomic increment, no read-back. + ## Advance the heartbeat the event thread polls; only movement matters, not value. ctx.ffiHeartbeat.atomicInc() proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = - ## FFI thread body that attends library user API requests ffiCurrentEventRegistry = addr ctx[].eventRegistry ffiCurrentEventQueue = addr ctx[].eventQueue ffiCurrentEventQueueStuck = addr ctx[].eventQueueStuck @@ -134,15 +107,13 @@ proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = defer: onFFIThread = false - # Free handle refs on the FFI thread that allocated them (refc heap is thread-local). + # Free handle refs on the thread that allocated them (refc heap is thread-local). ctx[].handles.releaseAll() - # Teardown has run and no more events will be emitted from this thread; let - # the event thread stop draining and exit. Wake it so it notices without - # waiting a full tick. + # Let the event thread stop draining and exit; wake it so it notices now. ctx.ffiThreadExited.store(true) ctx.eventQueueSignal.fireSync().isOkOr: error "failed to wake event thread on FFI thread exit", err = error - # Unblocks destroyFFIContext's bounded wait so cleanup can proceed. + # Unblocks destroyFFIContext's bounded wait. let fireRes = ctx.threadExitSignal.fireSync() if fireRes.isErr(): error "failed to fire threadExitSignal on FFI thread exit", err = fireRes.error @@ -150,7 +121,7 @@ proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = let ffiRun = proc(ctx: ptr FFIContext[T]) {.async.} = var ffiReqHandler: T # main library object (Waku, LibP2P, SDS, …) - # Tracked so shutdown can drain them; abandoning a mid-await future leaks the request. + # Tracked so shutdown can drain them; abandoning a future leaks its request. var pending: seq[Future[void]] = @[] proc cleanFinishedRequests() = @@ -162,38 +133,32 @@ proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = pending.del(i) proc processQueue() = - ## Process enqueued requests until the queue is empty. A single wake can - ## stand for many submits, so we drain fully rather than once per wake — - ## otherwise queued requests would sit until the next wake. + ## Drain fully: one wake can stand for many submits. while true: var request = ctx.reqQueueBank.mergeQueues() if request.isNil(): break while not request.isNil(): let nextRequest = request[].next # read before processRequest frees it - # Tick per dispatch so a large backlog can't flatline the heartbeat - # and trip the event thread's wedged-FFI-thread detection mid-drain. + # Tick per dispatch so a backlog can't flatline the heartbeat mid-drain. ctx.proveAlive() if ctx.myLib.isNil(): - # This reference must stay inside the closure: it's what keeps - # `ffiReqHandler` in the async env, so `myLib` survives across awaits. + # Must stay inside the closure: keeps `ffiReqHandler` alive across awaits. ctx.myLib = addr ffiReqHandler pending.add processRequest(request, ctx) request = nextRequest while ctx.running.load(): - # Freezes if a sync handler blocks the dispatcher; event thread reads to detect wedged FFI thread. ctx.proveAlive() cleanFinishedRequests() - # Block until a submit signals us, or for at most 100ms if none does. + # Block until a submit signals us, or at most 100ms. discard await ctx.reqSignal.wait().withTimeout(chronos.milliseconds(100)) processQueue() - # Drain once more so requests enqueued just before `running` flipped still - # dispatch and each pending handler's deleteRequest defer runs before exit. + # Drain once more for requests enqueued just before `running` flipped. processQueue() cleanFinishedRequests() if pending.len > 0: @@ -202,10 +167,7 @@ proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = except CatchableError as e: error "draining pending FFI requests on shutdown raised", error = e.msg - # In-flight requests drained; run the library's async shutdown (e.g. - # `switch.stop()`) on this event loop before the thread joins. Only if a - # `{.ffiDtor.}` registered a hook and a request populated `myLib`. Exceptions - # are logged, never propagated: the thread must still fire threadExitSignal. + # Run the library's async {.ffiDtor.} shutdown before join if one exists and a request populated `myLib`; exceptions logged, never propagated. let teardown = ffiTeardownHook[T]() if not teardown.isNil() and not ctx.myLib.isNil(): try: diff --git a/ffi/ffi_thread_request.nim b/ffi/ffi_thread_request.nim index da723bb..22a037a 100644 --- a/ffi/ffi_thread_request.nim +++ b/ffi/ffi_thread_request.nim @@ -1,14 +1,6 @@ -## Carries one CBOR-encoded request blob between the main thread and the FFI -## thread. The main thread allocates the request, the FFI thread frees it -## after invoking the user callback. -## -## All three pieces (envelope, reqId copy, payload buffer) are obtained from -## libc `malloc` and released by libc `free`. Nim's `allocShared` under -## `--mm:orc` is backed by a per-thread `MemRegion` stored in TLS; if the -## producer thread (commonly a transient `std::async` worker on the foreign -## side) has exited by the time the FFI thread runs `deleteRequest`, the -## chunk's `owner` pointer dangles into reclaimed TLS and the deallocator -## segfaults. `malloc`/`free` are process-global and immune to that. +## Request blob passed main→FFI thread. Uses libc malloc/free (not Nim +## allocShared) so a producer thread exiting before the FFI thread frees can't +## dangle into reclaimed per-thread ORC TLS. import system/ansi_c import results @@ -16,40 +8,31 @@ import chronos import ./ffi_types, ./alloc, ./cbor_serial const EmptyErrorMarker = "unknown error" - ## Sent verbatim on RET_ERR when the handler produced no message — keeps - ## the callback's msg ptr non-nil and gives the foreign side a recognizable - ## fallback to log. + ## RET_ERR fallback message; keeps the callback msg ptr non-nil. const MaxScalarArgs* = 8 - ## Inline capacity for the scalar fast path. A `.ffi.` method with more than - ## this many scalar params can't use the fast path (checked at compile time). + ## Inline scalar fast-path capacity; more params can't use it (compile-time checked). type FFIThreadRequest* = object callback*: FFICallBack userData*: pointer - reqId*: cstring ## Per-proc Req type name used to look up the handler. + reqId*: cstring ## Req type name used to look up the handler. data*: ptr UncheckedArray[byte] ## Owned CBOR-encoded request payload. dataLen*: int isScalar*: bool - ## Set by `initScalar`: the payload rode inline in `scalarArgs` (no CBOR, - ## no `data` buffer). Lets `handleRes` tell a scalar 0-length return (a real - ## empty string) from a CBOR "no value". + ## Scalar fast path: args rode inline in `scalarArgs`, so a 0-length return + ## is a real empty string, not a CBOR "no value". scalarArgs*: array[MaxScalarArgs, uint64] - ## Scalar-fast-path args inlined in the envelope so there's no per-call - ## `c_malloc`. A plain array rather than a `union` with `data`: costs a fixed - ## 64 bytes per request but keeps `deleteRequest` unaliased and branch-free. + ## Inlined scalar args (no per-call c_malloc); a plain array keeps + ## `deleteRequest` unaliased. next*: ptr FFIThreadRequest - ## Intrusive ingress-queue link (see `ffi_request_queue.nim`). Touched only - ## under the queue's lock; the request doubles as its own node, so no - ## separate node alloc lands on the per-thread ORC MemRegion. + ## Intrusive queue link; request doubles as its own node so enqueue needs no + ## ORC-heap alloc. responded*: bool - ## De-duplicates the callback across the timeout and completion paths. Both - ## run on the FFI thread, so a plain flag suffices — no cross-thread race. + ## De-dupes the callback across timeout/completion; both on FFI thread, no race. func ffiPackScalar*[T](x: T): uint64 = - ## Bit-cast one scalar into a `uint64` request slot. Signed ints sign-extend - ## to 64 bits; `float32` widens to `float64` (exactly representable, so the - ## value round-trips); `bool` becomes 0/1. Reverse with `ffiUnpackScalar`. + ## Bit-cast one scalar into a uint64 request slot. Reverse with `ffiUnpackScalar`. when T is SomeFloat: cast[uint64](float64(x)) elif T is bool: @@ -60,7 +43,7 @@ func ffiPackScalar*[T](x: T): uint64 = uint64(x) func ffiUnpackScalar*[T](u: uint64, _: typedesc[T]): T = - ## Inverse of `ffiPackScalar`: reinterpret a request slot back into `T`. + ## Inverse of `ffiPackScalar`. when T is SomeFloat: T(cast[float64](u)) elif T is bool: @@ -73,9 +56,7 @@ func ffiUnpackScalar*[T](u: uint64, _: typedesc[T]): T = proc allocBaseRequest( callback: FFICallBack, userData: pointer, reqId: cstring ): ptr FFIThreadRequest = - ## Allocates the request envelope via `c_malloc` and populates the routing - ## fields. Payload setup is delegated to one of the payload helpers below - ## depending on whether the bytes need to be copied or adopted. + ## c_malloc the envelope and set routing fields; payload set by a helper below. var ret = cast[ptr FFIThreadRequest](c_malloc(csize_t(sizeof(FFIThreadRequest)))) ret[].callback = callback ret[].userData = userData @@ -88,10 +69,7 @@ proc allocBaseRequest( return ret proc copySharedPayload(req: ptr FFIThreadRequest, data: ptr byte, dataLen: int) = - ## Allocates a fresh `c_malloc` buffer and copies `dataLen` bytes from - ## `data` into `req`. Empty payloads (non-positive `dataLen` or nil - ## `data`) leave the request's payload fields at their zero-initialised - ## state. + ## c_malloc a fresh buffer and copy `dataLen` bytes in; empty payload is a no-op. if dataLen > 0 and not data.isNil(): req[].data = cast[ptr UncheckedArray[byte]](c_malloc(csize_t(dataLen))) copyMem(req[].data, data, dataLen) @@ -100,9 +78,8 @@ proc copySharedPayload(req: ptr FFIThreadRequest, data: ptr byte, dataLen: int) proc adoptOwnedSharedPayload( req: ptr FFIThreadRequest, data: ptr UncheckedArray[byte], dataLen: int ) = - ## Embeds an already-`c_malloc`'d buffer into `req` without copying. - ## `(nil, 0)` is the empty-payload contract; a zero-length-but-non-nil - ## buffer is treated as empty and disposed here so it doesn't leak. + ## Embed an already-c_malloc'd buffer without copying; frees a zero-length + ## non-nil buffer so it doesn't leak. if dataLen > 0 and not data.isNil(): req[].data = data req[].dataLen = dataLen @@ -117,8 +94,7 @@ proc initFromPtr*( data: ptr byte, dataLen: int, ): ptr type T = - ## Takes a raw ptr+len; the bytes are copied into a fresh shared-memory - ## buffer owned by the returned request. + ## Copies raw ptr+len into a fresh buffer owned by the returned request. var ret = allocBaseRequest(callback, userData, reqId) copySharedPayload(ret, data, dataLen) return ret @@ -130,8 +106,7 @@ proc init*( reqId: cstring, data: openArray[byte], ): ptr type T = - ## Same contract as `initFromPtr` but accepts a Nim openArray, copying its - ## bytes into a fresh shared-memory buffer owned by the returned request. + ## Like `initFromPtr` but from a Nim openArray. let dataPtr = if data.len > 0: cast[ptr byte](unsafeAddr data[0]) @@ -147,14 +122,8 @@ proc initFromOwnedShared*( data: ptr UncheckedArray[byte], dataLen: int, ): ptr type T = - ## Takes ownership of an already-allocated buffer (`data`) and embeds it - ## in the request without copying. Pair with `cborEncodeShared` so the - ## request payload travels from encoder to FFI thread with a single - ## allocation instead of seq → c_malloc + copyMem. - ## - ## Ownership: `data` must have been allocated via `c_malloc`. After this - ## call, `deleteRequest` will `c_free` it. Pass `(nil, 0)` for an empty - ## payload. + ## Adopts an already-c_malloc'd buffer (no copy); `deleteRequest` c_frees it. + ## Pass `(nil, 0)` for an empty payload. var ret = allocBaseRequest(callback, userData, reqId) adoptOwnedSharedPayload(ret, data, dataLen) return ret @@ -166,9 +135,7 @@ proc initScalar*( reqId: cstring, args: varargs[uint64], ): ptr type T = - ## Builds a scalar-fast-path request: the packed scalar args ride inline in - ## `scalarArgs` with no payload `c_malloc`. `args` come from `ffiPackScalar`. - ## Only the routing `reqId` cstring is heap-allocated, same as the CBOR path. + ## Scalar-fast-path request: packed args ride inline, no payload c_malloc. doAssert args.len <= MaxScalarArgs, "initScalar: " & $args.len & " scalar args exceed MaxScalarArgs (" & $MaxScalarArgs & ")" @@ -179,11 +146,8 @@ proc initScalar*( ret func ffiScalarRetBytes*[T](x: T): seq[byte] = - ## Serializes a scalar handler result into the raw response payload — no CBOR - ## envelope. A `string`/`cstring` rides as its own UTF-8 bytes (like the error - ## path); every other scalar rides as the 8-byte native-endian image of - ## `ffiPackScalar(x)`. An empty string yields a 0-length payload (see - ## `handleRes`, which delivers it as `""` rather than the CBOR-null sentinel). + ## Scalar handler result as raw bytes, no CBOR: string/cstring ride as UTF-8, + ## other scalars as the 8-byte native image of `ffiPackScalar(x)`. when T is string: var b = newSeq[byte](x.len) if x.len > 0: @@ -209,12 +173,8 @@ proc deleteRequest*(request: ptr FFIThreadRequest) = c_free(request) proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) = - ## Delivers the response to the foreign callback, at most once per request: - ## the timeout path and the handler-completion path both call it, but the - ## foreign side must be answered exactly once. Both run on the FFI thread, so - ## the plain `responded` flag needs no synchronization. Success payload is the - ## encoded response bytes; error payload is the raw UTF-8 error string. Does - ## NOT free the request; that stays with `handleRes`. + ## Answers the foreign callback at most once (timeout and completion both call + ## it). Does NOT free the request; `handleRes` does. if request[].responded: return request[].responded = true @@ -236,10 +196,7 @@ proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest request[].userData, ) elif request[].isScalar: - # `isScalar` marks a scalar-fast-path request (args rode inline in - # `scalarArgs`, no CBOR): its result bytes come from `ffiScalarRetBytes`, - # not a CBOR encoder. So a 0-byte return is a real empty string, not - # "no value" — hand back a genuine empty buffer, not the CBOR-null sentinel. + # Scalar 0-byte return is a real empty string, not CBOR "no value". var empty: byte request[].callback( RET_OK, cast[ptr cchar](addr empty), 0.csize_t, request[].userData @@ -252,10 +209,8 @@ proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest ) proc fireStaleWarn*(request: ptr FFIThreadRequest, elapsedMs: int64) = - ## Tells the caller its request is still in flight after `elapsedMs` (sent as - ## decimal UTF-8). Unlike `fireCallback` it deliberately leaves `responded` - ## unset and may fire many times — the terminal RET_OK/RET_ERR is still owed. - ## Runs on the FFI thread, so reading `responded` needs no synchronization. + ## In-flight ping; leaves `responded` unset and may fire many times — the + ## terminal RET_OK/RET_ERR is still owed. if request[].responded: return foreignThreadGc: @@ -268,9 +223,7 @@ proc fireStaleWarn*(request: ptr FFIThreadRequest, elapsedMs: int64) = ) proc handleRes*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) = - ## Terminal step of every request: delivers the response and frees the request - ## exactly once. The `responded` guard in `fireStaleWarn` keeps this answer - ## last, after any progress pings. + ## Terminal step: delivers the response and frees the request exactly once. defer: deleteRequest(request) fireCallback(res, request) diff --git a/ffi/ffi_types.nim b/ffi/ffi_types.nim index b615fa7..1b1e8a7 100644 --- a/ffi/ffi_types.nim +++ b/ffi/ffi_types.nim @@ -1,37 +1,23 @@ import std/tables import chronos -################################################################################ -### Exported types - type FFICallBack* = proc( callerRet: cint, msg: ptr cchar, len: csize_t, userData: pointer ) {.cdecl, gcsafe, raises: [].} - ## Result-delivery callback. `callerRet` is one of the `RET_*` codes below: - ## `RET_OK`/`RET_ERR` fire exactly once and end the request, `RET_STALE_WARN` - ## may fire repeatedly before them and should be ignored unless progress - ## matters. + ## Result-delivery callback. `RET_OK`/`RET_ERR` fire once and end the request; + ## `RET_STALE_WARN` may fire repeatedly before them. const RET_OK*: cint = 0 const RET_ERR*: cint = 1 const RET_MISSING_CALLBACK*: cint = 2 const RET_STALE_WARN*: cint = 3 - ## Non-terminal: the request is still in flight. Fires every - ## `StaleWarnInterval` (default 5s) while the handler runs, `msg` carrying the - ## elapsed milliseconds as decimal ASCII, and is always followed by a terminal - ## code — nim-ffi never times a handler out, so the caller decides whether to - ## keep waiting. - -### End of exported types -################################################################################ - -################################################################################ -### FFI utils + ## Non-terminal: request still in flight, fires every `StaleWarnInterval` with + ## `msg` = elapsed ms as decimal ASCII, always followed by a terminal code. type FFIRequestProc* = proc( request: pointer, reqHandler: pointer ): Future[Result[seq[byte], string]] {.async.} - ## The OK payload is a CBOR-encoded response body. Errors are plain UTF-8. + ## OK payload is a CBOR-encoded response body; errors are plain UTF-8. template foreignThreadGc*(body: untyped) = when declared(setupForeignThreadGc): @@ -42,10 +28,5 @@ template foreignThreadGc*(body: untyped) = when declared(tearDownForeignThreadGc): tearDownForeignThreadGc() -## Registered requests table populated at compile time and never updated at run time. -## The key represents the request type name as cstring, e.g., "CreateNodeRequest". -## The value is a proc that handles the request asynchronously. +## Compile-time-populated table: request type name (cstring) -> async handler. var registeredRequests*: Table[cstring, FFIRequestProc] - -### End of FFI utils -################################################################################ diff --git a/ffi/internal/c_macro_helpers.nim b/ffi/internal/c_macro_helpers.nim index 2b484c9..18b3c7c 100644 --- a/ffi/internal/c_macro_helpers.nim +++ b/ffi/internal/c_macro_helpers.nim @@ -1,11 +1,6 @@ -## Compile-time helpers used by `ffi_macro.nim` for the `c` (`abi = c` C-struct) ABI. -## For each `{.ffi: "abi = c".}` object T, emits a `T_CWire` companion plus -## `cwirePack` / `cwireUnpack` / `cwireFree`. Field mapping: `string`→`cstring`, -## `seq[T]`→`_items`+`_len`, `Option[T]`/`Maybe[T]`→`ptr T_w` -## (nil=none), nested {.ffi.}→`T_CWire`, `array[N, T]`→inline `array[N, T_w]`, -## `tuple[a: T, ...]`→`tuple[a: T_w, ...]`, POD unchanged. seq/Option/array/tuple -## nest to any depth, but a `seq` may not nest inside another container (it has -## no single-field wire form — only the top-level `_items`/`_len` split). +## Compile-time helpers for the `abi = c` C-struct ABI: for each `{.ffi: "abi = c".}` +## object T, emits a `T_CWire` companion plus `cwirePack`/`cwireUnpack`/`cwireFree`. +## A `seq` may only be a top-level field (no single-field wire form to nest). import std/macros import ../codegen/meta @@ -17,8 +12,7 @@ const var emittedCWireTypes {.compileTime.}: seq[string] proc isCWireEmitted(typeName: string): bool {.compileTime.} = - ## Indexed scan: works around a Nim 2.2 compile-time VM quirk where `for x in - ## seq` over a freshly-mutated `{.compileTime.}` seq goes stale. + # Indexed scan: `for x in seq` over a freshly-mutated compileTime seq goes stale on the Nim 2.2 VM. for i in 0 ..< emittedCWireTypes.len: if emittedCWireTypes[i] == typeName: return true @@ -29,15 +23,12 @@ proc markCWireEmitted(typeName: string) {.compileTime.} = emittedCWireTypes.add(typeName) proc cwireTypeName(userTypeName: string): string = - ## Companion-type naming convention; stable so generated tests reach in by name. userTypeName & "_CWire" proc seqItemsField(obj, field: NimNode): NimNode = - ## `obj._items` — the buffer half of a seq's two-field wire split. newDotExpr(obj, ident($field & cwireItemsSuffix)) proc seqLenField(obj, field: NimNode): NimNode = - ## `obj._len` — the count half of a seq's two-field wire split. newDotExpr(obj, ident($field & cwireLenSuffix)) proc isStringType(t: NimNode): bool = @@ -59,8 +50,7 @@ proc isTupleType(t: NimNode): bool = t.kind == nnkTupleTy proc tupleComponents(t: NimNode): seq[tuple[name: string, typ: NimNode]] = - ## Flatten a named-tuple type into `(name, type)` pairs, expanding grouped - ## declarations like `tuple[a, b: int]` into one entry per name. + ## Flatten a named tuple into `(name, type)` pairs, one per name. var comps: seq[tuple[name: string, typ: NimNode]] = @[] for defs in t: if defs.kind != nnkIdentDefs: @@ -80,9 +70,7 @@ proc isNestedFFIType(t: NimNode): bool = t.kind == nnkIdent and isKnownFFIType($t) proc cwireNeedsFree(t: NimNode): bool = - ## Whether the wire form of `t` owns shared-memory allocations that - ## `cwireFree` must release. POD scalars (and aggregates entirely of POD) - ## own nothing, so their free is elided. + ## Whether the wire form of `t` owns allocations `cwireFree` must release. if isStringType(t) or isNestedFFIType(t) or isOptionType(t) or isSeqType(t): return true if isArrayType(t): @@ -95,18 +83,13 @@ proc cwireNeedsFree(t: NimNode): bool = false proc rejectNestedSeq(t: NimNode) = - ## `seq` has no single-field wire form (only the top-level `_items`/`_len` - ## split), so it can't sit inside another container. One message, one place. error( "cwire: `seq` has no single-field wire form, so it can't nest inside " & "another container (use it only as a top-level field): " & t.repr ) proc wireValueType(t: NimNode): NimNode = - ## Single-field wire form of value type `t`: `string`→`cstring`, nested - ## {.ffi.}→`T_CWire`, `Option[T]`→`ptr `, `array[N, T]`→ - ## `array[N, ]`, `tuple[a: T, ...]`→`tuple[a: , ...]`, - ## POD unchanged. `seq` has no single-field form, so it errors here. + ## Single-field wire form of value type `t`; `seq` has none, so it errors here. if isStringType(t): return ident("cstring") if isNestedFFIType(t): @@ -126,8 +109,7 @@ proc wireValueType(t: NimNode): NimNode = t proc wireFieldsFor(fieldName: string, fieldType: NimNode): seq[NimNode] = - ## IdentDefs for one field. `seq[T]` splits into `_items: ptr - ## UncheckedArray[]` + `_len: int`; else a single IdentDef. + ## IdentDefs for one field; `seq[T]` splits into `_items` + `_len`. if isSeqType(fieldType): let elemWire = wireValueType(fieldType[1]) let itemsField = newIdentDefs( @@ -143,8 +125,7 @@ proc wireFieldsFor(fieldName: string, fieldType: NimNode): seq[NimNode] = proc buildCWireTypeDef( userTypeName: string, fieldNames: seq[string], fieldTypes: seq[NimNode] ): NimNode = - ## Build the bare `nnkTypeDef` (no enclosing TypeSection) for the wire - ## companion of `userTypeName`. + ## Build the bare `nnkTypeDef` for the wire companion of `userTypeName`. let wireName = ident(cwireTypeName(userTypeName)) var fields: seq[NimNode] = @[] for i in 0 ..< fieldNames.len: @@ -171,8 +152,7 @@ proc emitTupleUnpack(dstAccess, srcAccess, tupType: NimNode): NimNode proc emitTupleFree(dstAccess, tupType: NimNode): NimNode proc emitElemPack(dstElem, srcElem, elemType: NimNode): NimNode = - ## Pack one value: cstring for `string`, recursive `cwirePack` for nested - ## ffi types, recursive Option/array/tuple handling, direct copy for POD. + ## Pack one value; recurses through nested ffi/Option/array/tuple, POD copied. if isStringType(elemType): return newAssignment(dstElem, newCall(ident("cwireAllocStr"), srcElem)) if isNestedFFIType(elemType): @@ -188,7 +168,7 @@ proc emitElemPack(dstElem, srcElem, elemType: NimNode): NimNode = newAssignment(dstElem, srcElem) proc emitElemUnpack(dstElem, srcElem, elemType: NimNode): NimNode = - ## Inverse of `emitElemPack`: copy one value back into Nim-managed memory. + ## Inverse of `emitElemPack`: copy one value back into Nim memory. if isStringType(elemType): return newAssignment(dstElem, newCall(ident("$"), srcElem)) if isNestedFFIType(elemType): @@ -204,7 +184,7 @@ proc emitElemUnpack(dstElem, srcElem, elemType: NimNode): NimNode = newAssignment(dstElem, srcElem) proc emitElemFree(elemAccess, elemType: NimNode): NimNode = - ## Free one value, or `nnkEmpty` for POD (nothing to free). + ## Free one value, or `nnkEmpty` for POD. if isStringType(elemType): return newCall(ident("cwireFreeStr"), elemAccess) if isNestedFFIType(elemType): @@ -220,15 +200,13 @@ proc emitElemFree(elemAccess, elemType: NimNode): NimNode = newEmptyNode() proc maybeStmt(n: NimNode): NimNode = - ## `n` as a one-statement list, or an empty list when `n` is `nnkEmpty` - ## (nothing to do) — keeps the surrounding `quote` block well-formed. + ## `n` as a one-statement list, empty list when `nnkEmpty`. if n.kind == nnkEmpty: return newStmtList() newStmtList(n) proc indexLoop(access, idx, body: NimNode): NimNode = - ## `for in low(access) .. high(access): body` — `low`/`high` so any - ## array index range (not just 0-based) is covered. + ## `for in low(access) .. high(access): body` (covers non-0-based ranges). nnkForStmt.newTree( idx, nnkInfix.newTree( @@ -238,8 +216,7 @@ proc indexLoop(access, idx, body: NimNode): NimNode = ) proc emitArrayPack(dstAccess, srcAccess, arrType: NimNode): NimNode = - ## Pack a fixed `array[N, T]` element-by-element into the inline wire array; - ## the array itself needs no allocation, only its GC'd element contents do. + ## Pack a fixed `array[N, T]` element-by-element into the inline wire array. let idx = genSym(nskForVar, "i") let body = emitElemPack( nnkBracketExpr.newTree(dstAccess, idx), @@ -249,7 +226,7 @@ proc emitArrayPack(dstAccess, srcAccess, arrType: NimNode): NimNode = indexLoop(srcAccess, idx, body) proc emitArrayUnpack(dstAccess, srcAccess, arrType: NimNode): NimNode = - ## Inverse of `emitArrayPack`: copy each wire element back into the Nim array. + ## Inverse of `emitArrayPack`. let idx = genSym(nskForVar, "i") let body = emitElemUnpack( nnkBracketExpr.newTree(dstAccess, idx), @@ -259,7 +236,7 @@ proc emitArrayUnpack(dstAccess, srcAccess, arrType: NimNode): NimNode = indexLoop(srcAccess, idx, body) proc emitArrayFree(dstAccess, arrType: NimNode): NimNode = - ## Free each array element; `nnkEmpty` when the element type owns nothing. + ## Free each array element; `nnkEmpty` when the element owns nothing. if not cwireNeedsFree(arrType[2]): return newEmptyNode() let idx = genSym(nskForVar, "i") @@ -275,7 +252,7 @@ proc emitTuplePack(dstAccess, srcAccess, tupType: NimNode): NimNode = body proc emitTupleUnpack(dstAccess, srcAccess, tupType: NimNode): NimNode = - ## Inverse of `emitTuplePack`: copy each wire component back out. + ## Inverse of `emitTuplePack`. let body = newStmtList() for c in tupleComponents(tupType): let nm = ident(c.name) @@ -285,7 +262,7 @@ proc emitTupleUnpack(dstAccess, srcAccess, tupType: NimNode): NimNode = body proc emitTupleFree(dstAccess, tupType: NimNode): NimNode = - ## Free each tuple component that owns allocations; `nnkEmpty` when none do. + ## Free each tuple component that owns allocations. if not cwireNeedsFree(tupType): return newEmptyNode() let body = newStmtList() @@ -294,8 +271,7 @@ proc emitTupleFree(dstAccess, tupType: NimNode): NimNode = body proc emitSeqPack(dstObj, srcAccess, fieldNameIdent, userType: NimNode): NimNode = - ## Pack a seq field into a freshly `allocShared`'d `UncheckedArray`; an empty - ## seq encodes as nil items + 0 len. + ## Pack a seq into an `allocShared` `UncheckedArray`; empty = nil items + 0 len. let elemType = userType[1] let wireElem = wireValueType(elemType) let items = seqItemsField(dstObj, fieldNameIdent) @@ -323,9 +299,8 @@ proc emitSeqPack(dstObj, srcAccess, fieldNameIdent, userType: NimNode): NimNode `count` = `srcAccess`.len() proc emitOptionPack(dstAccess, srcAccess, userType: NimNode): NimNode = - ## Pack an Option into a `ptr`: some → `allocShared` a box and pack into it, - ## none → nil. The payload is read into a local once, so a composite inner - ## type (e.g. `array`/`tuple`) isn't re-`get()`-copied per element. + ## Pack an Option into a `ptr`: some → `allocShared` box, none → nil. Payload + ## read into a local once so a composite inner type isn't re-`get()` per element. let innerType = userType[1] let wireInner = wireValueType(innerType) let bufType = nnkPtrTy.newTree(wireInner) @@ -340,8 +315,7 @@ proc emitOptionPack(dstAccess, srcAccess, userType: NimNode): NimNode = `dstAccess` = nil proc emitPackStmt(dstObj, srcObj, fieldNameIdent, userType: NimNode): seq[NimNode] = - ## Populate `dstObj.` from `srcObj.`, allocating shared-memory - ## cstrings/arrays as the field's natural type requires. + ## Populate `dstObj.` from `srcObj.`. let srcAccess = newDotExpr(srcObj, fieldNameIdent) let dstAccess = newDotExpr(dstObj, fieldNameIdent) if isSeqType(userType): @@ -349,7 +323,7 @@ proc emitPackStmt(dstObj, srcObj, fieldNameIdent, userType: NimNode): seq[NimNod @[emitElemPack(dstAccess, srcAccess, userType)] proc emitSeqUnpack(dstAccess, srcObj, fieldNameIdent, userType: NimNode): NimNode = - ## Rebuild a Nim seq from the `_items`/`_len` wire pair. + ## Rebuild a Nim seq from the `_items`/`_len` wire pair. let elemType = userType[1] let items = seqItemsField(srcObj, fieldNameIdent) let count = seqLenField(srcObj, fieldNameIdent) @@ -379,8 +353,7 @@ proc emitOptionUnpack(dstAccess, srcAccess, userType: NimNode): NimNode = proc emitUnpackStmt( resultObj, srcObj, fieldNameIdent, userType: NimNode ): seq[NimNode] = - ## Fill `resultObj.` from `srcObj.`, copying back into - ## Nim-managed memory. + ## Fill `resultObj.` from `srcObj.`. let srcAccess = newDotExpr(srcObj, fieldNameIdent) let dstAccess = newDotExpr(resultObj, fieldNameIdent) if isSeqType(userType): @@ -388,8 +361,7 @@ proc emitUnpackStmt( @[emitElemUnpack(dstAccess, srcAccess, userType)] proc emitSeqFree(dstObj, fieldNameIdent, userType: NimNode): NimNode = - ## Free a seq field: free each element (skipped entirely for POD), then the - ## shared buffer. + ## Free a seq field: each element (skipped for POD), then the shared buffer. let elemType = userType[1] let items = seqItemsField(dstObj, fieldNameIdent) let count = seqLenField(dstObj, fieldNameIdent) @@ -412,7 +384,7 @@ proc emitSeqFree(dstObj, fieldNameIdent, userType: NimNode): NimNode = `count` = 0 proc emitOptionFree(dstAccess, userType: NimNode): NimNode = - ## Free an Option field: free the pointee (skipped for POD), then the box. + ## Free an Option field: the pointee (skipped for POD), then the box. let innerType = userType[1] let freeInner = maybeStmt(emitElemFree(nnkBracketExpr.newTree(dstAccess), innerType)) quote: @@ -434,8 +406,7 @@ proc emitFreeStmt(dstObj, fieldNameIdent, userType: NimNode): seq[NimNode] = proc buildCWireProcs( userTypeName: string, fieldNames: seq[string], fieldTypes: seq[NimNode] ): seq[NimNode] = - ## Generate cwirePack / cwireUnpack / cwireFree procs for `userTypeName`. All - ## three are public (`*`) so the macro-expanded code can call them. + ## Generate public cwirePack / cwireUnpack / cwireFree procs for `userTypeName`. let userName = ident(userTypeName) let wireName = ident(cwireTypeName(userTypeName)) @@ -496,8 +467,7 @@ proc buildCWireProcs( proc fieldInfoForType( typeName: string ): tuple[names: seq[string], types: seq[NimNode]] {.compileTime.} = - ## Look up an ffi type's fields from the compile-time registry and parse each - ## field's recorded type back into a NimNode AST. + ## Look up an ffi type's fields from the registry, parsing each recorded type. for typeMeta in ffiTypeRegistry: if typeMeta.name != typeName: continue @@ -512,8 +482,8 @@ proc fieldInfoForType( proc collectNestedFFITypes( fieldTypes: seq[NimNode], deps: var seq[string] ) {.compileTime.} = - ## Append (deduped) the names of nested ffi types referenced anywhere in - ## `fieldTypes`, recursing through `seq`/`Option`/`array`/`tuple` to any depth. + ## Append (deduped) nested ffi type names in `fieldTypes`, recursing through + ## `seq`/`Option`/`array`/`tuple`. for t in fieldTypes: if isNestedFFIType(t): let n = $t @@ -528,9 +498,8 @@ proc collectNestedFFITypes( collectNestedFFITypes(@[c.typ], deps) proc ensureCWireFor(typeName: string, sink: NimNode) {.compileTime.} = - ## Idempotent: if `typeName`'s cwire companion has not yet been emitted, - ## append its TypeSection and conversion procs to `sink` and mark it emitted. - ## Nested ffi deps are ensured first so the resulting AST is self-contained. + ## Idempotent: append `typeName`'s cwire companion + procs to `sink` if not yet + ## emitted. Nested ffi deps are ensured first so the AST is self-contained. if isCWireEmitted(typeName): return let info = fieldInfoForType(typeName) @@ -546,26 +515,16 @@ proc ensureCWireFor(typeName: string, sink: NimNode) {.compileTime.} = sink.add(p) proc flushCWireCompanions*(): NimNode {.compileTime.} = - ## Emit the `_CWire` companion + conversion procs for every registered - ## `abi = c` type. Called by `genBindings()` (a type-pragma macro can't). + ## Emit the `_CWire` companion + procs for every registered `abi = c` type. let sink = newStmtList() for typeMeta in ffiTypeRegistry: if typeMeta.abiFormat == ABIFormat.C: ensureCWireFor(typeMeta.name, sink) sink -## abi = c proc dispatch. The foreign surface is CBOR-free — the `_CWire` -## structs are the C ABI — but transport reuses the proven CBOR request path -## internally: the generated exported wrapper `cwireUnpack`s the request into a -## Nim object, `cborEncodeShared`s it onto the FFI thread, and a Nim reply -## trampoline `cborDecode`s the reply and `cwirePack`s it back into a `_CWire` -## struct delivered to the caller's typed callback. So the C consumer never -## links CBOR, yet the whole thread/dispatch machinery is unchanged. -## -## All of this is emitted at `genBindings()` time (after `flushCWireCompanions`) -## so the request-envelope companions and their `cwireUnpack` overloads are in -## scope: the exported wrappers reference them, and a proc must follow the -## procs it calls. +## abi = c proc dispatch. The foreign surface is CBOR-free (the `_CWire` structs are +## the C ABI) but transport reuses the CBOR request path internally. Emitted at +## `genBindings()` time (after `flushCWireCompanions`) so the companions are in scope. type CAbiKind = enum @@ -596,10 +555,8 @@ proc registerCAbiMethod*( paramTypes: seq[NimNode], respType: NimNode, ) {.compileTime.} = - ## Record an `abi = c` method so `flushCAbiDispatch` can emit its wrapper. - ## Nodes are frozen with `copyNimTree` — the originals are shared with the Req - ## `type` section, which the compiler later binds to `nnkSym`, and a bound - ## type symbol reused in the generated body triggers a compiler ICE. + ## Record an `abi = c` method for `flushCAbiDispatch`. Nodes are `copyNimTree` + ## frozen: reusing the Req section's originals (bound to `nnkSym`) would ICE. cAbiSpecs.add( CAbiSpec( kind: cakMethod, @@ -618,8 +575,8 @@ proc registerCAbiCtor*( paramNames: seq[string], paramTypes: seq[NimNode], ) {.compileTime.} = - ## Record an `abi = c` constructor so `flushCAbiDispatch` can emit its wrapper. - ## See `registerCAbiMethod` for why the nodes are frozen with `copyNimTree`. + ## Record an `abi = c` ctor for `flushCAbiDispatch`; see `registerCAbiMethod` + ## for why nodes are `copyNimTree` frozen. cAbiSpecs.add( CAbiSpec( kind: cakCtor, @@ -640,8 +597,7 @@ proc cdeclReplyPragma(): NimNode = ) proc cAbiCbType(replyType: NimNode): NimNode = - ## `proc(err: cint, reply: , errMsg: cstring, ud: pointer) - ## {.cdecl, gcsafe, raises: [].}` — the caller's typed reply callback. + ## The caller's typed reply callback proc type. let fp = nnkFormalParams.newTree( newEmptyNode(), newIdentDefs(ident("err"), ident("cint")), @@ -652,8 +608,7 @@ proc cAbiCbType(replyType: NimNode): NimNode = nnkProcTy.newTree(fp, cdeclReplyPragma()) proc boxTypeDef(boxName, cbType: NimNode): NimNode = - ## `type = object` holding the caller's callback + user data, heap - ## boxed across the thread hand-off. + ## Box object holding the caller's callback + user data across the thread hop. let recList = nnkRecList.newTree( newIdentDefs(ident("fn"), cbType), newIdentDefs(ident("ud"), ident("pointer")) ) @@ -661,8 +616,7 @@ proc boxTypeDef(boxName, cbType: NimNode): NimNode = nnkTypeSection.newTree(nnkTypeDef.newTree(boxName, newEmptyNode(), objTy)) proc replyTrampProc(trampName, body: NimNode): NimNode = - ## A `FFICallBack`-shaped Nim proc: it runs on the FFI thread inside - ## `handleRes`' `foreignThreadGc`, converts the reply, and frees the box. + ## `FFICallBack`-shaped proc: runs on the FFI thread, converts the reply, frees the box. newProc( name = trampName, params = @[ @@ -677,19 +631,14 @@ proc replyTrampProc(trampName, body: NimNode): NimNode = ) proc objectTrampBody(boxName, respType, respWire: NimNode): NimNode = - ## Reply trampoline for an object return: recover the box, deliver a transport - ## error as a copied NUL-terminated string, else CBOR-decode the reply, - ## `cwirePack` it into the `_CWire` struct, hand a pointer to the caller, and - ## release the wire. `err_msg` is always a non-nil string; the `reply` struct - ## pointer is nil only on error, gated by a non-`RET_OK` `err_code`. + ## Reply trampoline for an object return: decode, `cwirePack` into `_CWire`, + ## hand a pointer to the caller, release. `reply` is nil only on error. quote: let box = cast[ptr `boxName`](ud) if box.isNil(): return if ret == RET_STALE_WARN: - # Non-terminal progress signal: keep the box for the eventual terminal - # reply and don't decode (there's no reply payload yet). Typed wrappers - # don't surface it; the raw FFICallBack boundary does. + # Non-terminal progress signal: keep the box, don't decode. return defer: freeBox(box) @@ -716,18 +665,14 @@ proc objectTrampBody(boxName, respType, respWire: NimNode): NimNode = proc stringTrampBody(boxName: NimNode): NimNode = ## Reply trampoline for a `string` return (and the ctor's address string): - ## CBOR-decode the reply into a Nim string and hand its (NUL-terminated) - ## `cstring` to the caller for the duration of the call. Reply and error - ## strings are always non-nil empty strings on the paths they don't apply to, - ## so a consumer can `strlen`/print either unconditionally without a nil deref. + ## decode and hand the caller a NUL-terminated `cstring`. Reply/error strings + ## are always non-nil empty on the paths they don't apply to (no nil deref). quote: let box = cast[ptr `boxName`](ud) if box.isNil(): return if ret == RET_STALE_WARN: - # Non-terminal progress signal: keep the box for the eventual terminal - # reply and don't decode. Typed wrappers don't surface it; the raw - # FFICallBack boundary does. + # Non-terminal progress signal: keep the box, don't decode. return defer: freeBox(box) @@ -752,17 +697,11 @@ proc stringTrampBody(boxName: NimNode): NimNode = proc exportedMethodProc( spec: CAbiSpec, boxName, envWire, trampName, poolIdent, cbType: NimNode ): NimNode = - # `cwireUnpack` and `cborEncodeShared` run on the *calling* thread and allocate - # GC memory, so the caller must be a GC-registered thread — which the dylib's - # load thread already is. Deliberately NOT wrapped in `foreignThreadGc`: its - # `tearDownForeignThreadGc` would destroy that thread's live ORC heap (a - # use-after-free / nil-read crash), and unlike the CBOR path — which only - # memcpy's bytes here — this path genuinely needs the heap intact. + # No `foreignThreadGc`: `cwireUnpack`/`cborEncodeShared` alloc on the calling thread (already GC-registered); wrapping would free its live ORC heap. let envName = spec.envelope let libFFICtx = nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), spec.libType)) - # A string reply is an empty (non-nil) cstring on the error path, matching the - # trampoline; an object reply is a nil struct pointer gated by `err_code`. + # String reply: empty non-nil cstring on error; object reply: nil ptr gated by err_code. let emptyReply = if isStringType(spec.respType): newDotExpr(newLit(""), ident("cstring")) @@ -816,10 +755,7 @@ proc exportedCtorProc( spec: CAbiSpec, boxName, envWire, trampName, poolIdent, cbType: NimNode ): NimNode = let envName = spec.envelope - # See exportedMethodProc: the request conversion allocates GC on the calling - # thread, so no `foreignThreadGc` (its teardown would free that thread's heap). - # `when declared(initializeLibrary): initializeLibrary()` — built as raw AST; - # a `when` with an undeclared symbol inside `quote` trips a compiler ICE. + # No `foreignThreadGc` (see exportedMethodProc). initGuard is built as raw AST because a `when declared` over an undeclared symbol inside `quote` ICEs. let initGuard = nnkWhenStmt.newTree( nnkElifBranch.newTree( newCall(ident("declared"), ident("initializeLibrary")), @@ -880,9 +816,8 @@ proc exportedCtorProc( proc ensureCWireForFields( sink: NimNode, typeName: string, names: seq[string], types: seq[NimNode] ) {.compileTime.} = - ## Emit the `_CWire` companion + conversion procs for a synthetic per-proc Req - ## envelope (not a user `{.ffi.}` type, so it isn't in `ffiTypeRegistry`). - ## Nested user-type deps are already emitted by `flushCWireCompanions`. + ## Emit the `_CWire` companion + procs for a synthetic per-proc Req envelope + ## (not a user `{.ffi.}` type, so not in `ffiTypeRegistry`). if isCWireEmitted(typeName): return var deps: seq[string] = @[] @@ -898,8 +833,7 @@ proc ensureCWireForFields( proc flushCAbiDispatch*(): NimNode {.compileTime.} = ## Emit the exported wrappers + reply trampolines for every registered - ## `abi = c` proc. Runs after `flushCWireCompanions` so nested companions and - ## their `cwireUnpack`/`cwirePack` overloads are already defined. + ## `abi = c` proc. Runs after `flushCWireCompanions`. let sink = newStmtList() for spec in cAbiSpecs: let envName = spec.envelope diff --git a/ffi/internal/c_wire.nim b/ffi/internal/c_wire.nim index 9628508..3d60fed 100644 --- a/ffi/internal/c_wire.nim +++ b/ffi/internal/c_wire.nim @@ -1,17 +1,13 @@ -## Runtime helpers for the macro-generated `*_CWire` companion types: only the -## `cstring` fields need allocation here (seq/Option are alloc'd inline by the -## macro), packed on pack and released on free. +## Runtime cstring alloc/free for the macro-generated `*_CWire` types. import ../alloc proc cwireAllocStr*(s: string): cstring {.inline.} = - ## NUL-terminated `malloc` copy of `s` (see `ffi/alloc.nim`); pair with - ## `cwireFreeStr`. Empty input still yields a valid buffer, never NULL. + ## NUL-terminated `malloc` copy of `s`; pair with `cwireFreeStr`. alloc.alloc(s) proc cwireFreeStr*(s: var cstring) {.inline.} = - ## Idempotent free for a `cwireAllocStr` cstring; `nil` is a no-op. Taken by - ## `var` and reset to `nil` after release so a repeated call can't double-free. + ## Idempotent free; reset to `nil` so a repeated call can't double-free. if s.isNil(): return alloc.dealloc(s) diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index a7c466e..e1e783c 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -4,11 +4,8 @@ import strutils import ../codegen/meta func nimMainPrefixOnCmdLine(cmdLine: string): tuple[found: bool, value: string] = - ## Scan the compiler command line for `--nimMainPrefix:X` (last one wins) and - ## return its value. Switch names in Nim are style-insensitive, so the name - ## is matched lowercased and with underscores stripped; the separator may be - ## `:` or `=`. Returns `(false, "")` when the flag is absent — note config.nims - ## switches may not surface here, so absence is not proof it was never set. + ## Last `--nimMainPrefix:X` on the command line (style-insensitive match, `:` + ## or `=`); absence isn't proof it was never set (config.nims may not surface). var found = false var value = "" for tok in cmdLine.splitWhitespace(): @@ -22,12 +19,9 @@ func nimMainPrefixOnCmdLine(cmdLine: string): tuple[found: bool, value: string] (found, value) proc validateNimMainPrefix(libraryName: string) {.compileTime.} = - ## The Nim runtime init symbol is importc'd as `lib{libraryName}NimMain`, so - ## the build must pass `--nimMainPrefix:lib{libraryName}`; a mismatch otherwise - ## surfaces only at link time as an obscure undefined-symbol error. Absence - ## can't be an error — config.nims may set the prefix without it showing on - ## `commandLine` — so it only warrants a hint, and only for the `--app:lib` - ## build where the prefix actually matters. + ## The init symbol is importc'd as `lib{libraryName}NimMain`, so the build must + ## pass `--nimMainPrefix:lib{libraryName}`; a mismatch errors, absence only + ## hints (config.nims may set it) and only under `--app:lib`. let expectedPrefix = "lib" & libraryName let (prefixFound, prefixValue) = nimMainPrefixOnCmdLine(querySetting(SingleValueSetting.commandLine)) @@ -49,14 +43,13 @@ proc validateNimMainPrefix(libraryName: string) {.compileTime.} = ) macro declareLibraryBase*(libraryName: static[string]): untyped = - # Record the library name for binding generation currentLibName = libraryName validateNimMainPrefix(libraryName) var res = newStmtList() - ## Generate {.pragma: exported, exportc, cdecl, raises: [].} + # {.pragma: exported, exportc, cdecl, raises: [].} res.add nnkPragma.newTree( nnkExprColonExpr.newTree(ident"pragma", ident"exported"), ident"exportc", @@ -64,7 +57,7 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = nnkExprColonExpr.newTree(ident"raises", nnkBracket.newTree()), ) - ## Generate {.pragma: callback, cdecl, raises: [], gcsafe.} + # {.pragma: callback, cdecl, raises: [], gcsafe.} res.add nnkPragma.newTree( nnkExprColonExpr.newTree(ident"pragma", ident"callback"), ident"cdecl", @@ -72,14 +65,12 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = ident"gcsafe", ) - ## Generate {.passc: "-fPIC".} + # {.passc: "-fPIC".} res.add nnkPragma.newTree(nnkExprColonExpr.newTree(ident"passc", newLit("-fPIC"))) - # soname / install_name only apply to a shared library and break an executable - # link (fatally on macOS), so emit them only under `--app:lib`. + # soname / install_name only apply to a shared library and break an executable link (fatally on macOS), so emit them only under `--app:lib`. if compileOption("app", "lib"): when defined(linux): - ## Generates {.passl: "-Wl,-soname,libwaku.so".} (considering libraryName=="waku", for example) let soName = fmt"-Wl,-soname,lib{libraryName}.so" res.add( newNimNode(nnkPragma).add( @@ -87,14 +78,13 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = ) ) elif defined(macosx): - ## Generates {.passl: "-install_name @rpath/libwaku.dylib".} let installName = fmt"-install_name @rpath/lib{libraryName}.dylib" res.add( newNimNode(nnkPragma).add( nnkExprColonExpr.newTree(ident"passl", newStrLitNode(installName)) ) ) - ## proc lib{libraryName}NimMain() {.importc.} + # proc lib{libraryName}NimMain() {.importc.} let libNimMainName = ident(fmt"lib{libraryName}NimMain") let importcPragma = nnkPragma.newTree(ident"importc") let procDef = newProc( @@ -105,19 +95,14 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = ) res.add(procDef) - # Create: var initState: Atomic[int] - # 0 = not started, 1 = in progress (some thread is running nimMainName), - # 2 = done. A boolean flag flipped before nimMainName runs would let a - # second concurrent caller skip past the gate while module init was - # still in flight — on Windows that surfaces as "WSAStartup failed" - # from chronos's later async dispatcher init on a watchdog thread. + # initState: 0=not started, 1=in progress, 2=done. Atomic (not a bool) so a racing caller can't skip past the gate mid-init (else Windows WSAStartup fails). let atomicType = nnkBracketExpr.newTree(ident("Atomic"), ident("int")) let varStmt = nnkVarSection.newTree( nnkIdentDefs.newTree(ident("initState"), atomicType, newEmptyNode()) ) res.add(varStmt) - ## Android chronicles redirection + # Android chronicles redirection let chroniclesBlock = quote: when defined(android) and compiles(defaultChroniclesStream.outputs[0].writer): defaultChroniclesStream.outputs[0].writer = proc( @@ -131,16 +116,9 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = let initializeLibraryProc = quote: proc `procName`*() {.exported.} = - ## Every Nim library needs to call `NimMain` once exactly, - ## to initialize the Nim runtime. - ## Being `` the value given in the optional - ## compilation flag --nimMainPrefix:yourprefix. - ## - ## Concurrent callers must NOT proceed past nimMainName until it has - ## fully returned: chronos's module-level globalInit (which calls - ## WSAStartup on Windows) runs as part of nimMainName, and a thread - ## that races past would later see "WSAStartup failed" when its - ## watchdog spins up a chronos dispatcher. + ## Calls `NimMain` exactly once to init the Nim runtime. Concurrent + ## callers must block until it returns (its chronos globalInit runs + ## WSAStartup on Windows; racing past yields "WSAStartup failed" later). var expected: int = 0 if initState.compareExchange(expected, 1): `nimMainName`() @@ -164,22 +142,9 @@ macro declareLibrary*( libType: untyped, defaultABIFormat: static[string] = "cbor", ): untyped = - ## Declares a library with the given name and emits the C-exported event - ## ABI on its `FFIContext`: - ## - ## - `{libraryName}_add_event_listener(ctx, event_name, cb, ud) -> uint64` - ## — registers `cb` for `event_name` and returns its stable id. `cb` - ## only receives events dispatched under `event_name`; subscribe to - ## each event separately. - ## - `{libraryName}_remove_event_listener(ctx, id) -> cint` — returns 0 on - ## success, non-zero if no listener with that id exists. - ## - ## `libType` is the Nim type of the main library object, used to type - ## the `ctx: ptr FFIContext[libType]` parameter. See - ## `examples/timer/timer.nim` for a working call site. - ## - ## `defaultABIFormat` (`"cbor"` default, or `"c"`) is the wire format every - ## annotation inherits unless it overrides with an `"abi = ..."` spec. + ## Declares a library and emits the C-exported event ABI (`_add_event_listener` / + ## `_remove_event_listener`) on its `FFIContext`. `defaultABIFormat` (`"cbor"`/`"c"`) + ## is inherited unless an annotation overrides via `"abi = ..."`. currentLibType = $libType # so handle-receiver `.ffi.` procs can resolve the pool let (abiOk, abiFmt) = parseABIFormatName(defaultABIFormat) @@ -193,10 +158,9 @@ macro declareLibrary*( var stmts = newStmtList() - # Emit the base bootstrap (pragmas, linker flags, NimMain, initializeLibrary) stmts.add(newCall(ident("declareLibraryBase"), newStrLitNode(libraryName))) - # The pool the generated wrappers validate against; ffiCtor/ffiDtor guard alike. + # The pool the generated wrappers validate against. let poolIdent = ident($libType & "FFIPool") stmts.add quote do: when not declared(`poolIdent`): @@ -242,10 +206,7 @@ macro declareLibrary*( ) ) - # --- {libraryName}_remove_event_listener -------------------------------- - # Param is `listenerId`, not `id` — `id` collides with chronos's - # `futures.id` template under quote injection rules and the captured - # symbol wins over the injected one. + # Param is `listenerId`, not `id`: `id` collides with chronos's `futures.id` template under quote injection and the captured symbol wins. let removeName = libraryName & "_remove_event_listener" let removeErr = "error: invalid context in " & removeName let removeBody = quote: diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index 2a10b8e..5fe7874 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -14,8 +14,7 @@ when defined(ffiGenBindings): import ../codegen/cddl proc requireLibraryDeclared(where: string) {.compileTime.} = - ## Enforce that `declareLibrary(...)` (which records name/type/default-ABI) - ## ran before this annotation. + ## Enforce that `declareLibrary(...)` ran before this annotation. if not libraryDeclared: error( where & @@ -25,9 +24,8 @@ proc requireLibraryDeclared(where: string) {.compileTime.} = proc resolveEventWireName( leading: seq[NimNode], userProcName: NimNode ): tuple[wireName: string, abiSpecStart: int] {.compileTime.} = - ## A leading string that doesn't parse as an `"abi = ..."` spec is the explicit - ## wire name; anything else means derive the name from the proc. Returns the - ## resolved name and the index where the trailing ABI specs begin. + ## A leading string that isn't an `"abi = ..."` spec is the explicit wire name; + ## otherwise derive from the proc. Returns name and index where ABI specs begin. if leading.len > 0 and leading[0].kind in {nnkStrLit, nnkRStrLit, nnkTripleStrLit} and ($leading[0]).len > 0 and not parseAbiSpec($leading[0]).ok: ($leading[0], 1) @@ -35,9 +33,8 @@ proc resolveEventWireName( (camelToSnakeCase($userProcName), 0) proc requireBeforeGenBindings(where: string) {.compileTime.} = - ## Enforce that this annotation expands before `genBindings()`. Anything - ## registered afterwards never reaches the generator, so turn what used to be - ## a silent drop into a loud error pointing at the fix. + ## Enforce this annotation expands before `genBindings()`; anything registered + ## afterwards never reaches the generator. if genBindingsEmitted: error( where & @@ -45,8 +42,7 @@ proc requireBeforeGenBindings(where: string) {.compileTime.} = ) proc resolveABIFormat(abiSpecs: seq[NimNode]): ABIFormat {.compileTime.} = - ## Resolve one annotation's ABI from its optional `"abi = ..."` string specs - ## (last wins), inheriting the library default when absent. + ## Resolve ABI from optional `"abi = ..."` specs (last wins), else lib default. var fmt = currentDefaultABIFormat for override in abiSpecs: if override.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}: @@ -61,8 +57,7 @@ proc resolveABIFormat(abiSpecs: seq[NimNode]): ABIFormat {.compileTime.} = fmt proc resolveFFISpecs(specs: seq[NimNode]): ABIFormat {.compileTime.} = - ## Resolve an annotation's `"abi = ..."` string specs (last wins), inheriting - ## the library-default ABI when absent. + ## Resolve `"abi = ..."` specs (last wins), else the library-default ABI. var abi = currentDefaultABIFormat for override in specs: if override.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}: @@ -80,8 +75,7 @@ proc resolveFFISpecs(specs: seq[NimNode]): ABIFormat {.compileTime.} = abi proc gateABIFormat(fmt: ABIFormat, where: string) {.compileTime.} = - ## Abort if the selected ABI's codegen isn't wired yet (only `Cbor` is), so a - ## `c` request fails loudly instead of emitting CBOR mislabeled as C. + ## Abort if the selected ABI's codegen isn't wired yet, failing loudly. if not abiCodegenImplemented(fmt): error( where & @@ -90,30 +84,18 @@ proc gateABIFormat(fmt: ABIFormat, where: string) {.compileTime.} = ) proc gateFFITypeABIFormat(fmt: ABIFormat, where: string) {.compileTime.} = - ## Type annotations only register metadata. `cbor` uses the generic CBOR - ## overloads, while `c` emits its `_CWire` companion from `genBindings()`. + ## Type annotations only register metadata; both ABIs are valid. case fmt of ABIFormat.Cbor, ABIFormat.C: discard proc isPtr(typ: NimNode): bool = - ## True iff `typ` is a `ptr T` type expression — i.e. an `nnkPtrTy` AST node. - ## Used by the binding-generator metadata path to flag pointer-typed params - ## and return types so the foreign side can render them as opaque addresses. + ## True iff `typ` is a `ptr T` type expression. typ.kind == nnkPtrTy proc rejectRawPtrType(typ: NimNode, where: string) = - ## Errors out at macro-expansion time if `typ` is `pointer` or `ptr T`. - ## Raw addresses must not cross the FFI boundary in user-declared fields, - ## parameters, or return types: the only pointer that legitimately crosses - ## the boundary is the opaque ctx handle returned by `.ffiCtor.` and passed - ## back as the first C-ABI argument, which the framework validates via - ## FFIContextPool.isValidCtx before dereferencing. Any other raw pointer - ## would hand the foreign caller an address with no way to validate its - ## memory state — see PR #23 review (discussion_r3236531712). - ## - ## `object` and `ref T` are not rejected: they flow as value copies through - ## cbor_serialization (the library's default `ref T` writer dereferences - ## and encodes the pointee, so no address crosses the boundary). + ## Reject `pointer`/`ptr T` at macro time: no unvalidatable raw address may + ## cross the FFI boundary (only the framework-managed ctx handle may). `object` + ## and `ref T` are fine — they flow as value copies through cbor_serialization. if typ.kind == nnkPtrTy: error( where & ": raw `ptr T` is not allowed across the FFI boundary " & @@ -128,9 +110,7 @@ proc rejectRawPtrType(typ: NimNode, where: string) = proc registerFFITypeInfo( typeDef: NimNode, abiFormat: ABIFormat ): NimNode {.compileTime.} = - ## Registers the type in ffiTypeRegistry for binding generation and returns - ## the clean typeDef. Serialization is handled by the generic overloads in - ## cbor_serial.nim. + ## Registers the type in ffiTypeRegistry and returns the clean typeDef. let typeName = if typeDef[0].kind == nnkPostfix: typeDef[0][1] @@ -164,8 +144,7 @@ proc registerFFITypeInfo( return typeDef proc nimTypeNameRepr(typ: NimNode): string = - ## Stringifies a parameter or field type for the binding-generator registry. - ## `$ident` works for simple types; bracket/dot/expression types need `repr`. + ## Stringifies a parameter or field type for the registry. case typ.kind of nnkIdent: $typ @@ -179,8 +158,7 @@ proc isHandleType(typ: NimNode): bool = typ.kind == nnkIdent and isFFIHandleTypeName($typ) proc storageType(typ: NimNode): NimNode = - ## In-Req-struct storage type. `cstring` rides as `string`; an {.ffiHandle.} - ## type rides as its `uint64` id; everything else as-is. + ## In-Req-struct storage type: `cstring`->`string`, handle->`uint64`, else as-is. if typ.kind == nnkIdent and $typ == "cstring": return ident("string") if isHandleType(typ): @@ -188,19 +166,9 @@ proc storageType(typ: NimNode): NimNode = typ proc unpackReqField*(fieldIdent, userType, decodedIdent: NimNode): NimNode = - ## Emits AST for unpacking one field from a CBOR-decoded Req struct into a - ## local typed as the user's original param type. - ## - ## `cstring` params are stored as `string` in the Req (per storageType) - ## and cast back via `.cstring` on unpack — safe because `decodedIdent` - ## outlives the cstring use within the generated proc body. - ## - ## Produces one of: - ## let : cstring = (.).cstring # for cstring - ## let = . # for everything else - ## - ## Built with the runtime AST API rather than `quote do:` so the proc is - ## callable from both macro context and ordinary code (e.g. unit tests). + ## Emits AST unpacking one field of a CBOR-decoded Req into a local of the + ## user's original type. `cstring` (stored as `string`) is cast back on unpack, + ## safe because `decodedIdent` outlives the cstring use in the generated body. let storedAsString = userType.kind == nnkIdent and $userType == "cstring" if not storedAsString: return newLetStmt(fieldIdent, newDotExpr(decodedIdent, fieldIdent)) @@ -213,8 +181,7 @@ proc unpackReqField*(fieldIdent, userType, decodedIdent: NimNode): NimNode = proc unpackHandleField*( fieldIdent, userType, ctxIdent, decodedIdent: NimNode ): NimNode = - ## Reconstitutes a handle param from its wire `uint64` via the ctx registry, - ## returning `RET_ERR` (Result.err) on a stale/forged/wrong-type id. + ## Reconstitutes a handle param from its wire `uint64` via the ctx registry. let errPrefix = "ffiHandle for parameter '" & $fieldIdent & "': " quote: let `fieldIdent` = block: @@ -223,9 +190,7 @@ proc unpackHandleField*( cast[`userType`](ffiH) proc cExportedParams(ctxType: NimNode): seq[NimNode] = - ## Standard parameter list for the C-exported wrapper of a .ffi. proc: - ## (returns cint; ctx, callback, userData, reqCbor, reqCborLen) - ## Shared by the async and sync paths so both wrappers carry the same ABI. + ## C-exported wrapper param list (cint; ctx, callback, userData, reqCbor, reqCborLen). var params: seq[NimNode] = @[] params.add(ident("cint")) params.add(newIdentDefs(ident("ctx"), ctxType)) @@ -238,34 +203,9 @@ proc cExportedParams(ctxType: NimNode): seq[NimNode] = proc buildReqTypeFromFields( reqTypeName: NimNode, paramNames: seq[string], paramTypes: seq[NimNode] ): NimNode = - ## Builds the per-proc Req `nnkTypeSection` (exported) from explicit - ## parallel lists of parameter names and types. The result is the AST for - ## a `type Foo* = object` declaration that the codegen later emits. - ## - ## `cstring` parameter types are rewritten to `string` (via storageType) - ## so the request can ride a plain CBOR text string on the wire. Empty - ## parameter lists get a single `_placeholder: uint8` field so the object - ## type is well-formed (Nim won't accept an empty `object` body here). - ## - ## Examples (in pseudo-Nim, showing the AST this proc produces): - ## - ## buildReqTypeFromFields( - ## reqTypeName = ident("EchoReq"), - ## paramNames = @["message", "delayMs"], - ## paramTypes = @[ident("cstring"), ident("int")]) - ## # → type EchoReq* = object - ## # message: string # cstring rewritten to string - ## # delayMs: int - ## - ## buildReqTypeFromFields( - ## reqTypeName = ident("VersionReq"), - ## paramNames = @[], - ## paramTypes = @[]) - ## # → type VersionReq* = object - ## # _placeholder: uint8 # placeholder for the empty-params case - ## - ## If `reqTypeName` is already a postfix node (e.g. `EchoReq*`) it is used - ## as-is; otherwise the `*` export marker is added. + ## Builds the exported per-proc Req `type Foo* = object` from parallel name/type + ## lists. `cstring` fields become `string`; an empty param list gets a single + ## `_placeholder: uint8` field since Nim rejects an empty object body here. var fields: seq[NimNode] = @[] for i in 0 ..< paramNames.len: let storedType = storageType(paramTypes[i]) @@ -292,20 +232,8 @@ proc buildReqTypeFromFields( newNimNode(nnkTypeSection).add(newTree(nnkTypeDef, typeName, newEmptyNode(), objTy)) proc buildRequestType(reqTypeName: NimNode, body: NimNode): NimNode = - ## Builds the per-proc Req object type from a registerReqFFI lambda body. - ## Field names match the lambda params; field types match the user-typed - ## param types (with `cstring` rewritten to `string` for transport). - ## - ## Builds: - ## type * = object - ## : - ## ... - ## - ## e.g.: - ## type EchoRequest* = object - ## message: string - ## delayMs: int - + ## Builds the per-proc Req object type from a registerReqFFI lambda body, + ## mirroring its param names and types (`cstring` -> `string`). var procNode = body if procNode.kind == nnkStmtList and procNode.len == 1: procNode = procNode[0] @@ -326,10 +254,8 @@ proc buildRequestType(reqTypeName: NimNode, body: NimNode): NimNode = return typeSection proc buildFFINewReqProc(reqTypeName, body: NimNode): NimNode = - ## Builds ffiNewReq: takes the user's typed params, packs them into a Req - ## object, CBOR-encodes the Req into one byte buffer, and constructs the - ## FFIThreadRequest that owns the buffer. - + ## Builds ffiNewReq: packs the user's typed params into a Req, CBOR-encodes it, + ## and constructs the FFIThreadRequest that owns the buffer. var formalParams = newSeq[NimNode]() var procNode: NimNode @@ -341,7 +267,6 @@ proc buildFFINewReqProc(reqTypeName, body: NimNode): NimNode = if procNode.kind != nnkLambda and procNode.kind != nnkProcDef: error "registerReqFFI expects a lambda definition. Found: " & $procNode.kind - # T: typedesc[XxxReq] let typedescParam = newIdentDefs(ident("T"), nnkBracketExpr.newTree(ident("typedesc"), reqTypeName)) formalParams.add(typedescParam) @@ -386,8 +311,7 @@ proc buildFFINewReqProc(reqTypeName, body: NimNode): NimNode = newBody.add( quote do: let typeStr = $T - # Encode directly into shared memory and hand ownership to the request, - # avoiding the seq[byte] → allocShared+copyMem second copy. + # Encode into shared memory, avoiding a second seq[byte] copy. let (sharedData, sharedLen) = cborEncodeShared(`reqObjIdent`) return FFIThreadRequest.initFromOwnedShared( callback, userData, typeStr.cstring, sharedData, sharedLen @@ -406,10 +330,7 @@ proc buildFFINewReqProc(reqTypeName, body: NimNode): NimNode = return newReqProc proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode = - ## Generates the FFI-thread-side processor for the Req type. - ## Decodes the CBOR payload into a Req struct, unpacks each field into a - ## local, then runs the user lambda body. - + ## FFI-thread processor: decodes the CBOR Req, unpacks fields, runs user body. if reqHandler.kind != nnkExprColonExpr: error( "Second argument must be a typed parameter, e.g., waku: ptr Waku. Found: " & @@ -431,10 +352,10 @@ proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode let procParams = procNode[3] var formalParams: seq[NimNode] = @[] - formalParams.add(procParams[0]) # return type + formalParams.add(procParams[0]) formalParams.add(typedescParam) formalParams.add(newIdentDefs(ident("request"), ident("pointer"))) - formalParams.add(newIdentDefs(reqHandler[0], rhs)) # e.g. waku: ptr Waku + formalParams.add(newIdentDefs(reqHandler[0], rhs)) let bodyNode = if procNode.body.kind == nnkStmtList: @@ -455,7 +376,6 @@ proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode ).valueOr: return err("CBOR decode failed for " & $T & ": " & $error) - # Unpack each field as a local typed as the user's original param type. for p in procParams[1 ..^ 1]: if isHandleType(p[1]): newBody.add unpackHandleField(p[0], p[1], reqHandler[0], decodedIdent) @@ -481,10 +401,8 @@ proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode return processProc proc addNewRequestToRegistry(reqTypeName, reqHandler: NimNode): NimNode = - ## Generates the dispatcher that the FFI thread calls: it invokes - ## processFFIRequest (which returns the user's typed Result[T, string]) and - ## encodes a successful T value with cborEncode into the seq[byte] payload. - + ## Dispatcher the FFI thread calls: runs processFFIRequest and cborEncodes the + ## typed T value into the seq[byte] payload. let returnType = nnkBracketExpr.newTree( ident("Future"), nnkBracketExpr.newTree( @@ -551,33 +469,9 @@ proc addNewRequestToRegistry(reqTypeName, reqHandler: NimNode): NimNode = return regAssign macro registerReqFFI*(reqTypeName, reqHandler, body: untyped): untyped = - ## Registers a request that will be handled by the FFI/working thread. - ## The request should be sent from the ffi consumer thread. - ## - ## The lambda passed to this macro must: - ## - Only have no-GC'ed types as parameters (cstring is allowed; it gets - ## transported as `string` in the per-proc Req struct). - ## - Return Future[Result[string, string]] and be annotated with {.async.} - ## The returned values are sent back to the ffi consumer thread. - ## - ## Example: - ## registerReqFFI(CreateNodeRequest, ctx: ptr FFIContext[Waku]): - ## proc( - ## config: NodeConfig, appCallbacks: AppCallbacks - ## ): Future[Result[string, string]] {.async.} = - ## ctx.myLib[] = (await createWaku(config, appCallbacks)).valueOr: - ## return err($error) - ## return ok("") - ## - ## The created FFI request is then dispatched from the ffi consumer thread - ## (generally the main thread) following something like: - ## - ## ffi.sendRequestToFFIThread( - ## ctx, CreateNodeRequest.ffiNewReq(callback, userData, config, appCallbacks) - ## ).isOkOr: - ## ... - - # Extract lambda params to generate fields + ## Registers a request handled by the FFI/working thread. The lambda takes only + ## no-GC'ed params (cstring travels as `string`) and must return + ## Future[Result[string, string]] {.async.}. let typeDef = buildRequestType(reqTypeName, body) let ffiNewReqProc = buildFFINewReqProc(reqTypeName, body) let processProc = buildProcessFFIRequestProc(reqTypeName, reqHandler, body) @@ -591,13 +485,8 @@ macro registerReqFFI*(reqTypeName, reqHandler, body: untyped): untyped = macro processReq*( reqType, ctx, callback, userData: untyped, args: varargs[untyped] ): untyped = - ## Expands T.processReq(ctx, callback, userData, a, b, ...) into a - ## sendRequestToFFIThread call that wraps the args in a freshly-built - ## FFIThreadRequest, with inline error reporting via `callback`. - ## - ## e.g.: - ## waku_dial_peerReq.processReq(ctx, callback, userData, peerMultiAddr, protocol, timeoutMs) - + ## Expands T.processReq(ctx, callback, userData, args...) into a + ## sendRequestToFFIThread call, reporting errors via `callback`. var callArgs = @[reqType, callback, userData] for a in args: callArgs.add a @@ -622,32 +511,9 @@ macro processReq*( return blockExpr macro ffiRaw*(args: varargs[untyped]): untyped = - ## Defines an FFI-exported proc that registers a request handler to be executed - ## asynchronously in the FFI thread. - ## - ## This is the "raw" / legacy form of the macro where the developer writes - ## the ctx, callback, and userData parameters explicitly. Additional parameters - ## travel as one CBOR blob. - ## - ## {.ffiRaw.} implicitly implies a Future[Result[string, string]] {.async.} - ## return type. - ## - ## When using {.ffiRaw.}, the first three parameters must be: - ## - ctx: ptr FFIContext[T] <-- T is the type that handles the FFI requests - ## - callback: FFICallBack - ## - userData: pointer - ## Then, additional parameters may be defined as needed, after these first - ## three, always considering that only no-GC'ed (or C-like) types are allowed. - ## - ## The wire format follows the library default and can be overridden with - ## `{.ffiRaw: "abi = c".}` / `{.ffiRaw: "abi = cbor".}`. - ## - ## e.g.: - ## proc waku_version( - ## ctx: ptr FFIContext[Waku], callback: FFICallBack, userData: pointer - ## ) {.ffiRaw.} = - ## return ok(WakuNodeVersionString) - + ## Raw/legacy FFI proc: first three params (ctx, callback, userData) are explicit, + ## extra no-GC'ed params travel as one CBOR blob, return is implied + ## Future[Result[string, string]] {.async.}. Override abi via `{.ffiRaw: "abi = c".}`. requireBeforeGenBindings("`.ffiRaw.`") requireLibraryDeclared("`.ffiRaw.`") let prc = args[^1] @@ -732,14 +598,9 @@ macro ffiRaw*(args: varargs[untyped]): untyped = return stmts macro ffiHandle*(args: varargs[untyped]): untyped = - ## Marks a `ref object` as an opaque FFI handle. Its wire form is a `uint64` - ## id; the live object stays in the per-ctx handle registry and never crosses. - ## - ## type Kernel {.ffiHandle.} = ref object - ## ... - ## - ## An optional `"abi = ..."` spec is accepted for surface parity but only - ## validated — a handle always rides as an abi-agnostic `uint64` id. + ## Marks a `ref object` as an opaque FFI handle: it rides as a `uint64` id while + ## the live object stays in the per-ctx registry. An `"abi = ..."` spec is + ## accepted but only validated (a handle is abi-agnostic). requireBeforeGenBindings("`.ffiHandle.`") requireLibraryDeclared("`.ffiHandle.`") let prc = args[^1] @@ -773,37 +634,15 @@ macro ffiHandle*(args: varargs[untyped]): untyped = return clean macro ffi*(args: varargs[untyped]): untyped = - ## Simplified FFI macro — applies to procs or types. - ## - ## On a type: `type Foo {.ffi.} = object` registers Foo for binding generation - ## and lets the generic cborEncode/cborDecode overloads handle serialization. - ## - ## On a proc: the annotated proc must have a first parameter of the library - ## type, optionally additional Nim-typed parameters, and return - ## Future[Result[RetType, string]]. It must NOT include ctx, callback, or - ## userData in its signature — the macro generates a C-exported wrapper that - ## takes one CBOR-encoded buffer as the call payload and fires the callback. - ## - ## The wire format defaults to the library's `defaultABIFormat` and can be - ## overridden per annotation with `{.ffi: "abi = c".}` / `{.ffi: "abi = cbor".}`. - ## - ## Example (type): - ## type EchoRequest {.ffi.} = object - ## message: string - ## delayMs: int - ## - ## Example (proc): - ## proc mylib_send*(w: MyLib, cfg: SendConfig): Future[Result[string, string]] {.ffi.} = - ## return ok("done") - + ## Simplified FFI macro for procs or types: a type registers for binding gen; a + ## proc takes a library-type param plus optional Nim params, returns + ## Future[Result[RetType, string]], and gets a C wrapper taking one CBOR buffer. requireBeforeGenBindings("`.ffi.`") # Annotated node is the last vararg; leading args are `"abi = ..."` specs. let prc = args[^1] let abiFormat = resolveFFISpecs(args[0 ..^ 2]) - # A value type stands alone (no library required). Its `c` companion is - # emitted later by `genBindings()`, since a type-pragma macro can only return - # a TypeDef; `cbor` rides the generic overloads. Both abis are valid here. + # A value type stands alone (no library required); its `c` companion is emitted later by `genBindings()`, since a type-pragma macro can only return a TypeDef. if prc.kind == nnkTypeDef: gateFFITypeABIFormat(abiFormat, "`.ffi.` type") var cleanTypeDef = prc.copyNimTree() @@ -881,10 +720,7 @@ macro ffi*(args: varargs[untyped]): untyped = var userProcName = procName if procName.kind == nnkPostfix: userProcName = procName[1] - ## Both the user-facing Nim proc and the C-exported wrapper share the user's - ## original name; their signatures differ so Nim resolves the call by - ## overload. The C wrapper additionally carries `{.exportc.}` so the foreign - ## ABI symbol is unchanged. + # Nim proc and C wrapper share the user's name (resolved by overload); the wrapper's `{.exportc.}` keeps the foreign ABI symbol. let cExportProcName = userProcName let ctxType = @@ -913,8 +749,7 @@ macro ffi*(args: varargs[untyped]): untyped = else: nimTypeNameRepr(retTypeInner) - # Built once, registered by whichever path runs (only `scalarFastPath` differs - # between them) and reused for the fast-path eligibility check below. + # Built once, registered by whichever path runs; reused for the check below. let procMeta = FFIProcMeta( procName: cExportName, libName: currentLibName, @@ -927,10 +762,7 @@ macro ffi*(args: varargs[untyped]): untyped = abiFormat: abiFormat, ) - # Does this proc qualify for the CBOR-free scalar fast path? Only `abi = c` - # opts in, and only when every wire param + the return is a plain scalar - # (see `isScalarOnly`) and the args fit the inline slots. A non-scalar - # `abi = c` proc rides the `_CWire` C-dispatch emitted by `asyncPath`. + # CBOR-free scalar fast path: only `abi = c` with all-scalar params/return that fit the inline slots; non-scalar `abi = c` rides the `_CWire` C-dispatch. let scalarEligible = abiFormat == ABIFormat.C and isScalarOnly(procMeta) and extraParamNames.len <= MaxScalarArgs @@ -938,8 +770,7 @@ macro ffi*(args: varargs[untyped]): untyped = let poolIdent = ident($libTypeName & "FFIPool") proc buildCtxGuard(): NimNode = - ## Nil-checks the callback and validates `ctx` against the lib's FFI pool, - ## replying `RET_ERR` before any request is built. Shared by both wire paths. + ## Nil-checks callback and validates `ctx`, replying `RET_ERR` before build. quote: if callback.isNil: return RET_MISSING_CALLBACK @@ -949,9 +780,7 @@ macro ffi*(args: varargs[untyped]): untyped = return RET_ERR proc buildSendAndReply(reqPtrIdent: NimNode): NimNode = - ## Hands `reqPtrIdent` to the FFI thread and maps the outcome to a C return - ## code, reporting any enqueue failure through the callback. Shared by both - ## wire paths. + ## Hands `reqPtrIdent` to the FFI thread and maps the outcome to a C return code. let sendResIdent = genSym(nskLet, "sendRes") quote: let `sendResIdent` = @@ -966,8 +795,7 @@ macro ffi*(args: varargs[untyped]): untyped = return RET_OK proc buildCExportProc(params: seq[NimNode], body: NimNode): NimNode = - ## The dynlib/exportc/cdecl C-ABI wrapper both wire paths emit; only the - ## params and body differ. + ## The dynlib/exportc/cdecl C-ABI wrapper both wire paths emit. newProc( name = postfix(cExportProcName, "*"), params = params, @@ -998,9 +826,7 @@ macro ffi*(args: varargs[untyped]): untyped = ) proc asyncPath(): NimNode = - ## Emits the C-exported wrapper and registers the handler. Every `.ffi.` proc - ## dispatches through the FFI thread and replies via its callback, honouring - ## `foreignThreadGc`, the MPSC ingress hand-off, and chronos's invariant. + ## Emits the C-exported wrapper and registers the FFI-thread handler. let helperProc = buildAsyncHelperProc() # registerReqFFI lambda: typed params, returns user's typed Result. @@ -1009,7 +835,7 @@ macro ffi*(args: varargs[untyped]): untyped = nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), libTypeName)) var lambdaParams = newSeq[NimNode]() - lambdaParams.add(retTypeNode) # Future[Result[RetType, string]] + lambdaParams.add(retTypeNode) for i in 0 ..< extraParamNames.len: lambdaParams.add(newIdentDefs(ident(extraParamNames[i]), extraParamTypes[i])) @@ -1038,15 +864,12 @@ macro ffi*(args: varargs[untyped]): untyped = registerReqFFI(`reqTypeName`, `ctxHandlerName`: `ptrFFICtx`): `lambdaNode` - # ------------------------------------------------------------------------- - # C-exported wrapper: takes (ctx, callback, userData, reqCbor, reqCborLen) - # ------------------------------------------------------------------------- + # C-exported wrapper: (ctx, callback, userData, reqCbor, reqCborLen). let exportedParams = cExportedParams(ctxType) let ffiBody = newStmtList() ffiBody.add buildCtxGuard() - # Build the FFIThreadRequest payload directly from the incoming bytes. let reqPtrIdent = genSym(nskLet, "reqPtr") ffiBody.add quote do: let typeStr = $`reqTypeName` @@ -1060,8 +883,7 @@ macro ffi*(args: varargs[untyped]): untyped = ffiProcRegistry.add(procMeta) if abiFormat == ABIFormat.C: - # The `abi = c` exported wrapper + reply trampoline are emitted at - # genBindings() time (see flushCAbiDispatch); the CBOR `ffiProc` is not. + # The `abi = c` wrapper + reply trampoline are emitted at genBindings() time (flushCAbiDispatch); the CBOR `ffiProc` is not. registerCAbiMethod( cExportName, libTypeName, reqTypeName, extraParamNames, extraParamTypes, resultRetType, @@ -1071,9 +893,8 @@ macro ffi*(args: varargs[untyped]): untyped = return newStmtList(helperProc, registerReq, ffiProc) proc scalarPath(): NimNode = - ## The scalar fast path lives in `ffi_scalar`; here we only build the shared - ## dispatch pieces (same helpers the usual path uses) and hand them over, so - ## the base macro carries none of the inline pack/unpack machinery. + ## Scalar fast path lives in `ffi_scalar`; here we only build the shared + ## dispatch pieces and hand them over. let reqPtrIdent = genSym(nskLet, "reqPtr") buildScalarPath( helperProc = buildAsyncHelperProc(), @@ -1196,9 +1017,7 @@ proc buildCtorProcessFFIRequestProc( paramTypes: seq[NimNode], libTypeName: NimNode, ): NimNode = - ## Decodes the CBOR payload, unpacks fields, runs the user body, and stores - ## the resulting library value in ctx.myLib. - + ## Decodes the Req, runs the user body, stores the library value in ctx.myLib. let returnType = nnkBracketExpr.newTree( ident("Future"), nnkBracketExpr.newTree(ident("Result"), ident("string"), ident("string")), @@ -1263,10 +1082,8 @@ proc buildCtorProcessFFIRequestProc( return processProc proc addCtorRequestToRegistry(reqTypeName, libTypeName: NimNode): NimNode = - ## Wraps the ctor processFFIRequest result in a seq[byte] dispatcher. - ## The ctor uniquely returns the ctx address as a decimal string; we wrap - ## it as raw UTF-8 bytes so the foreign side can read it back uniformly. - + ## Wraps the ctor processFFIRequest result in a seq[byte] dispatcher; the ctor + ## returns the ctx address as a decimal string, CBOR-encoded for the foreign side. let ctxType = nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), libTypeName)) @@ -1291,8 +1108,6 @@ proc addCtorRequestToRegistry(reqTypeName, libTypeName: NimNode): NimNode = let `resIdent` = await `callExpr` if `resIdent`.isErr: return err(`resIdent`.error) - # The ctor returns the ctx address as a decimal string; encode it as CBOR text - # for uniform decoding on the foreign side. return ok(cborEncode(`resIdent`.value)) let asyncProc = newProc( @@ -1315,31 +1130,9 @@ proc addCtorRequestToRegistry(reqTypeName, libTypeName: NimNode): NimNode = return regAssign macro ffiCtor*(args: varargs[untyped]): untyped = - ## Defines a C-exported constructor that creates an FFIContext and populates - ## ctx.myLib asynchronously in the FFI thread. - ## - ## The annotated proc must: - ## - Have Nim-typed parameters (carried over the wire as a single CBOR blob) - ## - Return Future[Result[LibType, string]] - ## - NOT include ctx, callback, or userData in its signature - ## - ## The wire format follows the library default and can be overridden with - ## `{.ffiCtor: "abi = c".}` / `{.ffiCtor: "abi = cbor".}`. - ## - ## Example: - ## proc mylib_create*(config: SimpleConfig): Future[Result[SimpleLib, string]] {.ffiCtor.} = - ## return ok(SimpleLib(value: config.initialValue)) - ## - ## The generated C-exported proc has the signature: - ## proc mylib_create(reqCbor: ptr byte, reqCborLen: csize_t, - ## callback: FFICallBack, userData: pointer): pointer - ## {.exportc, cdecl, raises: [].} - ## - ## Returns the context pointer synchronously, NULL on failure. The callback - ## also fires when async initialization completes, passing the ctx address as - ## a decimal string on success. The caller should hold the returned pointer - ## and pass it to subsequent .ffi. calls. - + ## C-exported constructor: creates an FFIContext and fills ctx.myLib async on the + ## FFI thread. Takes Nim params (one CBOR blob), no ctx/callback/userData. Wrapper + ## returns the ctx pointer sync (NULL on failure); callback fires with its address. requireBeforeGenBindings("`.ffiCtor.`") requireLibraryDeclared("`.ffiCtor.`") let prc = args[^1] @@ -1389,14 +1182,10 @@ macro ffiCtor*(args: varargs[untyped]): untyped = let typeDef = buildCtorRequestType(reqTypeName, paramNames, paramTypes) let ffiNewReqProc = buildCtorFFINewReqProc(reqTypeName, paramNames) - # The user-facing Nim proc keeps the user's original name with their declared - # signature; the C-exported wrapper moves to `ExportC` and - # binds the snake_case C symbol via `{.exportc.}`. var userProcName = procName if procName.kind == nnkPostfix: userProcName = procName[1] - # Both the Nim-facing async ctor and the C-exported wrapper share the user's - # name as overloads; the C wrapper's `{.exportc.}` keeps the ABI symbol. + # Nim ctor and C wrapper share the user's name as overloads; the wrapper's `{.exportc.}` keeps the ABI symbol. let cExportProcName = userProcName let helperProc = buildCtorBodyProc(userProcName, paramNames, paramTypes, libTypeName, bodyNode) @@ -1514,8 +1303,7 @@ macro ffiCtor*(args: varargs[untyped]): untyped = let stmts = if abiFormat == ABIFormat.C: - # The `abi = c` exported wrapper is emitted at genBindings() time (see - # flushCAbiDispatch); the CBOR `ffiProc` is not. + # The `abi = c` wrapper is emitted at genBindings() time; CBOR `ffiProc` isn't. registerCAbiCtor(cExportName, libTypeName, reqTypeName, paramNames, paramTypes) newStmtList(typeDef, ffiNewReqProc, helperProc, processProc, addToReg, poolDecl) else: @@ -1528,39 +1316,9 @@ macro ffiCtor*(args: varargs[untyped]): untyped = return stmts macro ffiDtor*(args: varargs[untyped]): untyped = - ## Defines a C-exported destructor that tears down the FFIContext. - ## - ## The annotated proc must have exactly one parameter of the library type. It - ## may be sync (no return type) or async (`Future[void]`) — an async dtor can - ## `await` a graceful library shutdown (e.g. `switch.stop()`) whose futures - ## live on the FFI event loop. - ## - ## The wire format follows the library default and can be overridden with - ## `{.ffiDtor: "abi = c".}` / `{.ffiDtor: "abi = cbor".}`. - ## - ## Example (sync): - ## proc echo_destroy*(e: Echo) {.ffiDtor.} = - ## e.close() - ## - ## Example (async): - ## proc waku_destroy*(w: Waku): Future[void] {.ffiDtor.} = - ## await w.stop() - ## - ## The generated C-exported proc has the signature: - ## int waku_destroy(void* ctx) - ## - ## A non-empty body is lifted into an async impl registered in the library's - ## `ffiTeardownHook` slot; the FFI thread awaits it on its own event loop, after - ## draining in-flight requests and just before it exits — so the body runs on - ## the worker thread, not the host (calling) thread. The C wrapper signals the - ## thread to stop and blocks (up to `ThreadExitTimeout`) until it, and the - ## teardown, finish, then frees the context. An empty/`discard` body registers - ## no hook. - ## - ## Returns RET_OK on success, RET_ERR on failure (null/invalid ctx, or - ## destroyFFIContext failure — e.g. a teardown that outlasts ThreadExitTimeout, - ## which leaks the context rather than hanging the caller). - + ## C-exported FFIContext destructor. Sync (no return) or async (`Future[void]`); + ## a non-empty body becomes an async `ffiTeardownHook` the FFI thread awaits at + ## shutdown, so teardown runs on the worker thread. RET_ERR on null/invalid ctx. requireBeforeGenBindings("`.ffiDtor.`") requireLibraryDeclared("`.ffiDtor.`") let prc = args[^1] @@ -1577,8 +1335,7 @@ macro ffiDtor*(args: varargs[untyped]): untyped = let libParamName = formalParams[1][0] let libTypeName = formalParams[1][1] - # A dtor is sync (no return type) or async (`Future[void]`); reject anything - # else up front rather than emitting an obscure downstream error. + # A dtor is sync (no return) or async (`Future[void]`); reject anything else. let retTypeNode = formalParams[0] let retIsFutureVoid = retTypeNode.kind == nnkBracketExpr and $retTypeNode[0] == "Future" and @@ -1596,10 +1353,7 @@ macro ffiDtor*(args: varargs[untyped]): untyped = else: raw let cExportName = camelToSnakeCase(procNameStr) - # The dtor only needs a C-exported wrapper; rename to a synthetic Nim ident - # so it doesn't shadow the user's chosen name (consistent with .ffi. / .ffiCtor.). - # The dtor only generates a C-exported wrapper; it uses the user's name - # directly (no overload needed — there's no Nim-facing helper here). + # The dtor only emits a C wrapper and uses the user's name directly (no Nim-facing helper to overload against). var cExportProcName = procName if procName.kind == nnkPostfix: cExportProcName = procName[1] @@ -1622,9 +1376,7 @@ macro ffiDtor*(args: varargs[untyped]): untyped = bodyNode[0].kind == nnkDiscardStmt ) - # Lift the body into an async impl registered in the per-library - # `ffiTeardownHook`, which the FFI thread awaits at shutdown (see ffi_thread.nim - # and ffiTeardownHook's docstring). The C wrapper no longer runs the body. + # Lift the body into an async `ffiTeardownHook` the FFI thread awaits at shutdown; the C wrapper no longer runs the body. let teardownImplName = genSym(nskProc, "ffiTeardownImpl") let teardownRegistration = if isNoop: @@ -1684,36 +1436,9 @@ macro ffiDtor*(args: varargs[untyped]): untyped = return stmts macro ffiEvent*(args: varargs[untyped]): untyped = - ## Declares a library-initiated event. The annotated proc has an empty - ## body — the macro fills it with a `dispatchFFIEventCbor` call so the - ## Nim author dispatches the event by calling the proc with a typed - ## payload, and the per-target codegens emit a typed handler dispatcher - ## on the foreign side. - ## - ## The wire-format event name is optional: when omitted it is derived from - ## the proc name via `camelToSnakeCase` (matching how {.ffi.} derives its C - ## export symbol), so `proc onPeerConnected(...)` becomes `on_peer_connected`. - ## Pass a string literal to override it verbatim (no case conversion). That - ## name appears in the CBOR `eventType` field and is the single source of - ## truth across Nim / C++ / Rust bindings. - ## - ## The wire format follows the library default and can be overridden by - ## passing an `"abi = ..."` spec (after the optional event name), e.g. - ## `{.ffiEvent("on_peer_connected", "abi = cbor").}`. - ## - ## Example: - ## type PeerInfo {.ffi.} = object - ## id: string - ## address: string - ## - ## proc onPeerConnected*(peer: PeerInfo) {.ffiEvent.} # -> "on_peer_connected" - ## - ## # ... then from inside any {.ffi.} handler: - ## onPeerConnected(PeerInfo(id: "p-1", address: "127.0.0.1")) - ## - ## Restriction (first pass): exactly one parameter. Multi-param events - ## need a synthesised envelope struct; planned for a follow-up. - + ## Declares a library-initiated event: the empty-bodied proc is filled with a + ## `dispatchFFIEventCbor` call. Wire name defaults to `camelToSnakeCase` of the + ## proc name (a string literal overrides it) and is the cross-binding source of truth. requireBeforeGenBindings("`.ffiEvent.`") requireLibraryDeclared("`.ffiEvent.`") if args.len < 1: @@ -1757,13 +1482,12 @@ macro ffiEvent*(args: varargs[untyped]): untyped = else: payloadTypeNode.repr - # The generated body: dispatchFFIEventCbor("wire_name", payload). let wireNameLit = newStrLitNode(wireName) let dispatchBody = newStmtList(newCall(ident("dispatchFFIEventCbor"), wireNameLit, payloadParamName)) var newParams = newSeq[NimNode]() - newParams.add(formalParams[0]) # return type (typically empty/void) + newParams.add(formalParams[0]) newParams.add(paramDef) let pragmas = @@ -1795,9 +1519,8 @@ macro ffiEvent*(args: varargs[untyped]): untyped = return generated proc reportScalarFastPathDrops(procs: seq[FFIProcMeta]) {.compileTime.} = - ## Scalar-fast-path procs have no foreign-binding codegen yet, so they can't - ## ride the generated bindings. Fail loudly, naming them, unless - ## `-d:ffiAllowScalarSkip` opts into the silent omission (then just hint). + ## Scalar-fast-path procs have no foreign-binding codegen yet; fail loudly + ## naming them unless `-d:ffiAllowScalarSkip` downgrades it to a hint. var skipped: seq[string] = @[] for p in procs: if p.scalarFastPath: @@ -1821,18 +1544,16 @@ proc reportScalarFastPathDrops(procs: seq[FFIProcMeta]) {.compileTime.} = ) proc bindingsOutputDir(lang, explicit: string): string {.compileTime.} = - ## Output dir for `lang`. Defaults to `_bindings/` next to the source - ## file being compiled (`querySetting(projectPath)`); an explicit - ## -d:ffiOutputDir override wins. + ## Output dir for `lang`; defaults to `_bindings/` next to the compiled + ## source, or an explicit -d:ffiOutputDir override. if explicit.len > 0: explicit else: return querySetting(SingleValueSetting.projectPath) / (lang & "_bindings") proc bindingsSrcPath(outDir, explicit: string): string {.compileTime.} = - ## Nim source path embedded in generated build files, expressed relative to - ## the output dir. Defaults to the compiled file (`querySetting(projectFull)`) - ## made relative to `outDir`; an explicit -d:ffiSrcPath override wins. + ## Nim source path embedded in build files, relative to `outDir`; defaults to + ## the compiled file, or an explicit -d:ffiSrcPath override. if explicit.len > 0: explicit else: @@ -1842,8 +1563,7 @@ when defined(ffiGenBindings): proc emitBindingsFor( lang: string, genProcs: seq[FFIProcMeta], libName, outDir, srcRel: string ) {.compileTime.} = - ## Route one language token to its generator; unknown tokens are a compile - ## error listing the valid set. + ## Route one language token to its generator; unknown tokens error. case lang of "rust": generateRustCrate( @@ -1868,36 +1588,9 @@ when defined(ffiGenBindings): macro genBindings*( outputDir: static[string] = ffiOutputDir, nimSrcRelPath: static[string] = ffiSrcPath ): untyped = - ## Emits C++ or Rust binding files from the compile-time FFI registries. - ## The foreign-side wrapper encodes one CBOR buffer per request. - ## - ## PLACEMENT REQUIREMENT: genBindings() must be called AFTER every {.ffi.}, - ## {.ffiCtor.} and {.ffiDtor.} annotation in the compilation unit. Each - ## pragma populates ffiProcRegistry / ffiTypeRegistry as the compiler - ## expands the AST; calling genBindings() earlier produces incomplete - ## bindings. - ## - ## In a single-file library, place it at the bottom of the file. - ## In a multi-file library, import all sub-modules first and call - ## genBindings() once at the bottom of the top-level compilation-root file. - ## - ## Supported languages (-d:targetLang): "rust" (default), "cpp", "c", "cddl". - ## Pass a comma-separated list to emit several at once from a single compile — - ## the backend dispatch loops over each language. The `c` target emits the - ## `abi = c` or CBOR C shape based on the library's ABI format (`defaultABIFormat`). - ## - ## Output dir defaults to `_bindings/` next to the compiled source; the - ## embedded nim source path is derived by making that source relative to the - ## output dir. Both can be overridden with -d:ffiOutputDir / -d:ffiSrcPath - ## (or the explicit arguments) — an override applies to every language. - ## Foreign-binding file emission is a no-op unless -d:ffiGenBindings is set; - ## the `abi = c` `_CWire` companions are emitted unconditionally (runtime - ## code, not generated files). - ## - ## Example (all via compile flags): - ## genBindings() - ## # nim c -d:ffiGenBindings -d:targetLang=rust,cpp,c mylib.nim - + ## Emits binding files from the compile-time FFI registries. MUST be called AFTER + ## every {.ffi.}/{.ffiCtor.}/{.ffiDtor.} annotation, so place it at the compilation + ## root's bottom. -d:targetLang picks languages; emission needs -d:ffiGenBindings. genBindingsEmitted = true when defined(ffiGenBindings): diff --git a/ffi/internal/ffi_scalar.nim b/ffi/internal/ffi_scalar.nim index bc7f1e7..a390590 100644 --- a/ffi/internal/ffi_scalar.nim +++ b/ffi/internal/ffi_scalar.nim @@ -1,14 +1,4 @@ ## CBOR-free scalar fast path for all-scalar `{.ffi: "abi = c".}` methods. -## -## Kept out of the base `ffi` macro so the usual CBOR/async dispatch path in -## `ffi_macro.nim` stays simple: the macro only decides eligibility -## (`isScalarOnly`) and, when it applies, hands the whole codegen to -## `buildScalarPath`. -## -## A scalar proc's C export takes its scalar args directly (no -## `reqCbor`/`reqCborLen`), packs them inline into the request (no envelope -## `c_malloc`, no CBOR), and the FFI-thread handler unpacks them, runs the user -## body, and returns the result as raw bytes (`ffiScalarRetBytes`). import std/macros import ../codegen/meta @@ -17,26 +7,19 @@ const scalarPodTypeNames = [ "int", "int8", "int16", "int32", "int64", "uint", "uint8", "uint16", "uint32", "uint64", "byte", "float", "float32", "float64", "bool", ] - ## Fixed-width POD scalars that fit one `uint64` slot and survive the async - ## hop by value. `cstring`/`string` are intentionally absent as *params*: - ## they point to caller memory the FFI thread reads after the call returns, - ## so passing them inline by value would be unsafe. + ## Fixed-width POD scalars that survive the async hop by value; `cstring`/ + ## `string` are excluded as params (they alias caller memory read after return). func isScalarParamTypeName*(name: string): bool = - ## A param type eligible for the CBOR-free scalar fast path. name in scalarPodTypeNames func isScalarReturnTypeName*(name: string): bool = - ## A return type eligible for the scalar fast path. Unlike params, a - ## `string`/`cstring` return is fine: the handler produces the bytes and they - ## ride back raw (like the error path), so no caller memory is aliased. + ## Unlike params, a `string`/`cstring` return is fine: the bytes ride back raw. name in scalarPodTypeNames or name == "string" or name == "cstring" func isScalarOnly*(p: FFIProcMeta): bool = - ## True iff `p` is a plain `{.ffi.}` method whose every wire param and return - ## is scalar — the whole signature crosses without CBOR or `_CWire`. Handles - ## and raw pointers are excluded (a handle needs a ctx-registry round-trip; - ## a pointer never crosses). + ## True iff every wire param and return of `p` is scalar. Handles and raw + ## pointers are excluded. if p.kind != FFIKind.FFI: return false if p.returnIsPtr or p.returnIsHandle: @@ -49,10 +32,8 @@ func isScalarOnly*(p: FFIProcMeta): bool = true func bindableProcs*(procs: seq[FFIProcMeta]): seq[FFIProcMeta] = - ## The procs the foreign-binding generators emit for. Scalar-fast-path procs - ## are dropped: their C export takes inline scalar args, not the CBOR - ## `(reqCbor, reqCborLen)` shape the current codegen assumes, so emitting a - ## CBOR caller for them would be wrong. Foreign codegen is a follow-up. + ## Procs the foreign-binding generators emit for; scalar-fast-path procs are + ## dropped (their inline-scalar export doesn't match the CBOR codegen shape). var kept: seq[FFIProcMeta] = @[] for p in procs: if not p.scalarFastPath: @@ -69,10 +50,8 @@ proc buildScalarPath*( extraParamTypes: seq[NimNode], procMeta: FFIProcMeta, ): NimNode {.compileTime.} = - ## Emits the scalar-fast-path codegen for one `.ffi.` proc. The generic - ## dispatch pieces (`helperProc`, `ctxGuard`, `sendAndReply`) are built by the - ## caller from the same shared helpers the usual path uses, so this only owns - ## the scalar-specific inline pack / unpack / raw-bytes wiring. + ## Emits the scalar-fast-path codegen for one `.ffi.` proc; the caller supplies + ## the generic dispatch pieces, this owns the inline pack/unpack/raw-bytes wiring. let scalarReqKey = camelName & "Req" let reqIdent = genSym(nskLet, "ffiReq") @@ -162,8 +141,7 @@ proc buildScalarPath*( ), ) - # Registered (not just skipped) so the compile-time metadata stays - # introspectable; `bindableProcs` drops it from foreign codegen. + # Registered so metadata stays introspectable; `bindableProcs` drops it later. var scalarMeta = procMeta scalarMeta.scalarFastPath = true ffiProcRegistry.add(scalarMeta) diff --git a/ffi/logging.nim b/ffi/logging.nim index bc772be..2350e88 100644 --- a/ffi/logging.nim +++ b/ffi/logging.nim @@ -1,6 +1,4 @@ -## This code has been copied and addapted from `status-im/nimbu-eth2` project. -## Link: https://github.com/status-im/nimbus-eth2/blob/c585b0a5b1ae4d55af38ad7f4715ad455e791552/beacon_chain/nimbus_binary_common.nim -## This is also copied in logos-messaging-nim repository (2025-12-10) +## Adapted from status-im/nimbus-eth2 nimbus_binary_common.nim. import std/[typetraits, os, strutils, syncio], chronicles, @@ -15,11 +13,8 @@ type LogFormat* = enum TEXT JSON -## Utils - proc stripAnsi(v: string): string = - ## Copied from: https://github.com/status-im/nimbus-eth2/blob/stable/beacon_chain/nimbus_binary_common.nim#L41 - ## Silly chronicles, colors is a compile-time property + ## chronicles colors are a compile-time property, so strip ANSI at runtime. var res = newStringOfCap(v.len) i: int @@ -31,14 +26,14 @@ proc stripAnsi(v: string): string = x = i + 1 found = false - while x < v.len: # look for [..m + while x < v.len: let c2 = v[x] if x == i + 1: if c2 != '[': break else: if c2 in {'0' .. '9'} + {';'}: - discard # keep looking + discard elif c2 == 'm': i = x + 1 found = true @@ -47,7 +42,7 @@ proc stripAnsi(v: string): string = break inc x - if found: # skip adding c + if found: continue res.add c inc i @@ -61,10 +56,8 @@ proc writeAndFlush(f: syncio.File, s: LogOutputStr) = except IOError: logLoggingFailure(cstring(s), getCurrentException()) -## Setup - proc setupLogLevel(level: LogLevel) = - # TODO: Support per topic level configuratio + # TODO: Support per topic level configuration topics_registry.setLogLevel(level) proc setupLogFormat(format: LogFormat, color = true) = @@ -94,7 +87,6 @@ proc setupLogFormat(format: LogFormat, color = true) = .} proc setupLog*(level: LogLevel, format: LogFormat) = - ## Logging setup # Adhere to NO_COLOR initiative: https://no-color.org/ let color = try: diff --git a/tests/bench/bench_ffi_submit.nim b/tests/bench/bench_ffi_submit.nim index 280371d..c5cec3d 100644 --- a/tests/bench/bench_ffi_submit.nim +++ b/tests/bench/bench_ffi_submit.nim @@ -17,8 +17,7 @@ var gSendErrors: Atomic[int] let settleTimeout = 30.seconds -## Forcing gate: min submit-throughput scaling (max-threads / 1-thread); red -## until the per-request submit lock is replaced. See README "Scaling gate". +## Min submit-throughput scaling gate (max-threads / 1-thread). See README. const RequiredScaling = 1.5 proc benchCallback( @@ -119,10 +118,7 @@ proc main() = let gateOn = getEnv("FFI_SCALING_GATE", "1") != "0" if perThread < 1 or iters < 1: quit("FFI_SUBMIT_PER_THREAD and FFI_SUBMIT_ITERS must be >= 1") - # Default sweep is light so CI (and the slower asan/tsan jobs) stays fast. Set - # FFI_SUBMIT_THREADS for the high-contention curve locally — under a sanitizer - # it can outrun `settleTimeout` and fail on timing, not a real bug. - # FFI_SUBMIT_THREADS="1,8,16,32,64,100" nimble bench_ffi_submit + # Default sweep is light so CI stays fast; set FFI_SUBMIT_THREADS locally for the high-contention curve. let threadCounts = block: var cs: seq[int] for part in getEnv("FFI_SUBMIT_THREADS", "1,2,4,8").split(','): diff --git a/tests/unit/fixtures/scalar_skip_fixture.nim b/tests/unit/fixtures/scalar_skip_fixture.nim index 91c4b2d..80c9a45 100644 --- a/tests/unit/fixtures/scalar_skip_fixture.nim +++ b/tests/unit/fixtures/scalar_skip_fixture.nim @@ -1,7 +1,6 @@ ## Compile fixture for the scalar-fast-path drop error (see -## tests/unit/test_scalar_skip_gen.nim). Under `-d:ffiGenBindings` the scalar -## `abi = c` proc below has no foreign-binding codegen, so genBindings() must -## fail — unless `-d:ffiAllowScalarSkip` is passed, which downgrades it to a hint. +## tests/unit/test_scalar_skip_gen.nim): the scalar `abi = c` proc has no +## foreign-binding codegen, so genBindings() fails unless -d:ffiAllowScalarSkip. import ffi, chronos @@ -19,8 +18,7 @@ proc scalarskip_create*(cfg: SkipConfig): Future[Result[SkipLib, string]] {.ffiC proc scalarskip_add*( lib: SkipLib, a: int, b: int ): Future[Result[int, string]] {.ffi: "abi = c".} = - ## All-scalar signature: dispatches through the CBOR-free fast path and has no - ## foreign-binding codegen yet. + ## All-scalar signature: CBOR-free fast path, no foreign-binding codegen yet. return ok(lib.base + a + b) genBindings() diff --git a/tests/unit/test_abi_format.nim b/tests/unit/test_abi_format.nim index 4e62c35..b09e35b 100644 --- a/tests/unit/test_abi_format.nim +++ b/tests/unit/test_abi_format.nim @@ -1,6 +1,4 @@ -## ABI-format annotation mechanism (issue #78): inherited default + per-spec -## override, recorded on the registry. `static:` blocks assert the registry at -## compile time; the parsing helpers run at runtime with unittest2. +## ABI-format annotation: inherited default + per-spec override, recorded on the registry. import std/strutils import unittest2 @@ -10,12 +8,11 @@ import ffi/codegen/meta type AbiLib = object -# Stub the dylib NimMain importc that declareLibrary emits (links as a plain exe). +# Stub the importc NimMain declareLibrary emits (plain-exe link). {.emit: "void libabitestNimMain(void) {}".} declareLibrary("abitest", AbiLib, defaultABIFormat = "cbor") -# declareLibrary must wire its parameter into the library-wide default. static: doAssert currentDefaultABIFormat == ABIFormat.Cbor @@ -25,31 +22,27 @@ type AbiConfig {.ffi.} = object type Pinged {.ffi.} = object n: int -# Plain annotations inherit the library default (cbor). +# Plain annotations inherit the library default. proc abitest_create*(c: AbiConfig): Future[Result[AbiLib, string]] {.ffiCtor.} = return ok(AbiLib()) proc abitest_ping*(lib: AbiLib): Future[Result[string, string]] {.ffi.} = return ok("pong") -# Explicit override — same value, but exercises the spec parser end-to-end. +# Explicit override exercises the spec parser. proc abitest_echo*( lib: AbiLib, n: int ): Future[Result[int, string]] {.ffi: "abi = cbor".} = return ok(n) -# Event with an explicit ABI override passed after the wire name. proc abitest_pinged*(p: Pinged) {.ffiEvent("on_pinged", "abi = cbor").} -# Handles accept the spec for surface parity; the wire form stays uint64. type PlainHandle {.ffiHandle.} = ref object v: int type AbiHandle {.ffiHandle: "abi = cbor".} = ref object v: int -# Both the inherited-default and the explicit-override annotations must record -# the resolved format on their registry entries. static: for name in ["abitest_create", "abitest_ping", "abitest_echo"]: var found = false @@ -99,9 +92,5 @@ suite "pragma override key parsing": suite "ABI proc-dispatch readiness": test "both cbor and c proc-dispatch are wired": - # This predicate is what the proc-form macros consult. Both ABIs now have a - # working dispatch path: `cbor` rides the generic overloads, `c` rides the - # `_CWire` companions (a CBOR-free foreign surface, CBOR transport - # internally). Events are the one `c` gap, gated separately in the macro. check abiCodegenImplemented(ABIFormat.Cbor) check abiCodegenImplemented(ABIFormat.C) diff --git a/tests/unit/test_c_abi_codegen.nim b/tests/unit/test_c_abi_codegen.nim index f5c6390..9b18af7 100644 --- a/tests/unit/test_c_abi_codegen.nim +++ b/tests/unit/test_c_abi_codegen.nim @@ -1,6 +1,5 @@ -## Unit tests for the CBOR-free (`abi = c`) C binding shape. Drives -## generateCAbiLibHeader against a synthetic registry (no macro pipeline, no -## files written), asserting on the emitted text. +## Unit tests for the CBOR-free (`abi = c`) C binding shape: drives +## generateCAbiLibHeader against a synthetic registry, asserting on the text. import std/strutils import unittest2 diff --git a/tests/unit/test_c_codegen.nim b/tests/unit/test_c_codegen.nim index f6bdeed..a7ff5ab 100644 --- a/tests/unit/test_c_codegen.nim +++ b/tests/unit/test_c_codegen.nim @@ -1,7 +1,5 @@ -## Unit-tests for the C binding generator. Drives generateCLibHeader (and the -## shared-header generators) directly against a synthetic registry (no macro -## pipeline, no files written) and asserts on the emitted text — the same -## approach as test_cddl_codegen. +## Unit-tests for the C binding generator: drives generateCLibHeader (and the +## shared-header generators) against a synthetic registry and asserts on the text. import std/[strutils, sequtils] import unittest2 @@ -81,8 +79,7 @@ suite "generateCLibHeader: types and codecs": check "bool has_value;" in header test "a struct whose fields own no heap memory gets no free helper": - # EchoResponse has only a string field, so it does get a free; assert the - # inverse with the per-proc Version-less example via the int-only check: + # EchoResponse has a string field, so it gets a free helper. check "timer_free_EchoResponse(" in header suite "generateCLibHeader: ABI declarations and context API": @@ -257,10 +254,7 @@ suite "generateCLibHeader: scalar-fast-path procs are excluded": check "int calc_add(" notin header # note: calc_add_event_listener is unrelated test "unfiltered, the generator would emit a wrong-ABI CBOR caller for it": - # Regression guard for the bug bindableProcs prevents: handed the raw - # registry, the C generator declares calc_add with the CBOR - # (req_cbor, req_cbor_len) prototype, which mismatches its real inline-arg - # export. genBindings must feed the filtered set (see the `of "c":` branch). + # Unfiltered, the generator emits a wrong-ABI CBOR prototype for the scalar proc. let header = generateCLibHeader(procs, types, "calc") check "int calc_add(void* ctx, FFICallback callback, void* user_data, " & "const uint8_t* req_cbor, size_t req_cbor_len);" in header diff --git a/tests/unit/test_c_wire.nim b/tests/unit/test_c_wire.nim index 15a435e..3e1a8c0 100644 --- a/tests/unit/test_c_wire.nim +++ b/tests/unit/test_c_wire.nim @@ -1,15 +1,4 @@ -## Round-trip correctness for the `c` (`abi = c` C-struct) ABI codec. -## -## Each `{.ffi: "abi = c".}` type gets a `_CWire` companion plus -## `cwirePack` / `cwireUnpack` / `cwireFree`. This asserts -## `cwireUnpack(cwirePack(x)) == x` across the supported field shapes — -## scalars, strings, `seq`, `Option`, `array`, named `tuple`, nested {.ffi.} -## structs, and nested composites (`seq[Option[T]]`, `array[N, tuple[...]]`, -## `Option[array[...]]`, ...) — including the empty/none/empty-string edge -## cases the cstring/pointer encoding must handle. -## -## `genBindings()` flushes the cwire companions for every abi=c type declared -## above it (a type-pragma macro can't splice them in at the type site). +## c (abi = c) ABI codec: asserts cwireUnpack(cwirePack(x)) == x across all field shapes. import std/options import unittest2 @@ -34,9 +23,7 @@ type Outer {.ffi: "abi = c".} = object optTags: seq[Option[string]] type Shapes {.ffi: "abi = c".} = object - ## Exercises array/tuple wire shapes and their cross-nestings (array of - ## array, tuple in array, array in Option, tuple in seq) alongside GC'd and - ## nested-{.ffi.} element types. + ## Exercises array/tuple wire shapes and their cross-nestings. coords: array[3, int] labels: array[2, string] cells: array[2, Inner] @@ -50,8 +37,6 @@ type Shapes {.ffi: "abi = c".} = object genBindings() proc roundTrip(o: Outer): Outer = - ## Pack into the wire struct, copy back out, then release the wire - ## allocations — the exact lifecycle a boundary crossing would use. var wire: Outer_CWire cwirePack(wire, o) let back = cwireUnpack(wire) @@ -59,8 +44,6 @@ proc roundTrip(o: Outer): Outer = return back proc roundTrip(o: Shapes): Shapes = - ## Same pack/unpack/free lifecycle as the `Outer` overload, for the - ## array/tuple shapes. var wire: Shapes_CWire cwirePack(wire, o) let back = cwireUnpack(wire) @@ -114,7 +97,7 @@ suite "c-ABI cwire round-trip": name: "n", inner: Inner(label: "i", weight: 9), items: @[Inner(label: "alpha", weight: 10), Inner(label: "beta", weight: 20)], - note: some(""), # some-of-empty-string must stay `some`, not collapse to none + note: some(""), # some-of-empty must stay `some` ) let back = roundTrip(o) check back.items.len == 2 diff --git a/tests/unit/test_cddl_codegen.nim b/tests/unit/test_cddl_codegen.nim index df32825..90075ef 100644 --- a/tests/unit/test_cddl_codegen.nim +++ b/tests/unit/test_cddl_codegen.nim @@ -1,6 +1,4 @@ -## Unit-tests for the CDDL schema generator. Drives it directly against a -## synthetic `ffiProcRegistry` / `ffiTypeRegistry` so we don't need to invoke -## the macro pipeline (and thus don't write any files). +## Unit-tests for the CDDL schema generator, driven against a synthetic registry. import std/strutils import unittest2 diff --git a/tests/unit/test_ctx_validation.nim b/tests/unit/test_ctx_validation.nim index dac3d04..2b834c5 100644 --- a/tests/unit/test_ctx_validation.nim +++ b/tests/unit/test_ctx_validation.nim @@ -5,7 +5,7 @@ import ffi type TestLib = object -# Stub the dylib NimMain importc that declareLibrary emits (this links as a plain exe). +# Stub the importc NimMain declareLibrary emits (plain-exe link). {.emit: "void libctxvaltestNimMain(void) {}".} declareLibrary("ctxvaltest", TestLib) @@ -38,11 +38,7 @@ proc validationCallback( s[].called.store(true) suite "ctx pointer validation at the FFI entry point": - # The macro-generated FFI entry point validates ctx via - # FFIPool.isValidCtx. Any caller — C or Nim — that passes a nil or - # offset-invalid ctx with a valid callback should receive RET_ERR via the - # callback and the proc should return RET_ERR, never crash. - + # A nil or offset-invalid ctx must yield RET_ERR via callback and return, never crash. test "nil ctx with valid callback returns RET_ERR via callback, no crash": var s: CallbackState initCbState(s) diff --git a/tests/unit/test_event_dispatch.nim b/tests/unit/test_event_dispatch.nim index 9d18c75..3f7341c 100644 --- a/tests/unit/test_event_dispatch.nim +++ b/tests/unit/test_event_dispatch.nim @@ -1,5 +1,4 @@ -## End-to-end tests for `dispatchFFIEvent` / `dispatchFFIEventCbor`, -## driven through a real `FFIContext` so the threadvar wiring is exercised. +## End-to-end tests for `dispatchFFIEvent` / `dispatchFFIEventCbor` via a real FFIContext. import std/[locks, os] import unittest2 @@ -29,7 +28,6 @@ proc deinitCallbackData(d: var CallbackData) = d.lock.deinitLock() template setupCallbackData(name: untyped) = - ## Declares `name`, inits it, and defers its deinit in the caller's scope. var name: CallbackData initCallbackData(name) defer: @@ -67,7 +65,6 @@ proc callbackBytes(d: var CallbackData): seq[byte] = bytes template withPool(ctxIdent: untyped, body: untyped) = - ## Sets up pool + ctx, runs body, destroys on exit. var pool: FFIContextPool[TestEvtLib] let ctxIdent = pool.createFFIContext().valueOr: check false @@ -89,7 +86,6 @@ registerReqFFI(EmitRawBytesEventRequest, lib: ptr TestEvtLib): @[byte 0x01, 0x02, 0x03] return ok("emitted") -# Add/remove worker for the registry-race regression test. type SetterArgs = tuple[ ctx: ptr FFIContext[TestEvtLib], stop: ptr Atomic[bool], target: ptr CallbackData @@ -103,8 +99,7 @@ proc setterThreadBody(args: SetterArgs) {.thread.} = suite "dispatchFFIEventCbor": test "delivers EventEnvelope-shaped CBOR payload to event callback": - # CallbackData defers declared first run last (LIFO), AFTER pool destroy - # joins the event thread — otherwise TSan flags captureCb on a destroyed mutex. + # LIFO defer order: pool destroy joins the event thread before mutex teardown. setupCallbackData(evt) setupCallbackData(rsp) @@ -144,17 +139,14 @@ suite "dispatchFFIEvent with seq[byte]": check callbackBytes(evt) == @[byte 0x01, 0x02, 0x03] when not defined(gcRefc): - ## Skipped under refc: setter threads grow/shrink the per-event listener - ## seq, and refc's per-thread GC heap makes that unsafe cross-thread. + ## Skipped under refc: cross-thread listener-seq growth is unsafe with refc's GC. suite "FFIEventRegistry concurrent access": - ## Regression for PR #39 (r3288220895 / r3289285387). - ## Validate with: NIM_FFI_SAN=tsan NIM_FFI_MM=orc nimble test_sanitized + ## Regression for PR #39; validate with NIM_FFI_SAN=tsan NIM_FFI_MM=orc. test "concurrent add/remove writers vs dispatch reads stay race-free": setupCallbackData(evt) setupCallbackData(rsp) withPool(ctx): - # Seed an initial listener so the first dispatch has a target. discard addEventListener(ctx[].eventRegistry, "message_sent", captureCb, addr evt) @@ -219,17 +211,12 @@ suite "registry lock held during invocation": check st.entered.load() check not st.exited.load() - # Lock-during-invocation: remove blocks until dispatch finishes, - # by which time slowEventCb has set exited=true. + # remove blocks until the in-flight dispatch finishes (exited=true by then). check removeEventListener(ctx[].eventRegistry, id) check st.exited.load() suite "liveness events": - ## `onNotResponding` / `onResponding` bypass the event queue and dispatch - ## directly to listeners — the queue itself may be wedged behind the same - ## stall they're signalling. These tests pin down the wire shape (event - ## name + CBOR-encoded `EventEnvelope[…]`) so a future refactor can't - ## silently break consumers polling for the "library hung" signal. + ## Liveness signals dispatch directly, bypassing the (maybe wedged) queue. test "onNotResponding delivers EventEnvelope[NotRespondingEvent] to subscribers": setupCallbackData(evt) @@ -263,17 +250,10 @@ suite "liveness events": test "liveness events with no subscriber are a no-op": withPool(ctx): - # No listener registered — must not crash, must not block. onNotResponding(ctx) onResponding(ctx) suite "event thread drains queued events": - ## The event thread wakes every `EventThreadTickInterval` (or on - ## `eventQueueSignal`, not exported) and drains `eventQueue` into the - ## registered listeners. This test pushes a c_malloc'd payload onto the - ## queue from the test thread and waits for the tick-driven drain to - ## deliver it — exercises the `tryEnqueueEvent` → `drainEventQueue` → - ## `dispatchQueuedEvent` → listener path end-to-end. test "enqueued event is delivered to subscriber within a tick": setupCallbackData(evt) @@ -281,8 +261,6 @@ suite "event thread drains queued events": const QueuedEvtName = "queued_evt" discard addEventListener(ctx[].eventRegistry, QueuedEvtName, captureCb, addr evt) - # `tryEnqueueEvent` copies the payload into the queue's slab and stores - # `name` non-owning; the const's cstring backing lives for the process. let payload = @[byte 0xDE, 0xAD, 0xBE, 0xEF] check tryEnqueueEvent( ctx[].eventQueue, cstring(QueuedEvtName), unsafeAddr payload[0], payload.len @@ -293,9 +271,7 @@ suite "event thread drains queued events": check callbackBytes(evt) == payload suite "oversize payload falls back to heap": - ## Payloads larger than `MaxEventPayloadBytes` can't use the fixed slab slot; - ## `tryEnqueueEvent` `c_malloc`s a one-off buffer (`dataHeapOwned`) that - ## `commitDequeue` frees after dispatch. This exercises that path end-to-end. + ## Payloads over MaxEventPayloadBytes take the c_malloc'd heap fallback. test "payload above the slab budget still delivers intact": setupCallbackData(evt) @@ -314,20 +290,15 @@ suite "oversize payload falls back to heap": waitCallback(evt) check evt.retCode == RET_OK - # captureCb caps at its 1024-byte buffer; compare the delivered prefix. - let n = min(payload.len, 1024) + let n = min(payload.len, 1024) # captureCb caps at its 1024-byte buffer check callbackBytes(evt) == payload[0 ..< n] suite "oversize event name falls back to heap": - ## A name longer than `MaxEventNameBytes` overflows the fixed name slot and - ## takes the same `nameHeapOwned` `c_malloc` fallback. Exercises that branch - ## and confirms the full (untruncated) name still routes to its listener. + ## A name over MaxEventNameBytes takes the heap fallback and still routes. test "name above the name-slab budget still delivers to the right listener": setupCallbackData(evt) withPool(ctx): - # Comfortably longer than MaxEventNameBytes (64), so the name can't fit - # its slab slot and must be heap-allocated. const LongEvtName = "oversize_event_name_that_is_deliberately_much_longer_than_the_name_slab_budget" check LongEvtName.len > MaxEventNameBytes diff --git a/tests/unit/test_event_thread.nim b/tests/unit/test_event_thread.nim index 1b5f252..41bdb0b 100644 --- a/tests/unit/test_event_thread.nim +++ b/tests/unit/test_event_thread.nim @@ -27,7 +27,6 @@ proc deinitCallbackData(d: var CallbackData) = d.lock.deinitLock() template setupCallbackData(name: untyped) = - ## Declares `name`, inits it, and defers its deinit in the caller's scope. var name: CallbackData initCallbackData(name) defer: @@ -59,7 +58,6 @@ proc resetCalled(d: var CallbackData) = release(d.lock) proc waitCallbackTimeout(d: var CallbackData, timeoutMs: int): bool = - ## Polls under `d.lock` so the load syncs with the `captureCb` writer. let deadline = Moment.now() + timeoutMs.milliseconds while true: acquire(d.lock) @@ -72,7 +70,6 @@ proc waitCallbackTimeout(d: var CallbackData, timeoutMs: int): bool = os.sleep(10) template withPool(ctxIdent: untyped, body: untyped) = - ## Sets up a pool + ctx, runs body, destroys on exit. var pool: FFIContextPool[TestEvtLib] let ctxIdent = pool.createFFIContext().valueOr: check false @@ -90,7 +87,6 @@ registerReqFFI(PingEvent, lib: ptr TestEvtLib): proc(): Future[Result[string, string]] {.async.} = return ok("pong") -# Atomic switch so the wedge fires deterministically per test. var gBlockingEnabled: Atomic[bool] gBlockingEnabled.store(false) @@ -123,9 +119,7 @@ registerReqFFI(CaptureFfiTidRequest, lib: ptr TestEvtLib): suite "event delivery is asynchronous": test "listener runs on the event thread, not the FFI thread": - # CallbackData defers declared first run last (LIFO): pool-destroy joins - # the event thread before any still-held mutex is torn down. TSan otherwise - # flags `captureCb` on a destroyed mutex. + # LIFO defer order: pool-destroy joins the event thread before mutex teardown. setupCallbackData(evt) setupCallbackData(rsp) @@ -172,8 +166,6 @@ suite "FFI thread independence": waitCallback(rsp) resetCalled(rsp) - # chronos's `Moment` — std/times exports a `milliseconds` that - # shadows chronos's at this generic-instantiation site. let started = Moment.now() check sendRequestToFFIThread(ctx, PingEvent.ffiNewReq(captureCb, addr rsp)).isOk() waitCallback(rsp) @@ -182,8 +174,7 @@ suite "FFI thread independence": check elapsed < 100.milliseconds # under the 150 ms slow-listener sleep when not defined(gcRefc): - ## Skipped under refc: sleeping the FFI thread inside a sync handler - ## interacts badly with refc + existing destroy-on-time policies. + ## Skipped under refc: sleeping the FFI thread in a sync handler misbehaves there. suite "FFI heartbeat staleness": test "wedged FFI thread triggers onNotResponding via heartbeat": setupCallbackData(notif) @@ -194,7 +185,7 @@ when not defined(gcRefc): check false return defer: - # Disable wedge first so destroy isn't blocked by the still-sleeping handler. + # Disable wedge first so destroy isn't blocked by the sleeping handler. gBlockingEnabled.store(false) discard pool.destroyFFIContext(ctx) @@ -205,9 +196,9 @@ when not defined(gcRefc): # Wait out the start-delay so the heartbeat check is armed. os.sleep(FFIHeartbeatStartDelay.milliseconds.int + 200) - # Wedge long enough to cross at least one tick boundary. gBlockingEnabled.store(true) let wedgeMs = + # long enough to cross a tick boundary (EventThreadTickInterval + FFIHeartbeatStaleThreshold).milliseconds.int + 1500 check sendRequestToFFIThread( ctx, BlockingRequest.ffiNewReq(captureCb, addr rsp, wedgeMs) @@ -241,8 +232,7 @@ proc deinitBackpressure(b: var BackpressureState) = proc backpressureCb( retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer ) {.cdecl, gcsafe, raises: [].} = - ## First call signals entered then blocks under reg.lock to back-pressure - ## subsequent dispatches — gives a deterministic way to fill the queue. + ## First call signals entered then blocks under reg.lock to fill the queue. let b = cast[ptr BackpressureState](userData) if not b[].entered.exchange(true): acquire(b[].enteredLock) @@ -277,8 +267,7 @@ suite "queue overflow": ctx[].eventRegistry, NotRespondingEventName, captureCb, addr notif ) - # Kick one event so the listener holds reg.lock; subsequent enqueues - # pile up undrained. + # Listener holds reg.lock so later enqueues pile up undrained. check sendRequestToFFIThread( ctx, EmitLatchEvent.ffiNewReq(captureCb, addr rsp, -1) ) @@ -290,7 +279,6 @@ suite "queue overflow": wait(bp.enteredCond, bp.enteredLock) release(bp.enteredLock) - # Burst > capacity in one request; tail enqueues flip the stuck flag. resetCalled(rsp) check sendRequestToFFIThread( ctx, BurstEmit.ffiNewReq(captureCb, addr rsp, EventQueueCapacity + 8) @@ -305,8 +293,6 @@ suite "queue overflow": check res.isErr() check res.error.contains("stuck") - # Release backpressure so drain advances and the stuck flag fires - # not_responding. acquire(bp.releaseLock) bp.release.store(true) signal(bp.releaseCond) diff --git a/tests/unit/test_ffi_context.nim b/tests/unit/test_ffi_context.nim index b1080a1..247142e 100644 --- a/tests/unit/test_ffi_context.nim +++ b/tests/unit/test_ffi_context.nim @@ -5,8 +5,6 @@ import ffi type TestLib = object -## Per-request callback state. The test thread blocks on `cond` until the -## FFI thread signals it — no polling, no CPU waste. type CallbackData = object lock: Lock cond: Cond @@ -27,8 +25,7 @@ proc deinitCallbackData(d: var CallbackData) = proc testCallback( retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer ) {.cdecl, gcsafe, raises: [].} = - # A progress ping is not an answer: waking waitCallback here would report a - # non-terminal code as the result. Tests asserting on pings use staleCallback. + # A progress ping is not a terminal answer; skip it here. if retCode == RET_STALE_WARN: return let d = cast[ptr CallbackData](userData) @@ -56,7 +53,6 @@ proc callbackBytes(d: var CallbackData): seq[byte] = return bytes proc callbackErr(d: var CallbackData): string = - ## Reads the error payload (sent as raw UTF-8 bytes on RET_ERR). var msg = newString(d.msgLen) if d.msgLen > 0: copyMem(addr msg[0], addr d.msg[0], d.msgLen) @@ -286,7 +282,6 @@ suite "sendRequestToFFIThread": check sendRequestToFFIThread(ctx, FailRequest.ffiNewReq(testCallback, addr d)).isOk() waitCallback(d) check d.retCode == RET_ERR - # Errors are raw UTF-8 — not CBOR. check callbackErr(d) == "intentional failure" test "empty ok response delivers empty message": @@ -306,7 +301,6 @@ suite "sendRequestToFFIThread": .isOk() waitCallback(d) check d.retCode == RET_OK - # CBOR-encoded "" is a single byte (text string of length 0): 0x60 check cborDecode(callbackBytes(d), string).value == "" test "sequential requests are all processed": @@ -333,7 +327,7 @@ suite "sendRequestToFFIThread": type SimpleLib = object value: int -# Stub the dylib NimMain importc that declareLibrary emits (this links as a plain exe). +# Stub the importc NimMain declareLibrary emits (plain-exe link). {.emit: "void libtestlibNimMain(void) {}".} declareLibrary("testlib", SimpleLib) @@ -353,7 +347,6 @@ proc encodedPtr(bytes: var seq[byte]): ptr byte = cast[ptr byte](addr bytes[0]) proc ctorAddrFromCbor(bytes: seq[byte]): uint = - ## The ctor success payload is a CBOR text string of the decimal address. let addrStr = cborDecode(bytes, string).valueOr: return 0 cast[uint](parseBiggestUInt(addrStr)) @@ -416,7 +409,6 @@ suite "simplified .ffi. macro": defer: deinitCallbackData(d) - # The .ffi. macro packs all extra params into one CBOR Req struct. var reqBytes = cborEncode(TestlibSendReq(cfg: SendConfig(message: "hello"))) let ret = testlib_send( ctx, testCallback, addr d, encodedPtr(reqBytes), reqBytes.len.csize_t @@ -432,10 +424,7 @@ proc testlib_version*(lib: SimpleLib): Future[Result[string, string]] {.ffi.} = return ok("v" & $lib.value) suite "sync-body .ffi. is dispatched on FFI thread": - ## Before PR #23 (items 1–5), a `.ffi.` body without `await` was emitted as - ## an inline-on-foreign-thread fast path. That was deleted; all `.ffi.` - ## procs now go through the FFI thread. This test asserts the round-trip - ## still produces the expected payload via the callback. + ## All `.ffi.` procs go through the FFI thread, even sync bodies (PR #23). test "sync body still produces correct payload via callback": var ctorD: CallbackData initCallbackData(ctorD) @@ -461,42 +450,31 @@ suite "sync-body .ffi. is dispatched on FFI thread": defer: deinitCallbackData(d2) - # No-extra-param .ffi. proc; pack an empty Req. var emptyBytes = cborEncode(TestlibVersionReq()) let ret = testlib_version( ctx, testCallback, addr d2, encodedPtr(emptyBytes), emptyBytes.len.csize_t ) check ret == RET_OK - waitCallback(d2) # always asynchronous now + waitCallback(d2) check d2.retCode == RET_OK check cborDecode(callbackBytes(d2), string).value == "v3" -# Nim-native API (no callbacks, no CBOR buffers): the original proc name -# resolves to the user's declared async signature and is callable directly. - suite "Nim-native .ffi. / .ffiCtor. API": test "user proc names retain their declared Future[Result[T,string]] shape": let lib = SimpleLib(value: 9) - # Async {.ffi.} proc — call directly without ctx/callback dance. let echoed = waitFor testlib_send(lib, SendConfig(message: "direct")) check echoed.isOk check echoed.value == "echo:direct:9" - # Sync {.ffi.} body — still typed as Future[Result[T,string]] per the - # user's source-level declaration (b): completed-future wrapper. let v = waitFor testlib_version(lib) check v.isOk check v.value == "v9" - # The ctor body is similarly callable from Nim with its declared signature. let ctorRes = waitFor testlib_create(SimpleConfig(initialValue: 21)) check ctorRes.isOk check ctorRes.value.value == 21 -# A sync `.ffi.` body (no `await`) must run on the FFI thread, not the caller's, -# so it goes through `foreignThreadGc`, the MPSC ingress hand-off, and chronos's -# single-thread invariant. Records `getThreadId()` in a sync body and asserts it. - +# Records getThreadId() to prove a sync `.ffi.` body runs on the FFI thread. var gRecordedHandlerTid: Atomic[int] type RecordTidReq {.ffi.} = object @@ -505,8 +483,6 @@ type RecordTidReq {.ffi.} = object proc testlib_record_tid*( lib: SimpleLib, req: RecordTidReq ): Future[Result[int, string]] {.ffi.} = - ## Sync body — used to live on the inline fast-path; must now run on the - ## FFI thread. let tid = getThreadId() gRecordedHandlerTid.store(tid) return ok(tid) @@ -548,23 +524,13 @@ suite "sync-body .ffi. runs on FFI thread (PR #23 regression)": let handlerTid = gRecordedHandlerTid.load() check handlerTid != 0 - # The whole point of the fix: even a sync-body handler is dispatched off - # the caller thread. If this fails the inline fast-path is back. check handlerTid != callerTid - # And the callback payload (the recorded tid) matches what the handler stored. check cborDecode(callbackBytes(d), int).value == handlerTid -# Reentrancy guard on sendRequestToFFIThread: a handler running on the FFI thread -# that re-dispatches through it would enqueue work onto the very queue its blocked -# dispatcher can never drain, so the guard returns an Err immediately. - +# Reentrancy guard: a handler re-dispatching gets an Err, not a deadlock. var gReentrantNestedRes: Channel[string] gReentrantNestedRes.open() -# Handler runs on the FFI thread; it nests a send back into the same ctx and -# reports the outcome through gReentrantNestedRes. Carrying the ctx address -# via the request payload sidesteps the cross-thread visibility issue of -# thread-local pointers. registerReqFFI(ReentrantTriggerReq, lib: ptr TestLib): proc(ctxAddr: int): Future[Result[string, string]] {.async.} = let ctx = cast[ptr FFIContext[TestLib]](cast[uint](ctxAddr)) @@ -607,8 +573,6 @@ suite "reentrancy guard (PR #23 review, item 6)": ) .isOk() - # The outer callback only fires once the handler — including its nested - # send attempt — has finished. No polling/sleep needed. waitCallback(d) check d.retCode == RET_OK check cborDecode(callbackBytes(d), string).value == "guard-fired" @@ -617,10 +581,7 @@ suite "reentrancy guard (PR #23 review, item 6)": check nestedMsg.startsWith("err:") check "reentrant ffi call" in nestedMsg -# Non-terminal RET_STALE_WARN progress signal: while a handler runs, the caller -# is pinged every `ctx.staleWarnInterval` with the elapsed time, then still gets -# exactly one terminal RET_OK/RET_ERR. nim-ffi never times the handler out. - +# RET_STALE_WARN pings every ctx.staleWarnInterval, then one terminal result. type StaleConfig {.ffi.} = object dummy: int @@ -631,7 +592,6 @@ proc testlib_slow_stale*( return ok("slow-stale-done") proc createSimpleCtx(): ptr FFIContext[SimpleLib] = - ## Spins up a SimpleLib context via the ctor and returns it (nil on failure). var ctorD: CallbackData initCallbackData(ctorD) defer: @@ -649,8 +609,7 @@ proc createSimpleCtx(): ptr FFIContext[SimpleLib] = return nil cast[ptr FFIContext[SimpleLib]](ctxAddr) -## Callback that keeps the non-terminal stale pings apart from the one terminal -## answer, so a test can assert on both. +## Keeps stale pings apart from the one terminal answer so a test can assert both. type StaleData = object lock: Lock cond: Cond @@ -703,7 +662,6 @@ suite "non-terminal RET_STALE_WARN progress signal": defer: check SimpleLibFFIPool.destroyFFIContext(ctx).isOk() - # Tune the cadence down so the 350 ms handler trips several pings quickly. ctx.staleWarnInterval = 80.milliseconds var d: StaleData @@ -719,16 +677,12 @@ suite "non-terminal RET_STALE_WARN progress signal": waitTerminal(d) - # A 350 ms handler at an 80 ms cadence trips at least a couple of pings, and - # the k-th ping reports exactly k*80 ms of elapsed time. check d.staleCount >= 2 check parseInt(d.lastElapsed) == d.staleCount * 80 - # Exactly one terminal answer carrying the handler's real result. check d.terminalRet == RET_OK check cborDecode(d.terminalBytes, string).value == "slow-stale-done" - # Nothing trails the terminal callback. let staleAtTerminal = d.staleCount os.sleep(200) check d.staleCount == staleAtTerminal diff --git a/tests/unit/test_ffi_handle.nim b/tests/unit/test_ffi_handle.nim index 372731e..3859bc0 100644 --- a/tests/unit/test_ffi_handle.nim +++ b/tests/unit/test_ffi_handle.nim @@ -1,6 +1,4 @@ -## {.ffiHandle.} round-trip through the full C-shape dispatch path: a handle -## returned by one `.ffi.` proc crosses as a uint64 and is reconstituted as the -## live object by another; stale/forged/null ids miss cleanly with RET_ERR. +## {.ffiHandle.} round-trip: a handle crosses as uint64; stale/forged/null ids RET_ERR. import std/[locks, strutils] import unittest2 @@ -10,7 +8,7 @@ import ffi type HandleLib = object base: int -# Stub the dylib NimMain importc that declareLibrary emits (this links as a plain exe). +# Stub the importc NimMain declareLibrary emits (plain-exe link). {.emit: "void libhandletestNimMain(void) {}".} declareLibrary("handletest", HandleLib) @@ -33,7 +31,7 @@ proc handletest_token*( s.hits.inc() return ok(s.token & "#" & $s.hits) -# Handle as the receiver (first param) — lib type comes from declareLibrary. +# Handle as the receiver (first param). proc handletest_session_bump*(s: Session): Future[Result[int, string]] {.ffi.} = s.hits.inc() return ok(s.hits) @@ -93,7 +91,6 @@ proc encodedPtr(b: var seq[byte]): ptr byte = cast[ptr byte](addr b[0]) template runCall(d, ctx, reqBytes, exportProc) = - ## Fires `exportProc` with `reqBytes` and blocks until the callback lands in `d`. initCallbackData(d) var rb = reqBytes check exportProc(ctx, testCallback, addr d, encodedPtr(rb), rb.len.csize_t) == RET_OK @@ -118,7 +115,7 @@ suite "{.ffiHandle.} round-trip": deinitCallbackData(od) check od.retCode == RET_OK let handle = cborDecode(payload(od), uint64).value - check handle == 1'u64 # ids start at 1 + check handle == 1'u64 var td: CallbackData runCall(td, ctx, cborEncode(HandletestTokenReq(s: handle)), handletest_token) diff --git a/tests/unit/test_ffi_teardown.nim b/tests/unit/test_ffi_teardown.nim index 7778791..a17a83d 100644 --- a/tests/unit/test_ffi_teardown.nim +++ b/tests/unit/test_ffi_teardown.nim @@ -3,15 +3,11 @@ import unittest2 import results import ffi -# Exercises the async `{.ffiDtor.}` teardown hook: a non-empty dtor body is -# lifted into an `ffiTeardown` overload the FFI thread awaits on its own event -# loop right before it exits. `destroyFFIContext` must block until that -# teardown finishes. +# Exercises async {.ffiDtor.}: destroyFFIContext must block until teardown finishes. type TeardownLib = object -# declareLibrary emits an importc `libNimMain`; this test links as a plain -# exe, so stub it (mirrors test_ffi_context.nim). +# Stub the importc NimMain declareLibrary emits (plain-exe link). {.emit: "void libteardownlibNimMain(void) {}".} declareLibrary("teardownlib", TeardownLib) @@ -28,9 +24,7 @@ proc teardownlib_create*( return ok(TeardownLib()) proc teardownlib_destroy*(lib: TeardownLib): Future[void] {.ffiDtor.} = - ## Async teardown: sleeps on the FFI event loop, then records that it ran and - ## on which thread. If destroy didn't wait, `gTeardownRan` would still be false - ## when the test checks it. + ## Async teardown: sleeps, then records that it ran and on which thread. await sleepAsync(200.milliseconds) gTeardownThreadId.store(getThreadId()) gTeardownRan.store(true) @@ -47,8 +41,7 @@ proc encodedPtr(bytes: var seq[byte]): ptr byte = cast[ptr byte](addr bytes[0]) proc createCtxWithLib(): ptr FFIContext[TeardownLib] = - ## Spins up a context via the ctor and waits until `myLib` is populated (set on - ## the FFI thread when the ctor request is dispatched), so teardown has a lib. + ## Spins up a context via the ctor and waits until `myLib` is populated. var cfg = cborEncode(TeardownlibCreateCtorReq(config: NoopConfig(dummy: 0))) let ret = teardownlib_create(encodedPtr(cfg), cfg.len.csize_t, noopCallback, nil) if ret.isNil(): @@ -72,9 +65,7 @@ suite "async {.ffiDtor.} teardown hook": check TeardownlibFFIPool.destroyFFIContext(ctx).isOk() - # If destroy returned before awaiting the teardown, this would be false. check gTeardownRan.load() - # And it must have run on the FFI thread, not the caller's. check gTeardownThreadId.load() != 0 check gTeardownThreadId.load() != callerTid diff --git a/tests/unit/test_gc_compat.nim b/tests/unit/test_gc_compat.nim index d04962d..3987391 100644 --- a/tests/unit/test_gc_compat.nim +++ b/tests/unit/test_gc_compat.nim @@ -1,9 +1,4 @@ -## Verifies correct behaviour under both --mm:orc and --mm:refc. -## -## The foreignThreadGc template must set up / tear down the GC for foreign -## threads under refc and be a no-op under orc/arc. The handleRes proc -## holds a string local long enough for the callback to read its cstring -## -- these tests catch regressions in that lifetime guarantee. +## foreignThreadGc + string-lifetime guarantees under both orc and refc. import std/locks import unittest2 @@ -62,9 +57,7 @@ proc callbackBytes(d: var CallbackData): seq[byte] = return bytes proc callbackOkString(d: var CallbackData): string = - ## Decodes the CBOR success payload as a string. Asserts the request - ## actually succeeded — silently treating an error payload as the empty - ## string would let a regression slip past the test that calls us. + ## Decodes the CBOR success payload as a string, asserting the request succeeded. doAssert d.retCode == RET_OK, "callbackOkString called on non-OK retCode " & $d.retCode & " (msg=" & callbackMsg( d @@ -72,16 +65,13 @@ proc callbackOkString(d: var CallbackData): string = cborDecode(callbackBytes(d), string).valueOr: return "" -# Concatenates GC-allocated strings so the result is not a string literal; -# exercises the resStr lifetime binding inside handleRes. +# Non-literal result exercises the resStr lifetime binding in handleRes. registerReqFFI(StringLifetimeRequest, lib: ptr GcTestLib): proc(input: cstring): Future[Result[string, string]] {.async.} = let prefix = "lifetime:" let suffix = $input return ok(prefix & suffix) -# Returns 512 bytes of repeating a-z to stress GC with a moderately large -# allocation that must survive the cross-thread callback. registerReqFFI(LargeStringRequest, lib: ptr GcTestLib): proc(): Future[Result[string, string]] {.async.} = var s = newString(512) @@ -89,7 +79,6 @@ registerReqFFI(LargeStringRequest, lib: ptr GcTestLib): s[i] = char(ord('a') + (i mod 26)) return ok(s) -# Error path: the error string must be alive when the callback fires. registerReqFFI(GcErrRequest, lib: ptr GcTestLib): proc(input: cstring): Future[Result[string, string]] {.async.} = return err("gc-err:" & $input) @@ -149,14 +138,9 @@ suite "GC safety - string lifetime across thread boundary": .isOk() waitCallback(d) check d.retCode == RET_ERR - # Error payloads are raw UTF-8, not CBOR. check callbackMsg(d) == "gc-err:test" test "large string result is delivered without corruption": - # Round-trip check: build the same 512-char string the FFI handler is - # specified to produce, run the request through the FFI thread (which - # CBOR-encodes the result), decode the callback payload, and assert - # the decoded string is byte-for-byte identical to the original. var expected = newString(512) for i in 0 ..< 512: expected[i] = char(ord('a') + (i mod 26)) diff --git a/tests/unit/test_main_prefix_validation.nim b/tests/unit/test_main_prefix_validation.nim index d8286f7..cc544a6 100644 --- a/tests/unit/test_main_prefix_validation.nim +++ b/tests/unit/test_main_prefix_validation.nim @@ -1,10 +1,7 @@ import std/[os, strutils] import unittest2 -# The validation only fires when --nimMainPrefix is on the command line, so we -# capture a real `nim check` of the fixture at this test's compile time. -# `mainprefix_fixture.nim` deliberately lacks the `test_` prefix so the nimble -# runner never compiles it standalone. +# Captures a real `nim check` of the fixture at compile time, with/without --nimMainPrefix. const nimExe = getCurrentCompilerExe() fixture = currentSourcePath.parentDir / "mainprefix_fixture.nim" @@ -15,8 +12,7 @@ const suite "compile-time --nimMainPrefix validation": test "a mismatched prefix errors and names the expected flag": check "Error:" in wrongPrefixOutput - # naming the expected flag is what distinguishes our error from any other - # compile failure, so assert on it rather than the bare "Error:". + # Assert on the flag name to distinguish our error from any other compile failure. check "needs --nimMainPrefix:libmpfixture" in wrongPrefixOutput test "the matching prefix compiles without error": diff --git a/tests/unit/test_meta.nim b/tests/unit/test_meta.nim index 8451416..c71c1cf 100644 --- a/tests/unit/test_meta.nim +++ b/tests/unit/test_meta.nim @@ -1,17 +1,12 @@ ## Unit tests for the AST helpers used by the FFI macro. -## The identifier-casing helpers used to live here too; they now have their -## own module and test file (`test_string_helpers.nim`). import unittest import std/[macros, strutils] import ffi/internal/ffi_macro suite "unpackReqField": - ## `unpackReqField` builds AST via `std/macros` helpers (`ident`, `newDotExpr`, - ## `newLetStmt`, etc.) which are compile-time magics. The tests therefore run - ## as `static:` blocks — a failed `doAssert` becomes a compile-time error, so - ## a broken helper aborts the build before the test binary is produced. - ## Whitespace in AST repr is normalised so the assertions are layout-stable. + ## Runs in `static:` blocks (a failed doAssert becomes a compile error). + ## AST repr whitespace is normalised so assertions are layout-stable. proc normalise(s: string): string {.compileTime.} = var buf = "" var prevSpace = true @@ -43,8 +38,7 @@ suite "unpackReqField": doAssert ".cstring" notin r test "non-cstring complex type passes through unchanged": - # Generic / bracket / dot expressions are not nnkIdent, so the cstring - # branch must not fire even if the type's textual repr contains "cstring". + # Non-nnkIdent types must not trigger the cstring branch. static: let userType = nnkBracketExpr.newTree(ident("seq"), ident("int")) let node = unpackReqField(ident("xs"), userType, ident("decoded")) diff --git a/tests/unit/test_nim_native_api.nim b/tests/unit/test_nim_native_api.nim index cd1194b..55c9496 100644 --- a/tests/unit/test_nim_native_api.nim +++ b/tests/unit/test_nim_native_api.nim @@ -1,8 +1,6 @@ -## Demonstrates the Nim-native side of the {.ffi.} / {.ffiCtor.} macros: -## every annotated proc remains callable from Nim with its declared signature -## (`Future[Result[T, string]]`), no callbacks or CBOR buffers involved. The -## C-exported wrapper exists in parallel as an overload distinguishable by -## arity — see `test_ffi_context.nim` for the C-shape callers. +## Nim-native side of {.ffi.} / {.ffiCtor.}: every annotated proc stays callable +## from Nim with its declared `Future[Result[T, string]]` signature, no callbacks +## or CBOR. The C-exported wrapper lives in parallel (see test_ffi_context.nim). import std/options import unittest2 @@ -12,7 +10,7 @@ import ffi type Counter = object start: int -# Stub the dylib NimMain importc that declareLibrary emits (this links as a plain exe). +# Stub the dylib NimMain importc that declareLibrary emits (links as an exe). {.emit: "void libcounterlibNimMain(void) {}".} declareLibrary("counterlib", Counter) @@ -32,8 +30,7 @@ proc counter_create*(cfg: CounterConfig): Future[Result[Counter, string]] {.ffiC return ok(Counter(start: cfg.initial)) proc counter_value*(c: Counter): Future[Result[CounterState, string]] {.ffi.} = - ## Sync body (no `await`); the Nim-facing wrapper still returns - ## Future[Result[...]] so the source-level shape is preserved. + ## Sync body (no `await`); the wrapper still returns Future[Result[...]]. return ok(CounterState(value: c.start)) proc counter_add*( @@ -61,10 +58,7 @@ proc counter_fail*(c: Counter, reason: string): Future[Result[string, string]] { proc counter_chain*( c: Counter, steps: int ): Future[Result[CounterState, string]] {.ffi.} = - ## Real async work: multiple awaits composing other {.ffi.} procs. - ## Shows that the Nim-facing wrapper for an {.ffi.} proc is itself - ## awaitable, so {.ffi.} procs can be composed naturally without ever - ## touching the C-export shape. + ## Multiple awaits composing other {.ffi.} procs — the wrapper is awaitable. var current = c for i in 0 ..< steps: await sleepAsync(1.milliseconds) @@ -94,9 +88,7 @@ type QueryReport {.ffi.} = object proc counter_query*( c: Counter, filter: RangeFilter, page: Pagination, projection: Projection ): Future[Result[QueryReport, string]] {.ffi.} = - ## Three independent object-typed parameters: `filter`, `page`, `projection`. - ## Verifies that the macro packs all three into one CBOR Req envelope on the - ## wire and unpacks them back into the typed locals before this body runs. + ## Three object-typed params packed into one CBOR Req envelope and unpacked back. if filter.hi < filter.lo: return err("filter range is empty") if page.limit <= 0: diff --git a/tests/unit/test_rust_codegen.nim b/tests/unit/test_rust_codegen.nim index 5f80f00..e01c8e8 100644 --- a/tests/unit/test_rust_codegen.nim +++ b/tests/unit/test_rust_codegen.nim @@ -1,8 +1,5 @@ -## Regression tests for the Rust type mapping. `nimTypeToRust` used to carry its -## own scalar table that had drifted from C/C++ — `int8`/`int16`/`uint8`/ -## `uint16`/`uint32`/`byte`/`float32` fell through to `capitalizeFirstLetter` -## and emitted invalid Rust. It now renders through the shared `parseFFIType` -## intermediate representation, so the full scalar set is pinned here. +## Regression tests for the Rust type mapping: `nimTypeToRust` renders through +## the shared `parseFFIType` IR, so the full scalar set is pinned here. import unittest2 import ffi/codegen/rust diff --git a/tests/unit/test_scalar_fastpath.nim b/tests/unit/test_scalar_fastpath.nim index 8a4a47c..7132abd 100644 --- a/tests/unit/test_scalar_fastpath.nim +++ b/tests/unit/test_scalar_fastpath.nim @@ -1,11 +1,6 @@ -## Exercises the CBOR-free scalar fast path (`{.ffi: "abi = c".}` on an -## all-scalar signature). A scalar proc's C export takes its scalar args -## directly (no `reqCbor`/`reqCborLen`), packs them inline into the request, -## and the response rides back as raw bytes — no CBOR envelope either way. -## -## Two angles are covered: the C-export shape (ctx, callback, userData, args…) -## driven through a real FFI thread, and the Nim-native shape (the user proc -## name still resolves to its declared `Future[Result[T, string]]`). +## Exercises the CBOR-free scalar fast path (`{.ffi: "abi = c".}` all-scalar): +## scalar args packed inline, response rides back as raw bytes. Covers both the +## C-export shape (through a real FFI thread) and the Nim-native shape. import std/[locks, strutils, sequtils] import unittest2 @@ -17,7 +12,7 @@ import ffi/internal/ffi_scalar type ScalarLib = object base: int -# Stub the dylib NimMain importc that declareLibrary emits (this links as an exe). +# Stub the dylib NimMain importc that declareLibrary emits (links as an exe). {.emit: "void libscalarfastNimMain(void) {}".} declareLibrary("scalarfast", ScalarLib) @@ -33,13 +28,12 @@ proc scalarfast_create*( proc scalarfast_add*( lib: ScalarLib, a: int, b: int ): Future[Result[int, string]] {.ffi: "abi = c".} = - ## Two scalar params, scalar return — the flagship fast-path shape. return ok(lib.base + a + b) proc scalarfast_version*( lib: ScalarLib ): Future[Result[string, string]] {.ffi: "abi = c".} = - ## No params, string return (rides back as raw UTF-8, no CBOR). + ## No params; string return rides back as raw UTF-8. return ok("scalarfast v1") proc scalarfast_scale*( @@ -64,8 +58,7 @@ proc scalarfast_checked*( proc scalarfast_blank*( lib: ScalarLib ): Future[Result[string, string]] {.ffi: "abi = c".} = - ## Empty-string return — must ride back as a genuine 0-length RET_OK payload, - ## not the CBOR-null sentinel. + ## Empty-string return rides back as a 0-length RET_OK payload, not the sentinel. return ok("") ## C-shape callback harness (mirrors test_ffi_context.nim). @@ -274,9 +267,7 @@ suite "scalar fast path — Nim-native shape": check blank.isOk() check blank.value == "" -# `ffiProcRegistry` is a compile-time var, so its assertions run in a static -# block (mirrors test_abi_format.nim). A scalar-only `abi = c` proc must be -# flagged, recognised by `isScalarOnly`, and dropped from `bindableProcs`. +# Compile-time checks: a scalar-only `abi = c` proc is flagged, isScalarOnly, and dropped from bindableProcs. static: const scalarNames = [ "scalarfast_add", "scalarfast_version", "scalarfast_scale", "scalarfast_positive", diff --git a/tests/unit/test_scalar_skip_gen.nim b/tests/unit/test_scalar_skip_gen.nim index 8017b14..156608f 100644 --- a/tests/unit/test_scalar_skip_gen.nim +++ b/tests/unit/test_scalar_skip_gen.nim @@ -1,11 +1,6 @@ -## Drives the scalar-fast-path drop error end to end: compiles -## `fixtures/scalar_skip_fixture.nim` (a library with an all-scalar `abi = c` -## proc) with `-d:ffiGenBindings` and asserts genBindings() fails loudly, and -## that `-d:ffiAllowScalarSkip` downgrades the drop to a clean build. -## -## The fixture is compiled in a child `nim check` (search paths and compiler -## captured at compile time) so its expected failure is observed as a test -## assertion, not this file's own compile error. +## Compiles fixtures/scalar_skip_fixture.nim (all-scalar `abi = c`) in a child +## `nim check` under `-d:ffiGenBindings`, asserting genBindings() fails loudly +## and that `-d:ffiAllowScalarSkip` downgrades the drop to a clean build. import std/[os, osproc, strutils, compilesettings] import unittest2 diff --git a/tests/unit/test_serial.nim b/tests/unit/test_serial.nim index b3e1476..02ce3e5 100644 --- a/tests/unit/test_serial.nim +++ b/tests/unit/test_serial.nim @@ -114,10 +114,7 @@ suite "CBOR object": check back.value.point.y == 2 suite "CBOR ref T (value-copy contract)": - ## cbor_serialization's default `ref T` writer dereferences and encodes the - ## pointee. On decode the receiving side allocates a fresh `ref` local to - ## its own GC heap — no address crosses the boundary and the two refs are - ## independent. Documented in ffi/cbor_serial.nim's module header. + ## Encode dereferences the pointee; decode allocates an independent ref. test "ref RefBox round-trip produces an independent ref": let original = (ref RefBox)(label: "hi", n: 7) @@ -127,9 +124,7 @@ suite "CBOR ref T (value-copy contract)": check back.value != nil check back.value.label == "hi" check back.value.n == 7 - # Mutate the decoded copy; the original must be untouched (proving no - # aliasing). If the wire format ever switched to identity-preserving - # transport, this would fail. + # Mutating the decoded copy must not touch the original (no aliasing). back.value.label = "mutated" check original.label == "hi" check cast[pointer](back.value) != cast[pointer](original) @@ -201,10 +196,7 @@ suite "CBOR error handling": let res = cborDecode(truncated, string) check res.isErr -# Regression for PR #23 review item 9: cborEncodeShared writes directly into -# a c_malloc buffer, letting the FFI thread request take ownership without -# an intermediate seq[byte] copy. The shared-encoder must produce -# byte-for-byte the same output as the seq-encoder. +# cborEncodeShared writes into a c_malloc buffer; must match the seq-encoder byte-for-byte. import system/ansi_c @@ -237,8 +229,7 @@ suite "cborEncodeShared": check back.point.y == 4 test "large string growth": - # Larger than the initial 16-byte cap so the encoder must grow several - # times; verifies the shared-mode grower handles repeated reallocations. + # Larger than the initial 16-byte cap so the shared-mode grower reallocates. var big = newString(4096) for i in 0 ..< big.len: big[i] = char(ord('a') + (i mod 26)) @@ -345,9 +336,7 @@ suite "CBOR boundaries": check back.value.classify == math.fcSubnormal test "float32 subnormal (compared by bit pattern)": - # The smallest positive float32 subnormal has bit pattern 0x00000001. - # `math.classify` widens to float64 and would re-classify this as normal, - # so we compare the raw 32-bit bit pattern instead. + # classify widens to float64 and misreads this as normal; compare raw bits. let v = cast[float32](0x00000001'u32) let bytes = cborEncode(v) let back = cborDecode(bytes, float32) @@ -493,8 +482,7 @@ suite "CBOR round-trips": check back.value.flags.len == 0 check back.value.blob.len == 0 - # Sizes chosen to cross the 24/256/65536-byte CBOR length-encoding - # boundaries and the encoder's internal buffer-grow thresholds. + # Sizes cross the 24/256/65536-byte CBOR length and buffer-grow boundaries. test "string >64 KiB": const n = 70_000 @@ -556,10 +544,7 @@ suite "CBOR round-trips": check back.value[n - 1].y == -(n - 1) suite "CBOR void / null sentinel": - ## `CborNullByte` is the wire sentinel used by `ffi_thread_request` to - ## carry a "successful but no value" reply (an `.ffi.` proc whose handler - ## returns `Result[void, string]`). See `ffi/cbor_serial.nim` and - ## `ffi/ffi_thread_request.nim` for the producer side. + ## `CborNullByte` is the "successful but no value" (Result[void]) wire sentinel. test "CborNullByte equals CBOR null (0xf6)": check CborNullByte == 0xf6'u8 @@ -576,8 +561,7 @@ suite "CBOR void / null sentinel": check back.value.isNone test "decoding the sentinel as a required object type errors out": - # A consumer that expects a real payload but is handed the void sentinel - # must fail explicitly rather than silently materializing a zeroed value. + # A required-payload consumer handed the void sentinel must fail explicitly. let bytes = @[CborNullByte] let back = cborDecode(bytes, Point) check back.isErr diff --git a/tests/unit/test_string_helpers.nim b/tests/unit/test_string_helpers.nim index 00bb913..a3f318a 100644 --- a/tests/unit/test_string_helpers.nim +++ b/tests/unit/test_string_helpers.nim @@ -1,7 +1,4 @@ ## Unit tests for the identifier-casing helpers used by the codegen. -## These names map identifier conventions between Nim (camelCase), -## Rust (snake_case) and C++ (PascalCase types), and they're load-bearing -## for binding generation, so it's worth pinning their behaviour with tests. import unittest import ffi/codegen/string_helpers @@ -82,6 +79,5 @@ suite "snakeToPascalCase": check snakeToPascalCase("_foo") == "Foo" test "already-mixed parts preserve their existing case after the first": - # split on '_', capitalize first letter of each part; "HasCaps" first - # letter is already 'H' so it's untouched. + # Existing caps after the first letter of each part are preserved. check snakeToPascalCase("already_HasCaps") == "AlreadyHasCaps" diff --git a/tests/unit/test_types_ir.nim b/tests/unit/test_types_ir.nim index 319d1d2..9c3fa35 100644 --- a/tests/unit/test_types_ir.nim +++ b/tests/unit/test_types_ir.nim @@ -1,7 +1,4 @@ -## Unit tests for the shared type intermediate representation that the C / C++ / -## Rust binding generators parse Nim type strings through. `parseFFIType` is the -## single source of truth the three backends consume, so its shape mappings are -## pinned here directly. +## Unit tests for `parseFFIType`, the shared type IR the C / C++ / Rust backends consume. import unittest2 import ffi/codegen/types_ir diff --git a/tests/unit/test_wire_compat.nim b/tests/unit/test_wire_compat.nim index e6d2f54..9d58cc6 100644 --- a/tests/unit/test_wire_compat.nim +++ b/tests/unit/test_wire_compat.nim @@ -1,18 +1,4 @@ -## Wire-format compatibility tests. -## -## The C++ side now uses vendored TinyCBOR (see -## `ffi/codegen/templates/cpp/vendor/tinycbor/`) and the Nim side uses -## `cbor_serialization`. Both implement RFC 8949, but a regression on either -## side could silently produce divergent bytes for the same logical value. -## -## These tests pin the *exact* byte sequences `cbor_serialization` emits for -## a handful of representative shapes. If a future bump to the Nim library -## ever shifts the encoding (e.g., key ordering, integer length choice, -## optional/null handling), the assertions here will fail loudly before the -## C++ side gets to discover the divergence at runtime. -## -## The same golden bytes are exercised on the C++ side by the timer -## example's end-to-end round-trip (`examples/timer/cpp_bindings/main.cpp`). +## Pins the exact CBOR bytes Nim emits so it can't silently diverge from C++ TinyCBOR. import std/[options, strutils] import unittest @@ -46,7 +32,6 @@ suite "wire format — single-field map": test "WireSimple{name:\"abc\"} round-trips to a stable byte sequence": let v = WireSimple(name: "abc") let bytes = cborEncode(v) - # map(1), key "name" (text-string len 4), value "abc" (text-string len 3) check toHex(bytes) == "a1646e616d6563616263" let back = cborDecode(bytes, WireSimple) check back.isOk @@ -56,7 +41,6 @@ suite "wire format — int field": test "WireWithInt encodes ints as CBOR integers": let v = WireWithInt(message: "hi", delayMs: 200) let bytes = cborEncode(v) - # map(2), "message"->"hi", "delayMs"->200 (uint8 form: 0x18 0xc8) check toHex(bytes) == "a2676d65737361676562686967" & "64656c61794d7318c8" let back = cborDecode(bytes, WireWithInt) check back.isOk @@ -66,62 +50,37 @@ suite "wire format — int field": test "negative int uses CBOR negative-integer major type": let v = WireWithInt(message: "x", delayMs: -1) let bytes = cborEncode(v) - # 0x20 is CBOR -1 check toHex(bytes).endsWith("20") suite "wire format — Option[T]": - ## Nim's `cbor_serialization/std/options` import encodes `Option[T]`: - ## - `some v` → emit the key and the inner value. - ## - `none T` → **omit the field entirely** from the map (the resulting - ## map is smaller by one entry). - ## - ## The C++ TinyCBOR helper currently encodes `std::nullopt` as CBOR null - ## (0xf6). That divergence is invisible while no consumer sends - ## `std::nullopt` over the wire (the timer example only sends `Some` - ## values). If a future caller does, we'll need to align the conventions - ## — either teach the C++ codec to skip None-valued keys (mirroring Nim), - ## or switch the Nim side to emit explicit nulls. This test pins the - ## current Nim behavior so the divergence is detectable instead of - ## silent. - + ## Nim emits `some v` as key+value and omits the key entirely for `none`. test "Option.some encodes as the inner value (no wrapper)": let v = WireWithOption(label: "x", note: some("hi")) let bytes = cborEncode(v) - # map(2): "label"->"x", "note"->"hi" (text strings, no null/tag wrapping) check toHex(bytes) == "a2656c6162656c6178646e6f7465626869" test "Option.none yields a smaller map without the optional key": let v = WireWithOption(label: "x", note: none(string)) let bytes = cborEncode(v) - # map(1): only "label"->"x"; the "note" key is absent. check toHex(bytes) == "a1656c6162656c6178" suite "wire format — seq[T]": test "empty seq encodes as CBOR array(0)": let v = WireWithVector(items: @[]) let bytes = cborEncode(v) - # a1 (map 1) 65 (text-str len 5) 69 74 65 6d 73 ("items") 80 (array 0) check toHex(bytes) == "a1656974656d7380" test "three-element seq[string]": let v = WireWithVector(items: @["a", "bb", "ccc"]) let bytes = cborEncode(v) - # map(1), "items" -> array(3) of text strings "a", "bb", "ccc": - # 83 (array 3) 61 61 ("a") 62 62 62 ("bb") 63 63 63 63 ("ccc") check toHex(bytes) == "a1656974656d7383616162626263636363" suite "wire format — seq[byte]": - ## `cbor_serialization` emits `seq[byte]` as a CBOR **byte string** (major - ## type 2), not an array (major type 4). The C++ codegen mirrors this with a - ## `std::vector` overload that uses `cbor_encode_byte_string`. - ## These goldens pin the cross-language contract. - + ## `seq[byte]` rides as a CBOR byte string (major type 2), never an array. test "seq[byte] field rides as a CBOR byte string, not an array": let v = WireWithBytes(blob: @[1'u8, 2'u8, 3'u8]) let bytes = cborEncode(v) - # map(1): "blob" -> byte-string len 3 (0x43) 01 02 03 check toHex(bytes) == "a164626c6f6243010203" - # The value is a byte string (0x40–0x5b), never an array (0x80–0x9b). let valMajor = bytes[6] check valMajor >= 0x40'u8 check valMajor <= 0x5b'u8 @@ -132,5 +91,4 @@ suite "wire format — seq[byte]": test "empty seq[byte] field rides as byte-string(0)": let v = WireWithBytes(blob: @[]) let bytes = cborEncode(v) - # map(1): "blob" -> byte-string len 0 (0x40) check toHex(bytes) == "a164626c6f6240"