From b98180f735222284c8f38860503c2ddac5b849e5 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz Date: Fri, 31 Jul 2026 15:47:26 -0300 Subject: [PATCH] chore: abi improvements --- CHANGELOG.md | 17 ++++ examples/echo/c_abi_bindings/echo.h | 18 ++++ ffi/codegen/c.nim | 41 ++++++++- ffi/codegen/meta.nim | 4 + ffi/internal/ffi_library.nim | 49 +++++++---- ffi/internal/ffi_macro.nim | 13 ++- .../fixtures/ctor_without_dtor_fixture.nim | 20 +++++ .../fixtures/declare_enum_abi_fixture.nim | 28 +++++++ .../ffistatic_abi_c_scalar_fixture.nim | 3 + tests/unit/fixtures/scalar_skip_fixture.nim | 3 + tests/unit/test_c_abi_dispatch.nim | 3 + tests/unit/test_c_codegen.nim | 84 +++++++++++++++++++ tests/unit/test_declare_library.nim | 34 ++++++++ 13 files changed, 299 insertions(+), 18 deletions(-) create mode 100644 tests/unit/fixtures/ctor_without_dtor_fixture.nim create mode 100644 tests/unit/fixtures/declare_enum_abi_fixture.nim create mode 100644 tests/unit/test_declare_library.nim diff --git a/CHANGELOG.md b/CHANGELOG.md index 0975823..44dcf65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ All notable changes to this project are documented in this file. ## [Unreleased] ### Added +- The `abi = c` C header now declares the event-listener ABI that + `declareLibrary` always exports (`_add_event_listener` / + `_remove_event_listener`) and the `FFICallBack` typedef they take, so a + consumer needs no hand-written header. The typed listener machinery for + `{.ffiEvent.}` is still unsupported under `abi = c`. +- The `abi = c` C header is self-contained: it emits the `` / + `` includes, the `NIMFFI_RET_*` status codes, and short + `#ifndef`-guarded `RET_*` aliases for consumers that use the unprefixed names. +- Each `{.ffi.}` proc's `##` doc comment reaches the generated C header as a + `/** ... */` block above the declaration and its wrapper. +- `declareLibrary` accepts the `ABIFormat` enum for `defaultABIFormat`, so + `defaultABIFormat = ABIFormat.C` compiles alongside the `"c"` string. +- `declareLibrary` takes an optional `headerBanner` argument, stamped as a + `//`-comment block at the top of every generated C header. +- `genBindings()` fails compilation when a library declares an `{.ffiCtor.}` but + no `{.ffiDtor.}`, so the context a constructor builds always has a way to be + released. - `{.ffiEvent.}` now accepts multiple parameters. The macro synthesises and registers an envelope object (`Payload`) whose fields are the parameters and dispatches an instance of it, so multi-field events no diff --git a/examples/echo/c_abi_bindings/echo.h b/examples/echo/c_abi_bindings/echo.h index 0365f73..123e990 100644 --- a/examples/echo/c_abi_bindings/echo.h +++ b/examples/echo/c_abi_bindings/echo.h @@ -13,6 +13,18 @@ carrying the elapsed milliseconds as decimal text; always followed by a terminal RET_OK/RET_ERR. Ignore it unless you want progress. */ #define NIMFFI_RET_STALE_WARN 3 +#ifndef RET_OK +#define RET_OK NIMFFI_RET_OK +#endif +#ifndef RET_ERR +#define RET_ERR NIMFFI_RET_ERR +#endif +#ifndef RET_MISSING_CALLBACK +#define RET_MISSING_CALLBACK NIMFFI_RET_MISSING_CALLBACK +#endif +#ifndef RET_STALE_WARN +#define RET_STALE_WARN NIMFFI_RET_STALE_WARN +#endif /* ============================================================ */ /* Generated constants */ @@ -68,6 +80,10 @@ static inline char* nimffi_abi_dup_cstr_n(const char* s, size_t n) { return p; } #endif +#ifndef NIMFFI_FFICALLBACK_DEFINED +#define NIMFFI_FFICALLBACK_DEFINED +typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_data); +#endif #ifdef __cplusplus extern "C" { #endif @@ -82,6 +98,8 @@ int echo_lib_version(EchoLibVersionReplyFn on_reply, void* user_data, const Echo int echo_shout_anon(EchoShoutAnonReplyFn on_reply, void* user_data, const EchoShoutAnonReq* req); /** Releases the echo context. */ int echo_destroy(void* ctx); +uint64_t echo_add_event_listener(void* ctx, const char* event_name, FFICallback callback, void* user_data); +int echo_remove_event_listener(void* ctx, uint64_t listener_id); #ifdef __cplusplus } /* extern "C" */ diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index 4568594..7908f92 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -842,6 +842,16 @@ func constDeclLines(consts: seq[FFIConstMeta]): seq[string] = lines.add("") return lines +func headerBannerLines(banner: string): seq[string] = + ## `banner` as `//` line comments for the top of a generated header; [] if empty. + if banner.strip().len == 0: + return @[] + var rendered: seq[string] = @[] + for line in banner.splitLines(): + # A trailing `\` would splice the next generated line into the `//` comment. + rendered.add("// " & line.strip(leading = false, chars = Whitespace + {'\\'})) + return rendered + func generateCPreludeHeader*(): string = ## The library-agnostic `nim_ffi_prelude.h`, emitted verbatim. return HeaderPreludeTpl & "\n" @@ -856,6 +866,7 @@ proc generateCLibHeader*( libName: string, events: seq[FFIEventMeta] = @[], consts: seq[FFIConstMeta] = @[], + banner: string = "", ): string = ## The `.h` header: library structs, monomorphised codecs and async API. let classified = classifyProcs(procs) @@ -869,6 +880,7 @@ proc generateCLibHeader*( let guard = "NIM_FFI_LIB_" & libName.toUpperAscii() & "_H_INCLUDED" var lines: seq[string] = @[] + lines.add(headerBannerLines(banner)) lines.add("#ifndef " & guard) lines.add("#define " & guard) lines.add("#include \"" & CborHeaderName & "\"") @@ -1192,6 +1204,15 @@ proc emitAbiExternDecls( ) for l in abiScalarDupHelper(): lines.add(l) + # declareLibrary always exports the event-listener ABI on the FFIContext, so + # declare it here (and its FFICallback) — no hand-written header needed. The + # typed listener machinery isn't emitted yet (see generateCAbiLibHeader). + lines.add("#ifndef NIMFFI_FFICALLBACK_DEFINED") + lines.add("#define NIMFFI_FFICALLBACK_DEFINED") + lines.add( + "typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_data);" + ) + lines.add("#endif") lines.add("#ifdef __cplusplus") lines.add("extern \"C\" {") lines.add("#endif") @@ -1224,6 +1245,13 @@ proc emitAbiExternDecls( ) of FFIKind.DTOR: lines.add("int " & p.procName & "(void* ctx);") + lines.add( + "uint64_t " & libName & "_add_event_listener(void* ctx, const char* event_name, " & + "FFICallback callback, void* user_data);" + ) + lines.add( + "int " & libName & "_remove_event_listener(void* ctx, uint64_t listener_id);" + ) lines.add("") lines.add("#ifdef __cplusplus") lines.add("} /* extern \"C\" */") @@ -1481,6 +1509,7 @@ proc generateCAbiLibHeader*( libName: string, events: seq[FFIEventMeta] = @[], consts: seq[FFIConstMeta] = @[], + banner: string = "", ): string = if events.len > 0: raise newException( @@ -1499,6 +1528,7 @@ proc generateCAbiLibHeader*( let guard = "NIM_FFI_LIB_" & libName.toUpperAscii() & "_C_ABI_H_INCLUDED" var lines: seq[string] = @[] + lines.add(headerBannerLines(banner)) lines.add("#ifndef " & guard) lines.add("#define " & guard) lines.add("#include ") @@ -1516,6 +1546,12 @@ proc generateCAbiLibHeader*( ) lines.add(" terminal RET_OK/RET_ERR. Ignore it unless you want progress. */") lines.add("#define NIMFFI_RET_STALE_WARN 3") + # Unprefixed aliases for consumers that spell the codes RET_*; guarded so a + # co-included header that also defines them doesn't clash. + for suffix in ["OK", "ERR", "MISSING_CALLBACK", "STALE_WARN"]: + lines.add("#ifndef RET_" & suffix) + lines.add("#define RET_" & suffix & " NIMFFI_RET_" & suffix) + lines.add("#endif") lines.add("") lines.add(constDeclLines(consts)) lines.add( @@ -1573,6 +1609,7 @@ proc generateCBindings*( nimSrcRelPath: string, events: seq[FFIEventMeta] = @[], consts: seq[FFIConstMeta] = @[], + banner: string = "", ) = ## Emits the C binding for `libName`, picking the `abi = c` or CBOR shape. createDir(outputDir) @@ -1580,7 +1617,7 @@ proc generateCBindings*( of ABIFormat.C: writeFile( outputDir / (libName & ".h"), - generateCAbiLibHeader(procs, types, libName, events, consts), + generateCAbiLibHeader(procs, types, libName, events, consts, banner), ) writeFile( outputDir / "CMakeLists.txt", generateCAbiCMakeLists(libName, nimSrcRelPath) @@ -1590,6 +1627,6 @@ proc generateCBindings*( writeFile(outputDir / CborHeaderName, generateCCborHeader()) writeFile( outputDir / (libName & ".h"), - generateCLibHeader(procs, types, libName, events, consts), + generateCLibHeader(procs, types, libName, events, consts, banner), ) writeFile(outputDir / "CMakeLists.txt", generateCCMakeLists(libName, nimSrcRelPath)) diff --git a/ffi/codegen/meta.nim b/ffi/codegen/meta.nim index a421547..19f1a44 100644 --- a/ffi/codegen/meta.nim +++ b/ffi/codegen/meta.nim @@ -87,6 +87,10 @@ var genBindingsEmitted* {.compileTime.}: bool = false # Library-wide default ABI, inherited by each annotation unless it overrides. var currentDefaultABIFormat* {.compileTime.}: ABIFormat = ABIFormat.Cbor +# Optional "generated — do not edit" banner stamped at the top of each generated +# header, set by declareLibrary's `headerBanner` argument (empty = no banner). +var currentHeaderBanner* {.compileTime.}: string = "" + proc abiCodegenImplemented*(fmt: ABIFormat): bool = ## Whether `fmt` has a working proc-dispatch path (both Cbor and C do). fmt in {ABIFormat.Cbor, ABIFormat.C} diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index 27c8fd4..6d33242 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -137,23 +137,14 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = return res -macro declareLibrary*( - libraryName: static[string], - libType: untyped, - defaultABIFormat: static[string] = "cbor", -): untyped = - ## 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 = ..."`. +proc declareLibraryImpl( + libraryName: string, libType: NimNode, abiFmt: ABIFormat, headerBanner: string +): NimNode {.compileTime.} = + ## Shared body behind both `declareLibrary` overloads: records the library-wide + ## defaults and emits the C-exported event ABI on its `FFIContext`. currentLibType = $libType # so handle-receiver `.ffi.` procs can resolve the pool - - let (abiOk, abiFmt) = parseABIFormatName(defaultABIFormat) - if not abiOk: - error( - "declareLibrary: unknown defaultABIFormat '" & defaultABIFormat & - "'; valid values are \"c\" and \"cbor\"" - ) currentDefaultABIFormat = abiFmt + currentHeaderBanner = headerBanner libraryDeclared = true var stmts = newStmtList() @@ -240,3 +231,31 @@ macro declareLibrary*( ) return stmts + +macro declareLibrary*( + libraryName: static[string], + libType: untyped, + defaultABIFormat: static[string] = "cbor", + headerBanner: static[string] = "", +): untyped = + ## 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 = ..."`. `headerBanner`, + ## when set, is stamped as a comment at the top of every generated header. + let (abiOk, abiFmt) = parseABIFormatName(defaultABIFormat) + if not abiOk: + error( + "declareLibrary: unknown defaultABIFormat '" & defaultABIFormat & + "'; valid values are \"c\" and \"cbor\"" + ) + return declareLibraryImpl(libraryName, libType, abiFmt, headerBanner) + +macro declareLibrary*( + libraryName: static[string], + libType: untyped, + defaultABIFormat: static[ABIFormat], + headerBanner: static[string] = "", +): untyped = + ## `ABIFormat` enum overload of `declareLibrary`, so a caller can pass + ## `defaultABIFormat = ABIFormat.C` instead of the `"c"` string. + return declareLibraryImpl(libraryName, libType, defaultABIFormat, headerBanner) diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index d30980c..35ce3dd 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -1944,7 +1944,7 @@ when defined(ffiGenBindings): of "c": generateCBindings( genProcs, ffiTypeRegistry, libName, outDir, srcRel, ffiEventRegistry, - ffiConstRegistry, + ffiConstRegistry, currentHeaderBanner, ) of "cddl": generateCddlBindings(genProcs, ffiTypeRegistry, libName, outDir, srcRel) @@ -1962,6 +1962,17 @@ macro genBindings*( ## root's bottom. -d:targetLang picks languages; emission needs -d:ffiGenBindings. genBindingsEmitted = true + # A ctor allocates a context; without a dtor the caller has no way to release + # the underlying Nim library object, so require the pair (issue #3). + let classified = classifyProcs(ffiProcRegistry) + if classified.ctors.len > 0 and classified.dtor.isNone(): + error( + "genBindings: library '" & classified.ctors[0].libName & + "' declares an {.ffiCtor.} (" & classified.ctors[0].procName & + ") but no {.ffiDtor.}. Add a `proc _destroy(x: ) {.ffiDtor.}` " & + "so the context it builds can be released." + ) + when defined(ffiGenBindings): let libName = deriveLibName(ffiProcRegistry) for rawLang in targetLang.split(','): diff --git a/tests/unit/fixtures/ctor_without_dtor_fixture.nim b/tests/unit/fixtures/ctor_without_dtor_fixture.nim new file mode 100644 index 0000000..366ed77 --- /dev/null +++ b/tests/unit/fixtures/ctor_without_dtor_fixture.nim @@ -0,0 +1,20 @@ +## Must fail: a library declares an {.ffiCtor.} but no {.ffiDtor.}, so genBindings +## rejects it (issue #3) rather than leaving the context with no way to release. + +import ffi, chronos + +type NoDtorLib = object + n: int + +# Stub the importc NimMain declareLibrary emits (plain-exe link). +{.emit: "void libnodtorNimMain(void) {}".} + +declareLibrary("nodtor", NoDtorLib) + +type NoDtorCfg {.ffi.} = object + n: int + +proc nodtor_create*(c: NoDtorCfg): Future[Result[NoDtorLib, string]] {.ffiCtor.} = + return ok(NoDtorLib(n: c.n)) + +genBindings() diff --git a/tests/unit/fixtures/declare_enum_abi_fixture.nim b/tests/unit/fixtures/declare_enum_abi_fixture.nim new file mode 100644 index 0000000..297a90b --- /dev/null +++ b/tests/unit/fixtures/declare_enum_abi_fixture.nim @@ -0,0 +1,28 @@ +## Must compile: declareLibrary accepts the `ABIFormat` enum for defaultABIFormat, +## so `defaultABIFormat = ABIFormat.C` resolves the enum overload rather than the +## `static[string]` one. + +import ffi, chronos +import ffi/codegen/meta + +type EnumLib = object + n: int + +# Stub the importc NimMain declareLibrary emits (plain-exe link). +{.emit: "void libenumabiNimMain(void) {}".} + +declareLibrary("enumabi", EnumLib, defaultABIFormat = ABIFormat.C) + +static: + doAssert currentDefaultABIFormat == ABIFormat.C + +type EnumCfg {.ffi.} = object + n: int + +proc enumabi_create*(c: EnumCfg): Future[Result[EnumLib, string]] {.ffiCtor.} = + return ok(EnumLib(n: c.n)) + +proc enumabi_destroy*(lib: EnumLib) {.ffiDtor.} = + discard + +genBindings() diff --git a/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim b/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim index 9d9e3d2..e5264e4 100644 --- a/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim +++ b/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim @@ -19,4 +19,7 @@ proc staticscalarCreate*( proc staticscalarAdd*(a: int, b: int): Future[Result[int, string]] {.ffiStatic.} = return ok(a + b) +proc staticscalar_destroy*(lib: ScalarLib) {.ffiDtor.} = + discard + genBindings() diff --git a/tests/unit/fixtures/scalar_skip_fixture.nim b/tests/unit/fixtures/scalar_skip_fixture.nim index 8322981..5eb0d7b 100644 --- a/tests/unit/fixtures/scalar_skip_fixture.nim +++ b/tests/unit/fixtures/scalar_skip_fixture.nim @@ -23,4 +23,7 @@ proc scalarskip_add*( ## library. return ok(lib.base + a + b) +proc scalarskip_destroy*(lib: SkipLib) {.ffiDtor.} = + discard + genBindings() diff --git a/tests/unit/test_c_abi_dispatch.nim b/tests/unit/test_c_abi_dispatch.nim index a3bcb02..6f2ff64 100644 --- a/tests/unit/test_c_abi_dispatch.nim +++ b/tests/unit/test_c_abi_dispatch.nim @@ -51,6 +51,9 @@ proc wirefast_greet*( return err("no items") return ok(lib.tag & " greets " & req.items[0]) +proc wirefast_destroy*(lib: WireLib) {.ffiDtor.} = + discard + genBindings() type ReplyData = object diff --git a/tests/unit/test_c_codegen.nim b/tests/unit/test_c_codegen.nim index c8504ae..f02c3c3 100644 --- a/tests/unit/test_c_codegen.nim +++ b/tests/unit/test_c_codegen.nim @@ -373,6 +373,90 @@ suite "generateCLibHeader: scalar-fast-path procs are excluded": check "int calc_add(void* ctx, FFICallback callback, void* user_data, " & "const uint8_t* req_cbor, size_t req_cbor_len);" in header +suite "generateCAbiLibHeader: self-contained header": + setup: + let procs = @[ + FFIProcMeta( + procName: "widget_create", + libName: "widget", + kind: FFIKind.CTOR, + libTypeName: "Widget", + extraParams: @[param("config", "Cfg")], + returnTypeName: "Widget", + ), + FFIProcMeta( + procName: "widget_poke", + libName: "widget", + kind: FFIKind.FFI, + libTypeName: "Widget", + extraParams: @[param("req", "Cfg")], + returnTypeName: "Cfg", + ), + FFIProcMeta( + procName: "widget_destroy", + libName: "widget", + kind: FFIKind.DTOR, + libTypeName: "Widget", + returnTypeName: "", + ), + ] + let types = @[FFITypeMeta(name: "Cfg", fields: @[field("tag", "string")])] + let header = generateCAbiLibHeader(procs, types, "widget") + + test "the header is self-contained: libc includes and NIMFFI_RET_* codes": + check "#include " in header + check "#include " in header + check "#define NIMFFI_RET_OK 0" in header + check "#define NIMFFI_RET_STALE_WARN 3" in header + + test "short RET_* aliases are emitted, each #ifndef-guarded": + check "#ifndef RET_OK\n#define RET_OK NIMFFI_RET_OK\n#endif" in header + check "#ifndef RET_ERR\n#define RET_ERR NIMFFI_RET_ERR\n#endif" in header + check "#define RET_MISSING_CALLBACK NIMFFI_RET_MISSING_CALLBACK" in header + check "#define RET_STALE_WARN NIMFFI_RET_STALE_WARN" in header + + test "the event-listener ABI and FFICallback are declared": + check "typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_data);" in + header + check "uint64_t widget_add_event_listener(void* ctx, const char* event_name, " & + "FFICallback callback, void* user_data);" in header + check "int widget_remove_event_listener(void* ctx, uint64_t listener_id);" in header + + test "the callback typedef matches the CBOR header's spelling, not the Nim symbol": + check "FFICallBack" notin header # the Nim symbol; the C header uses FFICallback + + test "the FFICallback typedef is include-guarded against co-inclusion": + check "#ifndef NIMFFI_FFICALLBACK_DEFINED" in header + + test "no banner is emitted when none is requested": + check not header.startsWith("//") + + test "a header banner is stamped above the include guard as // lines": + let banner = "GENERATED FILE — do not edit.\nRegenerate with nimble genbindings." + let withBanner = generateCAbiLibHeader(procs, types, "widget", banner = banner) + check withBanner.startsWith( + "// GENERATED FILE — do not edit.\n// Regenerate with nimble genbindings.\n#ifndef " + ) + + test "a banner line ending in a backslash cannot splice the include guard away": + let withBanner = + generateCAbiLibHeader(procs, types, "widget", banner = "edit me and lose\\") + check "// edit me and lose\n#ifndef " in withBanner + +suite "generateCLibHeader: header banner": + test "the CBOR lib header also stamps the banner above its include guard": + let procs = @[ + FFIProcMeta( + procName: "timer_create", + libName: "timer", + kind: FFIKind.CTOR, + libTypeName: "Timer", + returnTypeName: "Timer", + ) + ] + let header = generateCLibHeader(procs, @[], "timer", banner = "do not edit") + check header.startsWith("// do not edit\n#ifndef ") + suite "shared headers: prelude and cbor split": test "the prelude owns the leaf types and libc/TinyCBOR includes": let prelude = generateCPreludeHeader() diff --git a/tests/unit/test_declare_library.nim b/tests/unit/test_declare_library.nim new file mode 100644 index 0000000..f5c618a --- /dev/null +++ b/tests/unit/test_declare_library.nim @@ -0,0 +1,34 @@ +## Asserts the two `declareLibrary`/`genBindings` contracts that hold at macro +## time: the `ABIFormat` enum overload compiles, and an {.ffiCtor.} with no +## {.ffiDtor.} fails the build. Each fixture compiles in a child `nim check` so +## its expected result 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_declare_library_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 "declareLibrary accepts the ABIFormat enum overload": + test "defaultABIFormat = ABIFormat.C compiles and sets the library default": + let (output, code) = checkFixture("declare_enum_abi") + check code == 0 + check not output.contains("Error") + +suite "genBindings requires a dtor when a ctor is declared": + test "an {.ffiCtor.} with no {.ffiDtor.} fails, naming the ctor and the fix": + let (output, code) = checkFixture("ctor_without_dtor") + check code != 0 + check output.contains("nodtor_create") + check output.contains("ffiDtor")