From 3b6f8fc91e9538be2be90289805c0cc2066e8904 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz Date: Wed, 29 Jul 2026 10:28:23 -0300 Subject: [PATCH] feat: fold static, event, export, dtor into ffi --- CHANGELOG.md | 24 +++ ffi/codegen/rust.nim | 8 +- ffi/ffi_context.nim | 5 +- ffi/ffi_events.nim | 4 +- ffi/internal/ffi_export.nim | 80 +++++---- ffi/internal/ffi_library.nim | 8 +- ffi/internal/ffi_macro.nim | 117 +++++++++---- ffi/internal/ffi_route.nim | 99 +++++++++++ ffi/internal/ffi_scalar.nim | 9 +- .../router_dtor_wrong_shape_fixture.nim | 13 ++ .../router_event_wrong_shape_fixture.nim | 13 ++ .../router_export_wrong_shape_fixture.nim | 13 ++ tests/unit/test_ctx_after_failed_ctor.nim | 37 ++-- tests/unit/test_ffi_context.nim | 8 +- tests/unit/test_ffi_router.nim | 161 ++++++++++++++++++ tests/unit/test_ffi_router_reject.nim | 39 +++++ tests/unit/test_rust_codegen.nim | 9 +- tests/unit/test_wire_compat.nim | 8 +- 18 files changed, 541 insertions(+), 114 deletions(-) create mode 100644 ffi/internal/ffi_route.nim create mode 100644 tests/unit/fixtures/router_dtor_wrong_shape_fixture.nim create mode 100644 tests/unit/fixtures/router_event_wrong_shape_fixture.nim create mode 100644 tests/unit/fixtures/router_export_wrong_shape_fixture.nim create mode 100644 tests/unit/test_ffi_router.nim create mode 100644 tests/unit/test_ffi_router_reject.nim diff --git a/CHANGELOG.md b/CHANGELOG.md index 178a119..eca2b7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,30 @@ All notable changes to this project are documented in this file. longer need a hand-written payload type. A single parameter still rides the wire directly (a scalar, or an existing `{.ffi.}` object). The foreign bindings gain the envelope as a first-class struct plus a typed handler. +- `{.ffiExport.}`, for simple synchronous C exports, from the 0.2 line. +- `{.ffi.}` now picks the path from the shape of the signature. One pragma + covers the context method, the static call, the synchronous export, the + destructor and the event. `ffi/internal/ffi_route.nim` holds the rules: + + | Shape | Path | + |---|---| + | The first parameter is the library type or an `{.ffiHandle.}` type | Context method | + | No receiver, and the result is `Future[Result[T, string]]` | Static call | + | No parameters, and the result is a plain Nim type | Synchronous export | + | A library receiver, and no result or `Future[void]` | Destructor | + | A payload parameter, and no result | Event | + + Every shape that the router claims failed to compile under any other pragma + before, so the router only turns a compile error into the intended meaning. + + `{.ffiStatic.}`, `{.ffiExport.}`, `{.ffiDtor.}` and `{.ffiEvent.}` still work. + Each one now asserts its shape and names the right pragma when the shape does + not match. + + `{.ffiCtor.}` stays explicit, because its shape is not free. A ctor differs + from a static call by one token: the type inside `Result`. A static call that + returns the library type builds today and exports a working C symbol, so a + router would silently give it the ctor ABI instead. ### Fixed - A `{.ffi.}` call against a `ref` library type whose `{.ffiCtor.}` never stored diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 7e41a8f..613e19f 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -30,8 +30,8 @@ func rustOpt(elem: string): string = const rustMap = NativeTypeMap( scalar: rustScalar, str: "String", - # serde encodes a plain Vec as a CBOR integer array, which Nim rejects. - # ByteBuf gives the CBOR byte string that Nim decodes. + # serde encodes a plain Vec as a CBOR integer array, and Nim rejects that + # array. ByteBuf gives the CBOR byte string that Nim decodes. bytes: "serde_bytes::ByteBuf", ptrType: RustPtrType, seqOf: rustSeq, @@ -71,7 +71,7 @@ proc reqStructName(p: FFIProcMeta): string = camel & "Req" func typeUsesBytes(typeName: string): bool = - ## True if `typeName` resolves to a `seq[byte]` at any depth of Seq or Option. + ## True if `typeName` is a `seq[byte]` at any depth of Seq or Option. var t = parseFFIType(typeName) while t.kind in {ftSeq, ftOpt}: t = t.elem @@ -79,7 +79,7 @@ func typeUsesBytes(typeName: string): bool = func needsSerdeBytes*(types: seq[FFITypeMeta], procs: seq[FFIProcMeta]): bool = ## True if a field, a parameter or a return type maps to `serde_bytes::ByteBuf`. - ## `types` holds every struct, thus a scan of the fields also finds the bytes + ## `types` holds every struct. Thus a scan of the fields also finds the bytes ## in a nested struct. for t in types: for f in t.fields: diff --git a/ffi/ffi_context.nim b/ffi/ffi_context.nim index 36d948f..740d736 100644 --- a/ffi/ffi_context.nim +++ b/ffi/ffi_context.nim @@ -40,8 +40,9 @@ type FFIContext*[T] = object # fired by the recycle handler once the lib is freed and the slot released; # the synchronous recycleFFIContext caller waits on it. libReady*: Atomic[bool] - # False until a {.ffiCtor.} stores the library; until then `myLib` is the - # FFI thread's default-valued fallback, which for a `ref` type is nil. + # False until a {.ffiCtor.} stores the library. Before that, `myLib` points + # at the default fallback of the FFI thread. For a `ref` type that fallback + # is nil. ffiThread: Thread[(ptr FFIContext[T])] eventThread: Thread[(ptr FFIContext[T])] reqQueueBank: RequestQueueBank diff --git a/ffi/ffi_events.nim b/ffi/ffi_events.nim index 717cdbd..f5c722e 100644 --- a/ffi/ffi_events.nim +++ b/ffi/ffi_events.nim @@ -36,8 +36,8 @@ proc deinitEventRegistry*(reg: var FFIEventRegistry) = reg.nextId = 0'u64 proc clearListeners*(reg: var FFIEventRegistry) {.raises: [].} = - ## Drops all listeners (used when a context is recycled for reuse) without - ## touching the lock — the event thread keeps using it across recycles. + ## Removes all listeners. The pool calls this when it recycles a context. The + ## lock stays in place, because the event thread uses it across recycles. withLock reg.lock: reg.byEvent.clear() reg.nextId = 0'u64 diff --git a/ffi/internal/ffi_export.nim b/ffi/internal/ffi_export.nim index ae4ee79..86bba8d 100644 --- a/ffi/internal/ffi_export.nim +++ b/ffi/internal/ffi_export.nim @@ -1,63 +1,64 @@ ## Simple synchronous C export for a nim-ffi library. ## -## `{.ffi.}` / `{.ffiCtor.}` are the async, context-handle, CBOR-marshaled path — -## the right tool for stateful, multi-call libraries. `{.ffiExport.}` covers the -## other common case: a handful of DEAD-SIMPLE lifecycle/health entry points a host -## loads with plain `dlopen` + `dlsym` and calls synchronously — no context, no -## callback, no CBOR. The function's own return value crosses the ABI directly. +## `{.ffi.}` and `{.ffiCtor.}` give the async path. That path uses a context +## handle and encodes the data with CBOR. It fits a library that keeps state +## across many calls. `{.ffiExport.}` covers the other common case: a few simple +## lifecycle entry points. The host loads them with `dlopen` and `dlsym`, then +## calls them synchronously. There is no context, no callback and no CBOR. The +## return value of the function crosses the ABI directly. ## -## You write NATIVE Nim types; `ffiExport` bridges to the C ABI for you: +## Write native Nim types. `ffiExport` maps them to the C ABI: ## int / bool -> C int ## uint64 -> C unsigned long long -## string -> C const char* (kept alive in shared memory until the next call) +## string -> C const char* (stays alive in shared memory until the next call) ## (no return) -> C void -## and it injects the library's `initializeLibrary()` bootstrap so the Nim runtime -## is up on first call — the host never invokes NimMain itself. +## `ffiExport` also injects the `initializeLibrary()` call of the library. The Nim +## runtime therefore starts on the first call, and the host never calls NimMain. ## ## declareLibraryBase("myLib") # emits initializeLibrary() ## proc my_start(): int {.ffiExport.} = 0 # -> int my_start(void) ## proc my_alive(): uint64 {.ffiExport.} = beats # -> unsigned long long my_alive(void) ## proc my_error(): string {.ffiExport.} = lastErr # -> const char* my_error(void) ## -## Build the shared library with `--noMain --nimMainPrefix:libmyLib`. Arguments are -## not supported (these are no-arg lifecycle calls); use `{.ffi.}` for calls that -## take arguments. +## Build the shared library with `--noMain --nimMainPrefix:libmyLib`. A proc with +## `{.ffiExport.}` takes no arguments. For a call with arguments, use `{.ffi.}`. import std/macros +import ./ffi_route proc cReturnType(t: NimNode): NimNode = - ## Native Nim return type -> the C-ABI type that actually crosses the boundary. + ## Maps the native Nim return type to the C ABI type that crosses the boundary. if t.kind == nnkEmpty: - return t # void + return t # void if t.kind == nnkIdent: case $t - of "int", "int32", "bool": return ident("cint") - of "uint", "uint64": return ident("culonglong") - of "string": return ident("cstring") - else: discard - return t # already a C-compatible type + of "int", "int32", "bool": + return ident("cint") + of "uint", "uint64": + return ident("culonglong") + of "string": + return ident("cstring") + else: + discard + return t # already a C-compatible type -macro ffiExport*(prc: untyped): untyped = - ## Mark a no-argument proc as a simple synchronous C export (see module doc): - ## native Nim return type, bridged to the C ABI, with the runtime bootstrapped. +proc buildFFIExportProc*(prc: NimNode): NimNode {.compileTime.} = + ## Emits the synchronous C export. `{.ffi.}` and `{.ffiExport.}` share it. prc.expectKind({nnkProcDef, nnkFuncDef}) - let nameNode = if prc.name.kind == nnkPostfix: prc.name[1] else: prc.name - let exportName = $nameNode + let exportName = $procIdent(prc) let params = prc.params - if params.len > 1: - error("ffiExport supports no-argument procs; use {.ffi.} for calls with arguments") - let nativeRet = params[0] let cRet = cReturnType(nativeRet) - # The user's body becomes a private impl proc; the exported wrapper converts. + # The user body becomes a private impl proc. The exported wrapper converts the + # result. let implName = genSym(nskProc, exportName & "Impl") var impl = copyNimTree(prc) - impl[0] = implName # rename - impl[4] = newEmptyNode() # drop pragmas (internal, not exported) + impl[0] = implName # rename + impl[4] = newEmptyNode() # remove the pragmas: this proc stays internal let wrapName = ident(exportName) - let boot = quote do: + let boot = quote: when declared(initializeLibrary): initializeLibrary() @@ -71,16 +72,20 @@ macro ffiExport*(prc: untyped): untyped = proc `wrapName`(): cstring {.exportc: `exportName`, cdecl, dynlib.} = `boot` let s = `implName`() - if `buf` != nil: deallocShared(`buf`) + if `buf` != nil: + deallocShared(`buf`) `buf` = allocShared(s.len + 1) - if s.len > 0: copyMem(`buf`, unsafeAddr s[0], s.len) + if s.len > 0: + copyMem(`buf`, unsafeAddr s[0], s.len) cast[ptr char](cast[uint](`buf`) + uint(s.len))[] = '\0' return cast[cstring](`buf`) + elif nativeRet.kind == nnkEmpty: res.add quote do: proc `wrapName`() {.exportc: `exportName`, cdecl, dynlib.} = `boot` `implName`() + else: # scalar: convert the native result to the C return type (cint / culonglong / …). res.add quote do: @@ -89,3 +94,12 @@ macro ffiExport*(prc: untyped): untyped = return `cRet`(`implName`()) return res + +macro ffiExport*(prc: untyped): untyped = + ## Marks a proc that takes no arguments as a simple synchronous C export. The + ## macro maps the native Nim return type to the C ABI and starts the Nim + ## runtime. `{.ffi.}` reaches the same path from the shape alone. See the + ## module doc. + prc.expectKind({nnkProcDef, nnkFuncDef}) + assertFFIPath(prc, fpExport) + return buildFFIExportProc(prc) diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index 63de124..27c8fd4 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -179,10 +179,10 @@ macro declareLibrary*( let addName = libraryName & "_add_event_listener" let addErr = "error: invalid context in " & addName let addBody = quote: - # Runs on the foreign caller thread, which may not be the one that ran a - # prior entry point: initialize this thread's GC before any Nim allocation - # ($eventName / the registry Table+seq), else the per-thread allocator - # region is uninitialized and faults. + # This code runs on the foreign caller thread. That thread can differ from + # the thread of an earlier entry point. If the GC of the thread is not + # ready, the first Nim allocation ($eventName, the registry Table and seq) + # faults. Therefore initialize the GC here. when declared(initializeLibrary): initializeLibrary() var ret: uint64 = 0 diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index cd84196..6f43bd8 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -7,6 +7,8 @@ import ../ffi_thread_request import ../codegen/[meta, string_helpers] import ./c_macro_helpers import ./ffi_scalar +import ./ffi_route +import ./ffi_export when defined(ffiGenBindings): import ../codegen/rust import ../codegen/cpp @@ -531,7 +533,7 @@ proc replyEncode( return quote: # A `seq[byte]` result goes on the wire as CBOR, the same as every other # `abi = cbor` return. The C, C++ and Rust decoders expect CBOR. They reject - # raw bytes with "value encoded in non-canonical form". + # raw bytes with the error "value encoded in non-canonical form". when typeof(`typedResIdent`.value) is void: return ok(newSeq[byte]()) elif typeof(`typedResIdent`.value) is FFIHandleRoot: @@ -1145,29 +1147,64 @@ proc buildFFIProc( echo stmts.repr return stmts +proc buildFFIDtorProc(prc: NimNode, abiFormat: ABIFormat): NimNode {.compileTime.} +proc buildFFIEventProc(prc: NimNode, leading: seq[NimNode]): NimNode {.compileTime.} + macro ffi*(args: varargs[untyped]): untyped = - ## 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. + ## Simplified FFI macro for a type or a proc. A type registers for binding + ## generation. For a proc, `routeFFIProc` reads the signature and picks the + ## path: a context method, a static call, a synchronous export, a destructor, + ## or an event. See `ffi/internal/ffi_route.nim` for the rules. requireBeforeGenBindings("`.ffi.`") # Annotated node is the last vararg; leading args are `"abi = ..."` specs. let prc = args[^1] - let abiFormat = resolveFFISpecs(args[0 ..^ 2]) + let leading = 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. if prc.kind == nnkTypeDef: - gateFFITypeABIFormat(abiFormat, "`.ffi.` type") + let typeABIFormat = resolveFFISpecs(leading) + gateFFITypeABIFormat(typeABIFormat, "`.ffi.` type") var cleanTypeDef = prc.copyNimTree() if cleanTypeDef[0].kind == nnkPragmaExpr: cleanTypeDef[0] = cleanTypeDef[0][0] - return registerFFITypeInfo(cleanTypeDef, abiFormat) + return registerFFITypeInfo(cleanTypeDef, typeABIFormat) + if prc.kind notin {nnkProcDef, nnkFuncDef}: + error("`.ffi.` must be applied to a type or a proc definition") requireLibraryDeclared("`.ffi.`") - return buildFFIProc(prc, abiFormat, isStatic = false) + + let path = routeFFIProc(prc) + + # An event may lead with a wire-name literal, which the ABI parser rejects, so + # it resolves its own specs. + if path == fpEvent: + return buildFFIEventProc(prc, leading) + + let abiFormat = resolveFFISpecs(leading) + case path + of fpExport: + # The export crosses the ABI with its own return value, so no ABI applies. + if leading.len > 0: + error( + "`.ffi.` proc " & $procIdent(prc) & + " is a synchronous export and takes no `abi = ...` spec" + ) + return buildFFIExportProc(prc) + of fpDtor: + gateABIFormat(abiFormat, "`.ffi.` destructor") + return buildFFIDtorProc(prc, abiFormat) + of fpStatic: + gateABIFormat(abiFormat, "`.ffi.` static proc") + return buildFFIProc(prc, abiFormat, isStatic = true) + of fpMethod: + return buildFFIProc(prc, abiFormat, isStatic = false) + of fpEvent: + error("unreachable: the event path returns above") macro ffiStatic*(args: varargs[untyped]): untyped = ## Context-independent `{.ffi.}`: no library receiver, and no `ctx` in the C - ## wrapper, so a host calls it without constructing the library. + ## wrapper, so a host calls it without constructing the library. `{.ffi.}` + ## reaches the same path from the shape alone. requireBeforeGenBindings("`.ffiStatic.`") requireLibraryDeclared("`.ffiStatic.`") let prc = args[^1] @@ -1175,6 +1212,7 @@ macro ffiStatic*(args: varargs[untyped]): untyped = gateABIFormat(abiFormat, "`.ffiStatic.` proc") if prc.kind notin {nnkProcDef, nnkFuncDef}: error("`.ffiStatic.` must be applied to a proc definition") + assertFFIPath(prc, fpStatic) return buildFFIProc(prc, abiFormat, isStatic = true) proc buildCtorRequestType( @@ -1330,7 +1368,7 @@ proc buildCtorProcessFFIRequestProc( when `libTypeName` is ref: GC_ref(`myLibIdent`[]) `myLibRefdIdent` = true - # After the store, so an observer never sees the fallback. + # Set the flag after the store, so an observer never sees the fallback. `libReadyIdent`.store(true) newBody.add quote do: @@ -1602,16 +1640,9 @@ macro ffiCtor*(args: varargs[untyped]): untyped = echo stmts.repr return stmts -macro ffiDtor*(args: varargs[untyped]): untyped = - ## 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] - let abiFormat = resolveABIFormat(args[0 ..^ 2]) - gateABIFormat(abiFormat, "`.ffiDtor.` proc") - +proc buildFFIDtorProc(prc: NimNode, abiFormat: ABIFormat): NimNode {.compileTime.} = + ## Emits the C-exported FFIContext destructor. `{.ffi.}` and `{.ffiDtor.}` + ## share it. let procName = prc[0] let formalParams = prc[3] let bodyNode = prc[^1] @@ -1723,30 +1754,30 @@ macro ffiDtor*(args: varargs[untyped]): untyped = echo stmts.repr return stmts -macro ffiEvent*(args: varargs[untyped]): untyped = - ## 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. - ## +macro ffiDtor*(args: varargs[untyped]): untyped = + ## 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. + ## `{.ffi.}` reaches the same path from the shape alone. + requireBeforeGenBindings("`.ffiDtor.`") + requireLibraryDeclared("`.ffiDtor.`") + let prc = args[^1] + let abiFormat = resolveABIFormat(args[0 ..^ 2]) + gateABIFormat(abiFormat, "`.ffiDtor.` proc") + assertFFIPath(prc, fpDtor) + return buildFFIDtorProc(prc, abiFormat) + +proc buildFFIEventProc(prc: NimNode, leading: seq[NimNode]): NimNode {.compileTime.} = + ## Emits the event dispatcher. `{.ffi.}` and `{.ffiEvent.}` share it. ## One parameter rides the wire directly (a scalar, or an existing `{.ffi.}` ## object). Two or more are bundled into a synthesised, registered envelope ## object named `Payload` whose fields are the parameters, ## so the foreign side still decodes one typed value. - requireBeforeGenBindings("`.ffiEvent.`") - requireLibraryDeclared("`.ffiEvent.`") - if args.len < 1: - error("ffiEvent must be applied to a proc declaration") - - let prc = args[^1] - if prc.kind notin {nnkProcDef, nnkFuncDef}: - error("ffiEvent must be applied to a proc declaration") - let procName = prc[0] var userProcName = procName if procName.kind == nnkPostfix: userProcName = procName[1] - let leading = args[0 ..^ 2] let (wireName, abiSpecStart) = resolveEventWireName(leading, userProcName) let abiFormat = resolveABIFormat(leading[abiSpecStart ..^ 1]) gateABIFormat(abiFormat, "`.ffiEvent.` proc") @@ -1849,6 +1880,22 @@ macro ffiEvent*(args: varargs[untyped]): untyped = echo resultStmts.repr return resultStmts +macro ffiEvent*(args: varargs[untyped]): untyped = + ## 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. + ## `{.ffi.}` reaches the same path from the shape alone. + requireBeforeGenBindings("`.ffiEvent.`") + requireLibraryDeclared("`.ffiEvent.`") + if args.len < 1: + error("ffiEvent must be applied to a proc declaration") + + let prc = args[^1] + if prc.kind notin {nnkProcDef, nnkFuncDef}: + error("ffiEvent must be applied to a proc declaration") + assertFFIPath(prc, fpEvent) + return buildFFIEventProc(prc, args[0 ..^ 2]) + proc reportScalarFastPathDrops(procs: seq[FFIProcMeta]) {.compileTime.} = ## Fail loudly on scalar-fast-path procs a target can't bind, unless ## `-d:ffiAllowScalarSkip` downgrades it to a hint. diff --git a/ffi/internal/ffi_route.nim b/ffi/internal/ffi_route.nim new file mode 100644 index 0000000..154d001 --- /dev/null +++ b/ffi/internal/ffi_route.nim @@ -0,0 +1,99 @@ +## Picks the FFI path of a proc from the shape of its signature. +## +## `{.ffi.}`, `{.ffiStatic.}`, `{.ffiExport.}`, `{.ffiDtor.}` and `{.ffiEvent.}` +## own five disjoint shapes, so one router serves all five. Each shape that the +## router claims fails to compile under any other pragma today, so the router +## only turns a compile error into the meaning the writer intended. +## +## `{.ffiCtor.}` stays explicit, because its shape is not free. A ctor differs +## from a static call by one token: the type inside `Result`. A static call that +## returns the library type builds today and exports a working C symbol. A +## router would silently give it the ctor ABI instead. + +import std/macros +import ../codegen/meta + +type FFIPath* = enum + fpMethod ## A library or handle receiver, and an async result. + fpStatic ## No receiver, and an async result. + fpExport ## No arguments, and a synchronous result. + fpDtor ## A library receiver, and no result. + fpEvent ## A payload parameter, and no result. + +func pathPragma*(path: FFIPath): string = + case path + of fpMethod: "`.ffi.`" + of fpStatic: "`.ffiStatic.`" + of fpExport: "`.ffiExport.`" + of fpDtor: "`.ffiDtor.`" + of fpEvent: "`.ffiEvent.`" + +func pathShape*(path: FFIPath): string = + case path + of fpMethod: + "the first parameter is the library type or an {.ffiHandle.} type, and the " & + "return type is Future[Result[T, string]]" + of fpStatic: + "there is no library receiver, and the return type is Future[Result[T, string]]" + of fpExport: + "there are no parameters, and the return type is a plain Nim type" + of fpDtor: + "there is one library parameter, and the return type is nothing or Future[void]" + of fpEvent: + "there is a payload parameter that is not the library type, and there is no result" + +func isFuture(t: NimNode): bool = + return + t.kind == nnkBracketExpr and t.len == 2 and t[0].kind == nnkIdent and + $t[0] == "Future" + +func isFutureVoid(t: NimNode): bool = + return isFuture(t) and t[1].kind == nnkIdent and $t[1] == "void" + +proc isLibReceiver(t: NimNode): bool {.compileTime.} = + ## The receiver is the type that `declareLibrary` recorded, or a handle type. + if t.kind != nnkIdent: + return false + return ($t == currentLibType and currentLibType.len > 0) or isFFIHandleTypeName($t) + +func procIdent*(prc: NimNode): NimNode = + return + if prc[0].kind == nnkPostfix: + prc[0][1] + else: + prc[0] + +proc routeFFIProc*(prc: NimNode): FFIPath {.compileTime.} = + ## Reads the receiver and the return type, then names the path. + let params = prc[3] + let ret = params[0] + let hasReceiver = params.len > 1 and isLibReceiver(params[1][1]) + + if hasReceiver: + return if ret.kind == nnkEmpty or isFutureVoid(ret): fpDtor else: fpMethod + if params.len == 1 and not isFuture(ret): + return fpExport + # A static call always returns Future[Result[T, string]], so a payload + # parameter with no result can only be an event. + if params.len > 1 and ret.kind == nnkEmpty: + return fpEvent + return fpStatic + +proc assertFFIPath*(prc: NimNode, want: FFIPath) {.compileTime.} = + ## Guards an explicit pragma against a signature that routes elsewhere. + let got = routeFFIProc(prc) + if got == want: + return + let name = $procIdent(prc) + # A receiver is the one mismatch a caller can read straight off the signature. + if want == fpStatic and got == fpMethod: + error( + "`.ffiStatic.` proc " & name & " takes " & prc[3][1][1].repr & + " as its first parameter, which is the library type or an {.ffiHandle.} type. " & + "A receiver belongs to a context. Make it an `{.ffi.}` method instead." + ) + error( + pathPragma(want) & " proc " & name & " has the shape of a " & pathPragma(got) & + " proc. Use " & pathPragma(got) & " here, or make sure that " & pathShape(want) & + "." + ) diff --git a/ffi/internal/ffi_scalar.nim b/ffi/internal/ffi_scalar.nim index cbbfb4f..426b133 100644 --- a/ffi/internal/ffi_scalar.nim +++ b/ffi/internal/ffi_scalar.nim @@ -6,10 +6,11 @@ import ../codegen/meta proc buildLibReadyGuard*( ctxHandlerName, libTypeName: NimNode ): NimNode {.compileTime.} = - ## Rejects a request that reached the FFI thread with no library constructed. - ## Only for `ref` types: an `object` fallback is a usable zero value callers may - ## rely on, a `ref` one is nil. Sits in the handler, behind any queued ctor, so - ## calling without awaiting the create callback still works. + ## Rejects a request that reaches the FFI thread before the ctor stores a + ## library. The guard applies only to a `ref` type. For an `object` type the + ## fallback is a usable zero value, but for a `ref` type it is nil. The guard + ## runs in the handler, behind the ctor in the queue. Thus a host can send a + ## call before it waits for the create callback. quote: when `libTypeName` is ref: if not `ctxHandlerName`[].libReady.load(): diff --git a/tests/unit/fixtures/router_dtor_wrong_shape_fixture.nim b/tests/unit/fixtures/router_dtor_wrong_shape_fixture.nim new file mode 100644 index 0000000..08df3f1 --- /dev/null +++ b/tests/unit/fixtures/router_dtor_wrong_shape_fixture.nim @@ -0,0 +1,13 @@ +## Must fail: `{.ffiDtor.}` on a method shape (see tests/unit/test_ffi_router_reject.nim). + +import ffi, chronos + +type RouterRejLib = object + base: int + +declareLibrary("routerrej", RouterRejLib) + +proc routerrejBad*(lib: RouterRejLib): Future[Result[int, string]] {.ffiDtor.} = + return ok(lib.base) + +genBindings() diff --git a/tests/unit/fixtures/router_event_wrong_shape_fixture.nim b/tests/unit/fixtures/router_event_wrong_shape_fixture.nim new file mode 100644 index 0000000..ee5f2f8 --- /dev/null +++ b/tests/unit/fixtures/router_event_wrong_shape_fixture.nim @@ -0,0 +1,13 @@ +## Must fail: `{.ffiEvent.}` on a static shape (see tests/unit/test_ffi_router_reject.nim). + +import ffi, chronos + +type RouterRejLib = object + base: int + +declareLibrary("routerrej", RouterRejLib) + +proc routerrejBad*(n: int): Future[Result[int, string]] {.ffiEvent.} = + return ok(n) + +genBindings() diff --git a/tests/unit/fixtures/router_export_wrong_shape_fixture.nim b/tests/unit/fixtures/router_export_wrong_shape_fixture.nim new file mode 100644 index 0000000..379c825 --- /dev/null +++ b/tests/unit/fixtures/router_export_wrong_shape_fixture.nim @@ -0,0 +1,13 @@ +## Must fail: `{.ffiExport.}` on a static shape (see tests/unit/test_ffi_router_reject.nim). + +import ffi, chronos + +type RouterRejLib = object + base: int + +declareLibrary("routerrej", RouterRejLib) + +proc routerrejBad*(): Future[Result[int, string]] {.ffiExport.} = + return ok(1) + +genBindings() diff --git a/tests/unit/test_ctx_after_failed_ctor.nim b/tests/unit/test_ctx_after_failed_ctor.nim index 04459e0..f67de44 100644 --- a/tests/unit/test_ctx_after_failed_ctor.nim +++ b/tests/unit/test_ctx_after_failed_ctor.nim @@ -3,16 +3,17 @@ import unittest2 import results import ffi -# A failing {.ffiCtor.} still hands the caller a live context — the ctor body -# runs on the FFI thread, long after the C entry point returned the pointer. -# `myLib` is not nil (the FFI thread points it at a default-valued fallback), but -# for a `ref` library type that default IS nil, so a later {.ffi.} call used to -# hand the user body a nil ref and crash on its first field access. +# A {.ffiCtor.} that fails still gives the caller a live context. The ctor body +# runs on the FFI thread, long after the C entry point returns the pointer. +# `myLib` is not nil, because the FFI thread points it at a default fallback. +# For a `ref` library type that fallback is nil. A later {.ffi.} call therefore +# gave the user body a nil ref and crashed on the first field access. type FailedCtorLib = ref object marker: int -# Stub the importc NimMain declareLibrary emits (plain-exe link). +# A stub for the NimMain proc that declareLibrary imports. The test links as a +# plain executable. {.emit: "void libfailedctorNimMain(void) {}".} declareLibrary("failedctor", FailedCtorLib) @@ -27,7 +28,7 @@ proc failedctor_create*( return err("ctor deliberately failed") return ok(FailedCtorLib(marker: 1)) -# Both bodies touch a field, which is what faults on a nil ref receiver. +# Both bodies read a field. A nil ref receiver faults on that read. proc failedctor_ping*(lib: FailedCtorLib): Future[Result[string, string]] {.ffi.} = return ok("pong:" & $lib.marker) @@ -65,7 +66,7 @@ proc waitCalled(s: var CallbackState): bool = s.called.load() proc createFailedCtx(s: var CallbackState): ptr FFIContext[FailedCtorLib] = - ## Drives the ctor down its error path and returns the still-live context. + ## Sends the ctor down its error path and returns the context, which stays alive. resetState(s) var cfg = cborEncode(FailedctorCreateCtorReq(config: FailedCtorConfig(shouldFail: true))) @@ -82,15 +83,15 @@ suite "{.ffi.} call after a failed constructor": let ctx = createFailedCtx(s) check not ctx.isNil() check s.retCode.load() == int(RET_ERR) - # `myLib` is non-nil even here: the FFI thread points it at a default-valued - # fallback before dispatching. `libReady` is what says the ctor stored a real - # library. - check not ctx[].myLib.isNil() # the fallback, not a constructed library - check ctx[].myLib[].isNil() # ...and for a `ref` lib that fallback is nil + # `myLib` is not nil even here. The FFI thread points it at a default + # fallback before it dispatches the request. `libReady` shows if the ctor + # stored a real library. + check not ctx[].myLib.isNil() # the fallback, not a real library + check ctx[].myLib[].isNil() # for a `ref` lib the fallback is nil check not ctx[].libReady.load() - # The synchronous return only reports that the request was accepted; the - # rejection itself comes back through the callback. + # The synchronous return only reports that the FFI thread accepted the + # request. The callback delivers the rejection. test "a no-arg call on an uninitialized library reports RET_ERR, no crash": var s: CallbackState let ctx = createFailedCtx(s) @@ -117,8 +118,8 @@ suite "{.ffi.} call after a failed constructor": check waitCalled(s) check s.retCode.load() == int(RET_ERR) - # The guard runs on the FFI thread, behind the queued ctor, so it must not - # penalise a host that fires a call without first awaiting the create callback. + # The guard runs on the FFI thread, behind the ctor in the queue. Thus a host + # can send a call before it waits for the create callback. test "a call issued before the successful ctor callback still succeeds": var ctorState: CallbackState resetState(ctorState) @@ -130,7 +131,7 @@ suite "{.ffi.} call after a failed constructor": check not raw.isNil() let ctx = cast[ptr FFIContext[FailedCtorLib]](raw) - # Deliberately no wait: the request queues behind the in-flight ctor. + # No wait here, on purpose: the request goes into the queue behind the ctor. var callState: CallbackState resetState(callState) var req = cborEncode(FailedctorPingReq()) diff --git a/tests/unit/test_ffi_context.nim b/tests/unit/test_ffi_context.nim index 15f38c4..161349a 100644 --- a/tests/unit/test_ffi_context.nim +++ b/tests/unit/test_ffi_context.nim @@ -329,9 +329,9 @@ suite "sendRequestToFFIThread": check callbackErr(d) == "intentional failure" test "seq[byte] result rides as a CBOR byte string, not raw bytes": - # A `seq[byte]` return must be CBOR, the same as every other reply. The - # generated C, C++ and Rust decoders call `nimffi_dec_bytes` on the payload - # and reject a raw reply with "value encoded in non-canonical form". + # A `seq[byte]` return must be CBOR, the same as every other reply. The C, + # C++ and Rust decoders call `nimffi_dec_bytes` on the payload. They reject + # a raw reply with the error "value encoded in non-canonical form". var d: CallbackData initCallbackData(d) defer: @@ -349,7 +349,7 @@ suite "sendRequestToFFIThread": waitCallback(d) check d.retCode == RET_OK let reply = callbackBytes(d) - # The wire contract is a CBOR byte-string header (major type 2, 0x40..0x5b) + # The wire contract is a CBOR byte-string header (major type 2, 0x40..0x5b), # and then the 4 payload bytes. check reply.len == 5 check reply[0] == 0x44'u8 diff --git a/tests/unit/test_ffi_router.nim b/tests/unit/test_ffi_router.nim new file mode 100644 index 0000000..adcf782 --- /dev/null +++ b/tests/unit/test_ffi_router.nim @@ -0,0 +1,161 @@ +## `{.ffi.}` picks the path from the shape of the signature. This file writes one +## proc per path with the same pragma, then calls each generated C wrapper. + +import std/[atomics, os] +import unittest2 +import results +import ffi + +type RouterLib = ref object + marker: int + +type RouterTick {.ffi.} = object + count: int + +# A stub for the NimMain proc that declareLibrary imports. The test links as a +# plain executable. +{.emit: "void librouterNimMain(void) {}".} + +declareLibrary("router", RouterLib) + +proc router_create*(seed: int): Future[Result[RouterLib, string]] {.ffiCtor.} = + return ok(RouterLib(marker: seed)) + +# A library receiver, so the router picks the context method. +proc router_marker*(lib: RouterLib): Future[Result[int, string]] {.ffi.} = + return ok(lib.marker) + +# No receiver, so the router picks the static call. +proc router_version*(): Future[Result[string, string]] {.ffi.} = + return ok("router v1") + +# No arguments and a plain return type, so the router picks the synchronous +# export. +proc router_alive*(): int {.ffi.} = + 7 + +# A library receiver and no result, so the router picks the destructor. +proc router_destroy*(lib: RouterLib) {.ffi.} = + discard + +# A payload parameter and no result, so the router picks the event. The leading +# literal sets the wire name, exactly as {.ffiEvent.} accepts it. +proc onRouterTick*(evt: RouterTick) {.ffi: "on_router_tick".} = + discard + +# The event queue is per-thread, so only a handler on the FFI thread can fire. +proc router_tick*(lib: RouterLib): Future[Result[int, string]] {.ffi.} = + onRouterTick(RouterTick(count: 3)) + return ok(lib.marker) + +type CallbackState = object + called: Atomic[bool] + retCode: Atomic[int] + msg: string + +proc resetState(s: var CallbackState) = + s.called.store(false) + s.retCode.store(-1) + s.msg = "" + +proc recordingCallback( + retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer +) {.cdecl, gcsafe, raises: [].} = + let s = cast[ptr CallbackState](userData) + if not msg.isNil() and len > 0: + s[].msg = newString(int(len)) + copyMem(addr s[].msg[0], msg, int(len)) + s[].retCode.store(int(retCode)) + s[].called.store(true) + +proc encodedPtr(bytes: var seq[byte]): ptr byte = + if bytes.len == 0: + nil + else: + cast[ptr byte](addr bytes[0]) + +proc waitCalled(s: var CallbackState): bool = + var tries = 0 + while not s.called.load() and tries < 500: + os.sleep(5) + inc tries + s.called.load() + +proc createCtx(s: var CallbackState): pointer = + resetState(s) + var cfg = cborEncode(RouterCreateCtorReq(seed: 42)) + let ret = router_create(encodedPtr(cfg), cfg.len.csize_t, recordingCallback, addr s) + discard waitCalled(s) + ret + +suite "{.ffi.} routes on the shape of the signature": + test "a library receiver routes to the context method": + var s: CallbackState + let ctx = createCtx(s) + check not ctx.isNil() + defer: + discard router_destroy(ctx) + + resetState(s) + var req = cborEncode(RouterMarkerReq()) + check router_marker( + cast[ptr FFIContext[RouterLib]](ctx), + recordingCallback, + addr s, + encodedPtr(req), + req.len.csize_t, + ) == RET_OK + check waitCalled(s) + check s.retCode.load() == int(RET_OK) + check cborDecode(cast[seq[byte]](s.msg), int).value == 42 + + test "no receiver routes to the static call, which needs no context": + var s: CallbackState + resetState(s) + var req = cborEncode(RouterVersionReq()) + check router_version(recordingCallback, addr s, encodedPtr(req), req.len.csize_t) == + RET_OK + check waitCalled(s) + check s.retCode.load() == int(RET_OK) + check cborDecode(cast[seq[byte]](s.msg), string).value == "router v1" + + test "no arguments and a plain return type route to the synchronous export": + # The export returns its value directly, with no context and no callback. + check router_alive() == cint(7) + + test "a payload parameter and no result route to the event": + var s: CallbackState + let ctx = createCtx(s) + check not ctx.isNil() + defer: + discard router_destroy(ctx) + + var evt: CallbackState + resetState(evt) + check router_add_event_listener( + cast[ptr FFIContext[RouterLib]](ctx), + "on_router_tick".cstring, + recordingCallback, + addr evt, + ) != 0'u64 + + resetState(s) + var req = cborEncode(RouterTickReq()) + check router_tick( + cast[ptr FFIContext[RouterLib]](ctx), + recordingCallback, + addr s, + encodedPtr(req), + req.len.csize_t, + ) == RET_OK + check waitCalled(evt) + let env = cborDecode(cast[seq[byte]](evt.msg), EventEnvelope[RouterTick]) + check env.value.eventType == "on_router_tick" + check env.value.payload.count == 3 + + test "a library receiver and no result route to the destructor": + var s: CallbackState + let ctx = createCtx(s) + check not ctx.isNil() + check router_destroy(ctx) == RET_OK + check router_destroy(nil) == RET_ERR diff --git a/tests/unit/test_ffi_router_reject.nim b/tests/unit/test_ffi_router_reject.nim new file mode 100644 index 0000000..113de3a --- /dev/null +++ b/tests/unit/test_ffi_router_reject.nim @@ -0,0 +1,39 @@ +## `{.ffi.}` routes on the shape, but the named pragmas still assert it. Each +## fixture compiles in a child `nim check`, so its expected failure is an +## assertion rather than this file's own compile error. + +import std/[os, osproc, strutils, compilesettings] +import unittest2 + +const + fixtureDir = currentSourcePath().parentDir() / "fixtures" + nimExe = getCurrentCompilerExe() + ffiSearchPaths = querySettingSeq(searchPaths) + +proc checkFixture(name: string): tuple[output: string, exitCode: int] = + let cacheDir = getTempDir() / "ffi_router_reject_cache" / name + var cmd = quoteShell(nimExe) & " check --hints:off --warnings:off" + for p in ffiSearchPaths: + cmd.add(" --path:" & quoteShell(p)) + cmd.add(" --nimcache:" & quoteShell(cacheDir)) + cmd.add(" " & quoteShell(fixtureDir / (name & "_fixture.nim"))) + execCmdEx(cmd) + +suite "a named pragma asserts the shape it claims": + test "{.ffiExport.} on a static shape names the proc and the right pragma": + let (output, code) = checkFixture("router_export_wrong_shape") + check code != 0 + check output.contains("routerrejBad") + check output.contains("`.ffiStatic.`") + + test "{.ffiDtor.} on a method shape names the proc and the right pragma": + let (output, code) = checkFixture("router_dtor_wrong_shape") + check code != 0 + check output.contains("routerrejBad") + check output.contains("`.ffi.`") + + test "{.ffiEvent.} on a static shape names the proc and the right pragma": + let (output, code) = checkFixture("router_event_wrong_shape") + check code != 0 + check output.contains("routerrejBad") + check output.contains("`.ffiStatic.`") diff --git a/tests/unit/test_rust_codegen.nim b/tests/unit/test_rust_codegen.nim index 5db54b6..4574b57 100644 --- a/tests/unit/test_rust_codegen.nim +++ b/tests/unit/test_rust_codegen.nim @@ -44,10 +44,11 @@ suite "nimTypeToRust: strings, pointers and containers": check nimTypeToRust("echoRequest") == "EchoRequest" suite "generateTypesRs: seq[byte] rides as a CBOR byte string": - ## A `seq[byte]` in a struct that is a `seq` element became a `Vec` integer - ## array (CBOR major type 4). The Nim decoder rejects that array with "value - ## encoded in non-canonical form". ByteBuf makes ciborium write a byte string - ## (major type 2), the same as every other backend. + ## A struct with a `seq[byte]` field can be a `seq` element. For that shape the + ## Rust backend wrote a `Vec` integer array (CBOR major type 4). The Nim + ## decoder rejects that array with the error "value encoded in non-canonical + ## form". ByteBuf makes ciborium write a byte string (major type 2), the same + ## as every other backend. setup: let types = @[ FFITypeMeta( diff --git a/tests/unit/test_wire_compat.nim b/tests/unit/test_wire_compat.nim index d378baa..d607c79 100644 --- a/tests/unit/test_wire_compat.nim +++ b/tests/unit/test_wire_compat.nim @@ -29,7 +29,7 @@ type WireBytesEntry {.ffi.} = object type WireNestedBytes {.ffi.} = object ## A `seq[byte]` in a struct that is a `seq` element. This shape broke the - ## Rust backend, which wrote an integer array in place of a ByteBuf. + ## Rust backend. The backend wrote an integer array in place of a ByteBuf. entries: seq[WireBytesEntry] proc toHex(bytes: openArray[byte]): string = @@ -105,7 +105,7 @@ suite "wire format — seq[byte]": suite "wire format — seq[byte] nested in a seq-of-struct": ## A `seq[byte]` field in a struct that is a `seq` element must stay a CBOR - ## byte string (major type 2) at depth. Every backend must match this wire + ## byte string (major type 2) at depth. Every backend must obey this wire ## contract. The Rust generator broke it and wrote a `Vec` integer array. test "each nested seq[byte] rides as a byte string, request and response alike": let v = WireNestedBytes( @@ -118,8 +118,8 @@ suite "wire format — seq[byte] nested in a seq-of-struct": check toHex(bytes) == "a167656e747269657382a2626964627330646461746142aabba26269646273316464617461" & "43010203" - # The payloads must use byte-string headers (0x42 = bytes(2), 0x43 = - # bytes(3)), not array headers (0x82 or 0x83). + # The payloads must use a byte-string header (0x42 = bytes(2), 0x43 = + # bytes(3)). An array header (0x82 or 0x83) is wrong. check "6461746142aabb" in toHex(bytes) # "data" + 0x42 check "6461746143010203" in toHex(bytes) # "data" + 0x43 <01 02 03> let back = cborDecode(bytes, WireNestedBytes)