From 7e7632a8e9b06f490e2918aa7594a16306211dea Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Sat, 11 Jul 2026 01:07:04 +0200 Subject: [PATCH] refactor(codegen): name RET status codes in C++/Rust callbacks Avoid magic ret-code integers in the generated callback helpers. The C trampolines already reference NIMFFI_RET_STALE_WARN; mirror that in the other two backends: define the canonical NIMFFI_RET_* set once in the C++ header prelude (guarded so a mixed TU keeps a single definition) and emit module-level NIMFFI_RET_* consts in the Rust crate, then use the named constants for the stale-warn, ok, and missing-callback checks instead of 3/0/2. Co-Authored-By: Claude Opus 4.8 --- examples/echo/cpp_bindings/echo.hpp | 16 +++++++++++++--- examples/timer/cpp_bindings/my_timer.hpp | 16 +++++++++++++--- examples/timer/rust_bindings/src/api.rs | 13 +++++++++---- ffi/codegen/rust.nim | 13 +++++++++---- ffi/codegen/templates/cpp/header_prelude.hpp.tpl | 10 ++++++++++ .../templates/cpp/sync_call_helper.hpp.tpl | 6 +++--- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/examples/echo/cpp_bindings/echo.hpp b/examples/echo/cpp_bindings/echo.hpp index 2ee4453..4c47cf3 100644 --- a/examples/echo/cpp_bindings/echo.hpp +++ b/examples/echo/cpp_bindings/echo.hpp @@ -29,6 +29,16 @@ extern "C" { #include } +// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C +// header). Guarded so a translation unit that also pulls in the C header keeps +// a single definition. +#ifndef NIMFFI_RET_OK +#define NIMFFI_RET_OK 0 +#define NIMFFI_RET_ERR 1 +#define NIMFFI_RET_MISSING_CALLBACK 2 +#define NIMFFI_RET_STALE_WARN 3 +#endif + // ============================================================ // Result — exception-free error channel // ============================================================ @@ -452,7 +462,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { // still running. This blocking wrapper only reports the final result, so // ignore it WITHOUT touching `ud` — a terminal callback still owns the // shared handle and will free it. - if (ret == 3) return; + if (ret == NIMFFI_RET_STALE_WARN) return; // ffi_call_ heap-allocated a shared_ptr and passed its address as ud; // take ownership here so it's freed on every exit path. @@ -461,7 +471,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { FFICallState_& s = **handle; std::lock_guard lock(s.mtx); - s.ok = (ret == 0); + s.ok = (ret == NIMFFI_RET_OK); if (msg && len > 0) { const auto* p = reinterpret_cast(msg); if (s.ok) s.bytes.assign(p, p + len); @@ -478,7 +488,7 @@ inline Result> ffi_call_( auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); - if (ret == 2) { + if (ret == NIMFFI_RET_MISSING_CALLBACK) { delete cb_ref; return Result::err("RET_MISSING_CALLBACK (internal error)"); } diff --git a/examples/timer/cpp_bindings/my_timer.hpp b/examples/timer/cpp_bindings/my_timer.hpp index 64f86ec..9190cfb 100644 --- a/examples/timer/cpp_bindings/my_timer.hpp +++ b/examples/timer/cpp_bindings/my_timer.hpp @@ -29,6 +29,16 @@ extern "C" { #include } +// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C +// header). Guarded so a translation unit that also pulls in the C header keeps +// a single definition. +#ifndef NIMFFI_RET_OK +#define NIMFFI_RET_OK 0 +#define NIMFFI_RET_ERR 1 +#define NIMFFI_RET_MISSING_CALLBACK 2 +#define NIMFFI_RET_STALE_WARN 3 +#endif + #include // ============================================================ // Result — exception-free error channel @@ -752,7 +762,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { // still running. This blocking wrapper only reports the final result, so // ignore it WITHOUT touching `ud` — a terminal callback still owns the // shared handle and will free it. - if (ret == 3) return; + if (ret == NIMFFI_RET_STALE_WARN) return; // ffi_call_ heap-allocated a shared_ptr and passed its address as ud; // take ownership here so it's freed on every exit path. @@ -761,7 +771,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { FFICallState_& s = **handle; std::lock_guard lock(s.mtx); - s.ok = (ret == 0); + s.ok = (ret == NIMFFI_RET_OK); if (msg && len > 0) { const auto* p = reinterpret_cast(msg); if (s.ok) s.bytes.assign(p, p + len); @@ -778,7 +788,7 @@ inline Result> ffi_call_( auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); - if (ret == 2) { + if (ret == NIMFFI_RET_MISSING_CALLBACK) { delete cb_ref; return Result::err("RET_MISSING_CALLBACK (internal error)"); } diff --git a/examples/timer/rust_bindings/src/api.rs b/examples/timer/rust_bindings/src/api.rs index 09babee..829113b 100644 --- a/examples/timer/rust_bindings/src/api.rs +++ b/examples/timer/rust_bindings/src/api.rs @@ -31,10 +31,15 @@ unsafe fn ffi_payload(ret: c_int, msg: *const c_char, len: usize) -> FFIResult { } else { slice::from_raw_parts(msg as *const u8, len).to_vec() }; - if ret == 0 { Ok(bytes) } + if ret == NIMFFI_RET_OK { Ok(bytes) } else { Err(String::from_utf8_lossy(&bytes).into_owned()) } } +// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim). +const NIMFFI_RET_OK: c_int = 0; +const NIMFFI_RET_MISSING_CALLBACK: c_int = 2; +const NIMFFI_RET_STALE_WARN: c_int = 3; + unsafe extern "C" fn on_result( ret: c_int, msg: *const c_char, @@ -44,7 +49,7 @@ unsafe extern "C" fn on_result( // NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request // is still running. This wrapper only delivers the final result, so ignore // it WITHOUT reclaiming the box — a terminal callback still owns the Sender. - if ret == 3 { return; } + if ret == NIMFFI_RET_STALE_WARN { return; } // Take ownership of the boxed Sender — dropping it at end of scope // releases the only outstanding handle. @@ -71,7 +76,7 @@ where let (tx, rx) = flume::bounded::(1); let raw = Box::into_raw(Box::new(tx)) as *mut c_void; let ret = f(on_result, raw); - if ret == 2 { + if ret == NIMFFI_RET_MISSING_CALLBACK { // Callback will never fire; reclaim the box to avoid a leak. drop(unsafe { Box::from_raw(raw as *mut FFISender) }); return Err("RET_MISSING_CALLBACK (internal error)".into()); @@ -92,7 +97,7 @@ where let (tx, rx) = flume::bounded::(1); let raw = Box::into_raw(Box::new(tx)) as *mut c_void; let ret = f(on_result, raw); - if ret == 2 { + if ret == NIMFFI_RET_MISSING_CALLBACK { drop(unsafe { Box::from_raw(raw as *mut FFISender) }); return Err("RET_MISSING_CALLBACK (internal error)".into()); } diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 342a7bf..671ecc0 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -368,10 +368,15 @@ proc generateApiRs*( lines.add(" } else {") lines.add(" slice::from_raw_parts(msg as *const u8, len).to_vec()") lines.add(" };") - lines.add(" if ret == 0 { Ok(bytes) }") + lines.add(" if ret == NIMFFI_RET_OK { Ok(bytes) }") lines.add(" else { Err(String::from_utf8_lossy(&bytes).into_owned()) }") lines.add("}") lines.add("") + lines.add("// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim).") + lines.add("const NIMFFI_RET_OK: c_int = 0;") + lines.add("const NIMFFI_RET_MISSING_CALLBACK: c_int = 2;") + lines.add("const NIMFFI_RET_STALE_WARN: c_int = 3;") + lines.add("") lines.add("unsafe extern \"C\" fn on_result(") lines.add(" ret: c_int,") lines.add(" msg: *const c_char,") @@ -387,7 +392,7 @@ proc generateApiRs*( lines.add( " // it WITHOUT reclaiming the box — a terminal callback still owns the Sender." ) - lines.add(" if ret == 3 { return; }") + lines.add(" if ret == NIMFFI_RET_STALE_WARN { return; }") lines.add("") lines.add(" // Take ownership of the boxed Sender — dropping it at end of scope") lines.add(" // releases the only outstanding handle.") @@ -430,7 +435,7 @@ proc generateApiRs*( lines.add(" let (tx, rx) = flume::bounded::(1);") lines.add(" let raw = Box::into_raw(Box::new(tx)) as *mut c_void;") lines.add(" let ret = f(on_result, raw);") - lines.add(" if ret == 2 {") + lines.add(" if ret == NIMFFI_RET_MISSING_CALLBACK {") lines.add(" // Callback will never fire; reclaim the box to avoid a leak.") lines.add(" drop(unsafe { Box::from_raw(raw as *mut FFISender) });") lines.add(" return Err(\"RET_MISSING_CALLBACK (internal error)\".into());") @@ -453,7 +458,7 @@ proc generateApiRs*( lines.add(" let (tx, rx) = flume::bounded::(1);") lines.add(" let raw = Box::into_raw(Box::new(tx)) as *mut c_void;") lines.add(" let ret = f(on_result, raw);") - lines.add(" if ret == 2 {") + lines.add(" if ret == NIMFFI_RET_MISSING_CALLBACK {") lines.add(" drop(unsafe { Box::from_raw(raw as *mut FFISender) });") lines.add(" return Err(\"RET_MISSING_CALLBACK (internal error)\".into());") lines.add(" }") diff --git a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl index 66029b6..91713ef 100644 --- a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl +++ b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl @@ -28,3 +28,13 @@ extern "C" { #include } + +// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C +// header). Guarded so a translation unit that also pulls in the C header keeps +// a single definition. +#ifndef NIMFFI_RET_OK +#define NIMFFI_RET_OK 0 +#define NIMFFI_RET_ERR 1 +#define NIMFFI_RET_MISSING_CALLBACK 2 +#define NIMFFI_RET_STALE_WARN 3 +#endif diff --git a/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl b/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl index 8d0409a..8229383 100644 --- a/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl +++ b/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl @@ -21,7 +21,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { // still running. This blocking wrapper only reports the final result, so // ignore it WITHOUT touching `ud` — a terminal callback still owns the // shared handle and will free it. - if (ret == 3) return; + if (ret == NIMFFI_RET_STALE_WARN) return; // ffi_call_ heap-allocated a shared_ptr and passed its address as ud; // take ownership here so it's freed on every exit path. @@ -30,7 +30,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { FFICallState_& s = **handle; std::lock_guard lock(s.mtx); - s.ok = (ret == 0); + s.ok = (ret == NIMFFI_RET_OK); if (msg && len > 0) { const auto* p = reinterpret_cast(msg); if (s.ok) s.bytes.assign(p, p + len); @@ -47,7 +47,7 @@ inline Result> ffi_call_( auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); - if (ret == 2) { + if (ret == NIMFFI_RET_MISSING_CALLBACK) { delete cb_ref; return Result::err("RET_MISSING_CALLBACK (internal error)"); }