chore: abi improvements

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 (`<lib>_add_event_listener` /
+  `<lib>_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 `<stdint.h>` /
+  `<stddef.h>` 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 (`<WireNamePascalCase>Payload`) whose fields are
   the parameters and dispatches an instance of it, so multi-field events no

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" */

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 `<lib>.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 <stdint.h>")
@@ -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))

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}

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)

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 <lib>_destroy(x: <LibType>) {.ffiDtor.}` " &
+        "so the context it builds can be released."
+    )
+
   when defined(ffiGenBindings):
     let libName = deriveLibName(ffiProcRegistry)
     for rawLang in targetLang.split(','):

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()

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()

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()

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()

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

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 <stdint.h>" in header
+    check "#include <stddef.h>" 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()

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")