From 16dc1b3573415274b3e6edcda7ab75ef15a97385 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz <8129788+gmelodie@users.noreply.github.com> Date: Wed, 10 Jun 2026 16:30:30 -0300 Subject: [PATCH] chore(ci): add nph linting (#77) --- .github/workflows/linters.yml | 27 ++++++++ config.nims | 2 +- examples/echo/echo.nimble | 6 +- examples/timer/timer.nimble | 3 +- ffi.nimble | 107 +++++++++++++---------------- ffi/cbor_serial.nim | 1 - ffi/codegen/cpp.nim | 54 ++++++++------- ffi/codegen/rust.nim | 41 +++++------ ffi/event_thread.nim | 9 +-- ffi/ffi_context.nim | 10 +-- ffi/ffi_events.nim | 28 +++----- ffi/ffi_thread.nim | 3 +- ffi/internal/ffi_library.nim | 6 +- ffi/internal/ffi_macro.nim | 21 ++---- tests/e2e/cpp/CMakeLists.txt | 13 +++- tests/unit/test_event_dispatch.nim | 46 +++++-------- tests/unit/test_event_thread.nim | 19 ++--- tests/unit/test_ffi_context.nim | 2 - tests/unit/test_serial.nim | 14 ++-- 19 files changed, 204 insertions(+), 208 deletions(-) create mode 100644 .github/workflows/linters.yml diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml new file mode 100644 index 0000000..cfd3aff --- /dev/null +++ b/.github/workflows/linters.yml @@ -0,0 +1,27 @@ +name: Linters + +on: + pull_request: + merge_group: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + nph: + name: nph + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 2 # In PR, has extra merge commit: ^1 = PR, ^2 = base + + - name: Check nph formatting + uses: arnetheduck/nph-action@v1 + with: + version: 0.7.0 + options: "./. *.nim*" + fail: true + suggest: true diff --git a/config.nims b/config.nims index be54fdf..540cf13 100644 --- a/config.nims +++ b/config.nims @@ -6,4 +6,4 @@ switch("path", thisDir()) --noNimblePath when withDir(thisDir(), system.fileExists("nimble.paths")): include "nimble.paths" -# end Nimble config \ No newline at end of file +# end Nimble config diff --git a/examples/echo/echo.nimble b/examples/echo/echo.nimble index 7dd665a..013a49e 100644 --- a/examples/echo/echo.nimble +++ b/examples/echo/echo.nimble @@ -1,7 +1,8 @@ version = "0.1.0" packageName = "echo" author = "Institute of Free Technology" -description = "Second nim-ffi example library, used as the cross-library partner of the timer example in C++ e2e tests" +description = + "Second nim-ffi example library, used as the cross-library partner of the timer example in C++ e2e tests" license = "MIT or Apache License 2.0" requires "nim >= 2.2.4" @@ -13,8 +14,7 @@ requires "https://github.com/logos-messaging/nim-ffi >= 0.2.0" const nimFlags = "--mm:orc -d:chronicles_log_level=WARN" task build, "Compile the echo library": - exec "nim c " & nimFlags & - " --app:lib --noMain --nimMainPrefix:libecho echo.nim" + exec "nim c " & nimFlags & " --app:lib --noMain --nimMainPrefix:libecho echo.nim" task genbindings_cpp, "Generate C++ bindings for the echo example": exec "nim c " & nimFlags & " --app:lib --noMain --nimMainPrefix:libecho" & diff --git a/examples/timer/timer.nimble b/examples/timer/timer.nimble index 58efbc7..0b1966d 100644 --- a/examples/timer/timer.nimble +++ b/examples/timer/timer.nimble @@ -13,8 +13,7 @@ requires "https://github.com/logos-messaging/nim-ffi >= 0.2.0" const nimFlags = "--mm:orc -d:chronicles_log_level=WARN" task build, "Compile the timer library": - exec "nim c " & nimFlags & - " --app:lib --noMain --nimMainPrefix:libmy_timer timer.nim" + exec "nim c " & nimFlags & " --app:lib --noMain --nimMainPrefix:libmy_timer timer.nim" task genbindings_rust, "Generate Rust bindings for the timer example": exec "nim c " & nimFlags & " --app:lib --noMain --nimMainPrefix:libmy_timer" & diff --git a/ffi.nimble b/ffi.nimble index d52a8ac..12c9f3a 100644 --- a/ffi.nimble +++ b/ffi.nimble @@ -56,17 +56,12 @@ proc sanFlags(san: string): string = of "none", "": "" of "asan-ubsan": - " --passC:-fsanitize=address,undefined" & - " --passC:-fno-sanitize-recover=all" & - " --passC:-fno-omit-frame-pointer" & - " --passC:-g" & - " --passL:-fsanitize=address,undefined" + " --passC:-fsanitize=address,undefined" & " --passC:-fno-sanitize-recover=all" & + " --passC:-fno-omit-frame-pointer" & " --passC:-g" & + " --passL:-fsanitize=address,undefined" of "tsan": - " --passC:-fsanitize=thread" & - " --passC:-fno-omit-frame-pointer" & - " --passC:-g" & - " --passC:-O1" & - " --passL:-fsanitize=thread" + " --passC:-fsanitize=thread" & " --passC:-fno-omit-frame-pointer" & " --passC:-g" & + " --passC:-O1" & " --passL:-fsanitize=thread" else: raise newException(ValueError, "unknown NIM_FFI_SAN: " & san) @@ -95,17 +90,24 @@ task test_cpp_e2e, "Build and run the C++ end-to-end tests for the timer example runOrQuit "nimble genbindings_cpp" runOrQuit "nimble genbindings_cpp_echo" runOrQuit "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" - runOrQuit "cmake --build tests/e2e/cpp/build" - runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure" + runOrQuit "cmake --build tests/e2e/cpp/build --config Debug" + # `-C Debug` is required on Windows multi-config generators because + # gtest_discover_tests(PRE_TEST) loads per-config include files; harmless on + # single-config generators (Make/Ninja) on Linux/macOS. + runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure -C Debug" -task test_sanitized, "Run all unit tests under a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": +task test_sanitized, + "Run all unit tests under a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": let san = getEnv("NIM_FFI_SAN", "none") - let mm = getEnv("NIM_FFI_MM", "") + let mm = getEnv("NIM_FFI_MM", "") let extra = sanFlags(san) let modes = - if mm == "orc": @[nimFlagsOrc] - elif mm == "refc": @[nimFlagsRefc] - else: @[nimFlagsOrc, nimFlagsRefc] + if mm == "orc": + @[nimFlagsOrc] + elif mm == "refc": + @[nimFlagsRefc] + else: + @[nimFlagsOrc, nimFlagsRefc] if san == "tsan": let suppPath = thisDir() & "/tsan.supp" let existing = getEnv("TSAN_OPTIONS") @@ -117,82 +119,67 @@ task test_sanitized, "Run all unit tests under a sanitizer (NIM_FFI_SAN) and mm for t in unitTests: exec "nim c -r " & flags & extra & " tests/unit/" & t & ".nim" -task test_cpp_e2e_sanitized, "Build and run the C++ e2e tests with a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": - let mm = getEnv("NIM_FFI_MM", "orc") +task test_cpp_e2e_sanitized, + "Build and run the C++ e2e tests with a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": + let mm = getEnv("NIM_FFI_MM", "orc") let san = getEnv("NIM_FFI_SAN", "none") runOrQuit "nimble genbindings_cpp" runOrQuit "nimble genbindings_cpp_echo" - runOrQuit "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" & - " -DNIM_FFI_MM=" & mm & - " -DNIM_FFI_SANITIZER=" & san - runOrQuit "cmake --build tests/e2e/cpp/build -j" - runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure" + runOrQuit "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" & " -DNIM_FFI_MM=" & mm & + " -DNIM_FFI_SANITIZER=" & san + runOrQuit "cmake --build tests/e2e/cpp/build --config Debug -j" + runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure -C Debug" task genbindings_example, "Generate Rust bindings for the timer example": - exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libmy_timer -d:ffiGenBindings -o:/dev/null examples/timer/timer.nim" - exec "nim c " & nimFlagsRefc & " --app:lib --noMain --nimMainPrefix:libmy_timer -d:ffiGenBindings -o:/dev/null examples/timer/timer.nim" + exec "nim c " & nimFlagsOrc & + " --app:lib --noMain --nimMainPrefix:libmy_timer -d:ffiGenBindings -o:/dev/null examples/timer/timer.nim" + exec "nim c " & nimFlagsRefc & + " --app:lib --noMain --nimMainPrefix:libmy_timer -d:ffiGenBindings -o:/dev/null examples/timer/timer.nim" task genbindings_rust, "Generate Rust bindings for the timer example": - exec "nim c " & nimFlagsOrc & - " --app:lib --noMain --nimMainPrefix:libmy_timer" & + exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libmy_timer" & " -d:ffiGenBindings -d:targetLang=rust" & - " -d:ffiOutputDir=examples/timer/rust_bindings" & - " -d:ffiSrcPath=../timer.nim" & + " -d:ffiOutputDir=examples/timer/rust_bindings" & " -d:ffiSrcPath=../timer.nim" & " -o:/dev/null examples/timer/timer.nim" - exec "nim c " & nimFlagsRefc & - " --app:lib --noMain --nimMainPrefix:libmy_timer" & + exec "nim c " & nimFlagsRefc & " --app:lib --noMain --nimMainPrefix:libmy_timer" & " -d:ffiGenBindings -d:targetLang=rust" & - " -d:ffiOutputDir=examples/timer/rust_bindings" & - " -d:ffiSrcPath=../timer.nim" & + " -d:ffiOutputDir=examples/timer/rust_bindings" & " -d:ffiSrcPath=../timer.nim" & " -o:/dev/null examples/timer/timer.nim" task genbindings_cddl, "Generate CDDL schema for the timer example": - exec "nim c " & nimFlagsOrc & - " --app:lib --noMain --nimMainPrefix:libtimer" & + exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libtimer" & " -d:ffiGenBindings -d:targetLang=cddl" & - " -d:ffiOutputDir=examples/timer/cddl_bindings" & - " -d:ffiSrcPath=../timer.nim" & + " -d:ffiOutputDir=examples/timer/cddl_bindings" & " -d:ffiSrcPath=../timer.nim" & " -o:/dev/null examples/timer/timer.nim" task genbindings_cpp, "Generate C++ bindings for the timer example": - exec "nim c " & nimFlagsOrc & - " --app:lib --noMain --nimMainPrefix:libmy_timer" & + exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libmy_timer" & " -d:ffiGenBindings -d:targetLang=cpp" & - " -d:ffiOutputDir=examples/timer/cpp_bindings" & - " -d:ffiSrcPath=../timer.nim" & + " -d:ffiOutputDir=examples/timer/cpp_bindings" & " -d:ffiSrcPath=../timer.nim" & " -o:/dev/null examples/timer/timer.nim" - exec "nim c " & nimFlagsRefc & - " --app:lib --noMain --nimMainPrefix:libmy_timer" & + exec "nim c " & nimFlagsRefc & " --app:lib --noMain --nimMainPrefix:libmy_timer" & " -d:ffiGenBindings -d:targetLang=cpp" & - " -d:ffiOutputDir=examples/timer/cpp_bindings" & - " -d:ffiSrcPath=../timer.nim" & + " -d:ffiOutputDir=examples/timer/cpp_bindings" & " -d:ffiSrcPath=../timer.nim" & " -o:/dev/null examples/timer/timer.nim" task genbindings_cpp_echo, "Generate C++ bindings for the echo example": - exec "nim c " & nimFlagsOrc & - " --app:lib --noMain --nimMainPrefix:libecho" & + exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libecho" & " -d:ffiGenBindings -d:targetLang=cpp" & - " -d:ffiOutputDir=examples/echo/cpp_bindings" & - " -d:ffiSrcPath=../echo.nim" & + " -d:ffiOutputDir=examples/echo/cpp_bindings" & " -d:ffiSrcPath=../echo.nim" & " -o:/dev/null examples/echo/echo.nim" - exec "nim c " & nimFlagsRefc & - " --app:lib --noMain --nimMainPrefix:libecho" & + exec "nim c " & nimFlagsRefc & " --app:lib --noMain --nimMainPrefix:libecho" & " -d:ffiGenBindings -d:targetLang=cpp" & - " -d:ffiOutputDir=examples/echo/cpp_bindings" & - " -d:ffiSrcPath=../echo.nim" & + " -d:ffiOutputDir=examples/echo/cpp_bindings" & " -d:ffiSrcPath=../echo.nim" & " -o:/dev/null examples/echo/echo.nim" task check_bindings_rust, "Verify checked-in Rust bindings match Nim source": exec "nimble genbindings_rust" - exec "git diff --exit-code --" & - " examples/timer/rust_bindings/Cargo.toml" & - " examples/timer/rust_bindings/build.rs" & - " examples/timer/rust_bindings/src" + exec "git diff --exit-code --" & " examples/timer/rust_bindings/Cargo.toml" & + " examples/timer/rust_bindings/build.rs" & " examples/timer/rust_bindings/src" task check_bindings_cpp, "Verify checked-in C++ bindings match Nim source": exec "nimble genbindings_cpp" - exec "git diff --exit-code --" & - " examples/timer/cpp_bindings/my_timer.hpp" & + exec "git diff --exit-code --" & " examples/timer/cpp_bindings/my_timer.hpp" & " examples/timer/cpp_bindings/CMakeLists.txt" task check_bindings, "Verify all checked-in example bindings match Nim source": diff --git a/ffi/cbor_serial.nim b/ffi/cbor_serial.nim index 0fc87ce..54b1f8a 100644 --- a/ffi/cbor_serial.nim +++ b/ffi/cbor_serial.nim @@ -35,7 +35,6 @@ export cbor_serialization, options, results const CborNullByte*: byte = 0xf6'u8 ## CBOR encoding of `null` — used as the wire sentinel for empty OK payloads. - proc cborEncode*[T](x: T): seq[byte] = ## CBOR-encode any cbor_serialization-supported type (plus `pointer` / `ptr T` ## via our custom writers) into a fresh `seq[byte]`. diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index 3401a83..0ae4bff 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -158,7 +158,9 @@ proc emitEventDispatcher( ## until `removeEventListener` removes it. if events.len == 0: return - lines.add(" // ── Event listener API ──────────────────────────────────") + lines.add( + " // ── Event listener API ──────────────────────────────────" + ) lines.add(" struct ListenerHandle { std::uint64_t id = 0; };") lines.add("") # Per-event typed registration helpers. @@ -174,9 +176,7 @@ proc emitEventDispatcher( [ev.payloadTypeName] ) lines.add(" auto* raw = owned.get();") - lines.add( - " const auto id = $1_add_event_listener(" % [libName] - ) + lines.add(" const auto id = $1_add_event_listener(" % [libName]) lines.add( " ptr_, \"$1\", &$2::typedTrampoline<$3>, raw);" % [ev.wireName, ctxTypeName, ev.payloadTypeName] @@ -197,9 +197,7 @@ proc emitEventDispatcher( lines.add(" }") lines.add("") -proc emitEventTrampoline( - lines: var seq[string], events: seq[FFIEventMeta] -) = +proc emitEventTrampoline(lines: var seq[string], events: seq[FFIEventMeta]) = ## Private listener machinery for the public API emitted by ## `emitEventDispatcher`: ## @@ -217,12 +215,16 @@ proc emitEventTrampoline( lines.add(" template ") lines.add(" struct TypedListener : ListenerBase {") lines.add(" std::function fn;") - lines.add(" explicit TypedListener(std::function f) : fn(std::move(f)) {}") + lines.add( + " explicit TypedListener(std::function f) : fn(std::move(f)) {}" + ) lines.add(" };") lines.add("") # Typed trampoline — one instantiation per payload type, all sharing a body. lines.add(" template ") - lines.add(" static void typedTrampoline(int ret, const char* msg, std::size_t len, void* ud) {") + lines.add( + " static void typedTrampoline(int ret, const char* msg, std::size_t len, void* ud) {" + ) lines.add(" if (!ud || ret != 0 || !msg || len == 0) return;") lines.add(" auto* listener = static_cast*>(ud);") lines.add(" if (!listener->fn) return;") @@ -236,9 +238,7 @@ proc emitEventTrampoline( " if (cbor_value_map_find_value(&it, \"payload\", &payloadField) != CborNoError) return;" ) lines.add(" T payload{};") - lines.add( - " if (decode_cbor(payloadField, payload) != CborNoError) return;" - ) + lines.add(" if (decode_cbor(payloadField, payload) != CborNoError) return;") lines.add(" listener->fn(payload);") lines.add(" }") lines.add("") @@ -416,12 +416,12 @@ proc generateCppHeader*( # itself (see ContextRuleOf5Tpl) and hand out ownership through a # smart pointer that callers can move, store in containers, etc. let createRet = "Result>" % [ctxTypeName] - lines.add( - " static $1 create($2) {" % [createRet, ctorParamsWithTimeout] - ) + lines.add(" static $1 create($2) {" % [createRet, ctorParamsWithTimeout]) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) lines.add(" auto ffi_enc_ = encodeCborFFI(ffi_req_);") - lines.add(" if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [createRet]) + lines.add( + " if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [createRet] + ) lines.add(" const auto& ffi_req_bytes_ = ffi_enc_.value();") lines.add(" auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") lines.add( @@ -430,9 +430,13 @@ proc generateCppHeader*( ) lines.add(" return 0;") lines.add(" }, timeout);") - lines.add(" if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [createRet]) + lines.add( + " if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [createRet] + ) lines.add(" auto ffi_addr_ = decodeCborFFI(ffi_raw_.value());") - lines.add(" if (ffi_addr_.isErr()) return $1::err(ffi_addr_.error());" % [createRet]) + lines.add( + " if (ffi_addr_.isErr()) return $1::err(ffi_addr_.error());" % [createRet] + ) lines.add(" const auto& addr_str = ffi_addr_.value();") # Parse the ctx address without exceptions: std::stoull would throw on a # non-numeric payload, so use std::from_chars and surface the failure as @@ -515,7 +519,9 @@ proc generateCppHeader*( lines.add(" $1 $2($3) const {" % [methRet, methodName, methParamsStr]) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) lines.add(" auto ffi_enc_ = encodeCborFFI(ffi_req_);") - lines.add(" if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [methRet]) + lines.add( + " if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [methRet] + ) lines.add(" const auto& ffi_req_bytes_ = ffi_enc_.value();") lines.add(" auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") lines.add( @@ -523,7 +529,9 @@ proc generateCppHeader*( [m.procName] ) lines.add(" }, timeout_);") - lines.add(" if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [methRet]) + lines.add( + " if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [methRet] + ) lines.add(" return decodeCborFFI<$1>(ffi_raw_.value());" % [retCppType]) lines.add(" }") lines.add("") @@ -533,8 +541,7 @@ proc generateCppHeader*( # parse as invoking the `schedule` parameter). if methParamsStr.len > 0: lines.add( - " std::future<$1> $2Async($3) const {" % - [methRet, methodName, methParamsStr] + " std::future<$1> $2Async($3) const {" % [methRet, methodName, methParamsStr] ) lines.add( " return std::async(std::launch::async, [this, $1]() { return this->$2($3); });" % @@ -591,7 +598,6 @@ proc generateCppBindings*( ) = createDir(outputDir) writeFile( - outputDir / (libName & ".hpp"), - generateCppHeader(procs, types, libName, events), + outputDir / (libName & ".hpp"), generateCppHeader(procs, types, libName, events) ) writeFile(outputDir / "CMakeLists.txt", generateCppCMakeLists(libName, nimSrcRelPath)) diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index c8d278e..a2acb17 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -63,7 +63,6 @@ proc reqStructName(p: FFIProcMeta): string = else: camel & "Req" - proc generateCargoToml*(libName: string): string = # `flume` is the unified callback channel (PR #23 Rust review, item 8): one # primitive that supports both `recv_timeout` (blocking trampoline) and @@ -458,9 +457,7 @@ proc generateApiRs*( let handlerStruct = capitalizeFirstLetter(ev.nimProcName) & "Handler" let trampolineName = camelToSnakeCase(ev.nimProcName) & "_trampoline" lines.add("struct $1 {" % [handlerStruct]) - lines.add( - " f: Box," % [ev.payloadTypeName] - ) + lines.add(" f: Box," % [ev.payloadTypeName]) lines.add("}") lines.add("") lines.add("unsafe extern \"C\" fn $1(" % [trampolineName]) @@ -472,9 +469,7 @@ proc generateApiRs*( lines.add(" let h = &*(ud as *const $1);" % [handlerStruct]) lines.add(" let bytes = slice::from_raw_parts(msg as *const u8, len);") lines.add(" #[derive(serde::Deserialize)]") - lines.add( - " struct Envelope { payload: $1 }" % [ev.payloadTypeName] - ) + lines.add(" struct Envelope { payload: $1 }" % [ev.payloadTypeName]) lines.add( " if let Ok(env) = ciborium::de::from_reader::(bytes) {" ) @@ -597,7 +592,9 @@ proc generateApiRs*( " let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?;" ) if events.len > 0: - lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })") + lines.add( + " Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })" + ) else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") @@ -623,7 +620,9 @@ proc generateApiRs*( " let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?;" ) if events.len > 0: - lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })") + lines.add( + " Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })" + ) else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") @@ -666,19 +665,16 @@ proc generateApiRs*( [ev.wireName] ) lines.add(" /// passed to `remove_event_listener` to unregister.") - lines.add( - " pub fn $1(&self, handler: F) -> ListenerHandle" % [methodName] - ) - lines.add( - " where F: Fn(&$1) + Send + Sync + 'static," % [ev.payloadTypeName] - ) + lines.add(" pub fn $1(&self, handler: F) -> ListenerHandle" % [methodName]) + lines.add(" where F: Fn(&$1) + Send + Sync + 'static," % [ev.payloadTypeName]) lines.add(" {") lines.add( " let owned: Box<$1> = Box::new($1 { f: Box::new(handler) });" % [handlerStruct] ) - lines.add(" let raw = &*owned as *const $1 as *mut c_void;" % - [handlerStruct]) + lines.add( + " let raw = &*owned as *const $1 as *mut c_void;" % [handlerStruct] + ) lines.add( " self.add_listener_inner(b\"$1\\0\".as_ptr() as *const c_char, $2, raw, owned)" % [ev.wireName, trampolineName] @@ -688,20 +684,15 @@ proc generateApiRs*( # Remove by handle. Drops the Box (and the user's closure) after the # C ABI confirms the listener has been unregistered. - lines.add( - " /// Remove a previously-registered listener by handle. Returns true" - ) - lines.add( - " /// if the listener existed and was removed; false otherwise." - ) + lines.add(" /// Remove a previously-registered listener by handle. Returns true") + lines.add(" /// if the listener existed and was removed; false otherwise.") lines.add( " pub fn remove_event_listener(&self, handle: ListenerHandle) -> bool {" ) lines.add(" if handle.id == 0 { return false; }") lines.add(" let rc = unsafe {") lines.add( - " ffi::$1_remove_event_listener(self.ptr, handle.id)" % - [libName] + " ffi::$1_remove_event_listener(self.ptr, handle.id)" % [libName] ) lines.add(" };") lines.add(" self.listeners.lock().unwrap().remove(&handle.id);") diff --git a/ffi/event_thread.nim b/ffi/event_thread.nim index bfbb93f..e1037f5 100644 --- a/ffi/event_thread.nim +++ b/ffi/event_thread.nim @@ -46,8 +46,10 @@ proc emitLivenessEvent[T, P](ctx: ptr FFIContext[T], name: string, payload: P) = chronicles.error "liveness event encode failed", name = name, err = e.msg return let dataPtr: pointer = - if event.len > 0: cast[pointer](unsafeAddr event[0]) - else: cast[pointer](emptyListenerPayload) + if event.len > 0: + cast[pointer](unsafeAddr event[0]) + else: + cast[pointer](emptyListenerPayload) ctx.dispatchToListeners(name, dataPtr, event.len) proc onNotResponding*(ctx: ptr FFIContext) = @@ -98,8 +100,7 @@ proc check[T](hb: var HeartbeatMonitor, ctx: ptr FFIContext[T]) = hb.lastValue = cur hb.lastChange = Moment.now() hb.notifiedStale = false - elif not hb.notifiedStale and - Moment.now() - hb.lastChange > FFIHeartbeatStaleThreshold: + elif not hb.notifiedStale and Moment.now() - hb.lastChange > FFIHeartbeatStaleThreshold: onNotResponding(ctx) hb.notifiedStale = true diff --git a/ffi/ffi_context.nim b/ffi/ffi_context.nim index 4ec7c9d..3fa50cf 100644 --- a/ffi/ffi_context.nim +++ b/ffi/ffi_context.nim @@ -22,14 +22,16 @@ type FFIContext*[T] = object reqReceivedSignal: ThreadSignalPtr stopSignal: ThreadSignalPtr threadExitSignal: ThreadSignalPtr - eventQueueSignal: ThreadSignalPtr - eventThreadExitSignal: ThreadSignalPtr + # bounds destroyFFIContext's wait so a blocked loop cannot hang the caller + eventQueueSignal: ThreadSignalPtr # wakes the event thread on enqueue + eventThreadExitSignal: ThreadSignalPtr # mirrors threadExitSignal for the event thread userData*: pointer eventRegistry*: FFIEventRegistry eventQueue*: EventQueue - ffiHeartbeat*: Atomic[int64] # advanced each FFI-thread loop; event thread reads for liveness + ffiHeartbeat*: Atomic[int64] + # advanced each FFI-thread loop; event thread reads for liveness eventQueueStuck*: Atomic[bool] # sticky overflow flag - running: Atomic[bool] + running: Atomic[bool] # To control when the threads are running registeredRequests: ptr Table[cstring, FFIRequestProc] var onFFIThread* {.threadvar.}: bool diff --git a/ffi/ffi_events.nim b/ffi/ffi_events.nim index c66ca34..898746c 100644 --- a/ffi/ffi_events.nim +++ b/ffi/ffi_events.nim @@ -9,13 +9,10 @@ import std/[atomics, locks, sequtils, options, tables] import chronicles import ./ffi_types, ./cbor_serial, ./alloc - -type EventEnvelope*[T] = object - ## CBOR wire shape: { eventType: tstr, payload: }. +type EventEnvelope*[T] = object ## CBOR wire shape: { eventType: tstr, payload: }. eventType*: string payload*: T - type FFIEventListener* = object id*: uint64 @@ -27,7 +24,6 @@ type nextId*: uint64 # 0 is reserved as "invalid"; ids start at 1. byEvent*: Table[string, seq[FFIEventListener]] - proc initEventRegistry*(reg: var FFIEventRegistry) = ## Must run once on the owning thread before sharing — `initLock` on a ## live primitive is UB at the OS layer. @@ -103,7 +99,6 @@ proc snapshotListeners*( listeners.add(l) listeners - const EventQueueCapacity* = 1024 # Sustained backlog at this depth means a listener is wedged. @@ -115,8 +110,7 @@ type data*: ptr UncheckedArray[byte] dataLen*: int - EventQueue* = object - # SPSC ring; plain lock since ops are short and uncontended. + EventQueue* = object # SPSC ring; plain lock since ops are short and uncontended. lock*: Lock head*: int tail*: int @@ -176,7 +170,6 @@ proc eventQueueLen*(q: var EventQueue): int {.raises: [], gcsafe.} = withLock q.lock: return q.count - const emptyListenerPayload*: cstring = "" ## Non-nil zero-length buffer handed to listeners when the payload is empty ## (a nil pointer would be UB for consumers doing `memcpy` even at len 0). @@ -188,15 +181,19 @@ proc notifyListeners*( ## `std::string(data, len)` / `memcpy` never see a nil pointer. let n = max(dataLen, 0) let dataPtr = - if n > 0 and not data.isNil(): cast[ptr cchar](data) - else: cast[ptr cchar](emptyListenerPayload) + if n > 0 and not data.isNil(): + cast[ptr cchar](data) + else: + cast[ptr cchar](emptyListenerPayload) for listener in listeners: listener.callback(retCode, dataPtr, cast[csize_t](n), listener.userData) proc notifyListenersErr*(listeners: seq[FFIEventListener], msg: string) = let p = - if msg.len > 0: cast[pointer](unsafeAddr msg[0]) - else: cast[pointer](emptyListenerPayload) + if msg.len > 0: + cast[pointer](unsafeAddr msg[0]) + else: + cast[pointer](emptyListenerPayload) notifyListeners(listeners, RET_ERR, p, msg.len) var ffiCurrentEventRegistry* {.threadvar.}: ptr FFIEventRegistry @@ -213,10 +210,7 @@ var ffiCurrentNotifyEventEnqueued* {.threadvar.}: proc() {.gcsafe, raises: [].} # Nil-safe; tick-driven tests leave it unset. template enqueueOrMarkStuck( - eventName: string, - namePtr: cstring, - dataPtr: ptr UncheckedArray[byte], - dataLen: int, + eventName: string, namePtr: cstring, dataPtr: ptr UncheckedArray[byte], dataLen: int ) = ## Takes ownership of `namePtr`/`dataPtr`. On queue-full sets the sticky ## stuck flag and wakes the event thread (firing onNotResponding from here diff --git a/ffi/ffi_thread.nim b/ffi/ffi_thread.nim index 94f561d..9685a45 100644 --- a/ffi/ffi_thread.nim +++ b/ffi/ffi_thread.nim @@ -56,7 +56,8 @@ proc processRequest[T]( ## Invoked within the FFI thread to process a request coming from the FFI API consumer thread. let reqId = $request[].reqId - let reqIdCs = reqId.cstring # keeps reqId alive; implicit string→cstring is a warning. + let reqIdCs = reqId.cstring + # keeps reqId alive; implicit string→cstring is a warning. let retFut = if not ctx[].registeredRequests[].contains(reqIdCs): diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index 824f54b..ed04d9e 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -143,7 +143,11 @@ macro declareLibrary*(libraryName: static[string], libType: untyped): untyped = if isNil(ctx): echo `addErr` return ret - let evtName = if eventName.isNil(): "" else: $eventName + let evtName = + if eventName.isNil(): + "" + else: + $eventName ret = addEventListener(ctx[].eventRegistry, evtName, callback, userData) return ret diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index 5d3b605..b8e7be4 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -7,7 +7,6 @@ when defined(ffiGenBindings): import ../codegen/cpp import ../codegen/cddl - proc isPtr(typ: NimNode): bool = ## True iff `typ` is a `ptr T` type expression — i.e. an `nnkPtrTy` AST node. ## Used by the binding-generator metadata path to flag pointer-typed params @@ -597,7 +596,6 @@ macro ffiRaw*(prc: untyped): untyped = echo stmts.repr return stmts - macro ffi*(prc: untyped): untyped = ## Simplified FFI macro — applies to procs or types. ## @@ -837,7 +835,6 @@ macro ffi*(prc: untyped): untyped = echo stmts.repr return stmts - proc buildCtorRequestType( reqTypeName: NimNode, paramNames: seq[string], paramTypes: seq[NimNode] ): NimNode = @@ -1248,7 +1245,6 @@ macro ffiCtor*(prc: untyped): untyped = echo stmts.repr return stmts - macro ffiDtor*(prc: untyped): untyped = ## Defines a C-exported destructor that tears down the FFIContext after the ## body runs. @@ -1361,7 +1357,6 @@ macro ffiDtor*(prc: untyped): untyped = echo stmts.repr return stmts - macro ffiEvent*(wireName: static[string], prc: untyped): untyped = ## Declares a library-initiated event. The annotated proc has an empty ## body — the macro fills it with a `dispatchFFIEventCbor` call so the @@ -1404,8 +1399,10 @@ macro ffiEvent*(wireName: static[string], prc: untyped): untyped = let payloadTypeNameStr = case payloadTypeNode.kind - of nnkIdent: $payloadTypeNode - else: payloadTypeNode.repr + of nnkIdent: + $payloadTypeNode + else: + payloadTypeNode.repr var userProcName = procName if procName.kind == nnkPostfix: @@ -1413,13 +1410,8 @@ macro ffiEvent*(wireName: static[string], prc: untyped): untyped = # The generated body: dispatchFFIEventCbor("wire_name", payload). let wireNameLit = newStrLitNode(wireName) - let dispatchBody = newStmtList( - newCall( - ident("dispatchFFIEventCbor"), - wireNameLit, - payloadParamName, - ) - ) + let dispatchBody = + newStmtList(newCall(ident("dispatchFFIEventCbor"), wireNameLit, payloadParamName)) var newParams = newSeq[NimNode]() newParams.add(formalParams[0]) # return type (typically empty/void) @@ -1452,7 +1444,6 @@ macro ffiEvent*(wireName: static[string], prc: untyped): untyped = echo generated.repr return generated - macro genBindings*( outputDir: static[string] = ffiOutputDir, nimSrcRelPath: static[string] = ffiSrcPath ): untyped = diff --git a/tests/e2e/cpp/CMakeLists.txt b/tests/e2e/cpp/CMakeLists.txt index 7aa3959..e9bd115 100644 --- a/tests/e2e/cpp/CMakeLists.txt +++ b/tests/e2e/cpp/CMakeLists.txt @@ -81,9 +81,18 @@ elseif(NIM_FFI_SANITIZER STREQUAL "tsan") "TSAN_OPTIONS=halt_on_error=1:second_deadlock_stack=1:history_size=7") endif() +# Discover at test time, not build time: a POST_BUILD discovery run launches the +# freshly-linked exe while Windows Defender is still scanning it (and its staged +# DLLs), which routinely overran the default 5s timeout on CI. PRE_TEST defers +# enumeration to `ctest`, and the bumped timeout absorbs first-launch scan delays. include(GoogleTest) if(_san_test_env) - gtest_discover_tests(timer_e2e_tests PROPERTIES ENVIRONMENT "${_san_test_env}") + gtest_discover_tests(timer_e2e_tests + DISCOVERY_MODE PRE_TEST + DISCOVERY_TIMEOUT 120 + PROPERTIES ENVIRONMENT "${_san_test_env}") else() - gtest_discover_tests(timer_e2e_tests) + gtest_discover_tests(timer_e2e_tests + DISCOVERY_MODE PRE_TEST + DISCOVERY_TIMEOUT 120) endif() diff --git a/tests/unit/test_event_dispatch.nim b/tests/unit/test_event_dispatch.nim index 6ff8d08..05005fc 100644 --- a/tests/unit/test_event_dispatch.nim +++ b/tests/unit/test_event_dispatch.nim @@ -79,8 +79,7 @@ template withPool(ctxIdent: untyped, body: untyped) = registerReqFFI(EmitCborEventRequest, lib: ptr TestEvtLib): proc(): Future[Result[string, string]] {.async.} = dispatchFFIEventCbor( - "message_sent", - MessageSentBody(requestId: "req-1", messageHash: "0xdeadbeef"), + "message_sent", MessageSentBody(requestId: "req-1", messageHash: "0xdeadbeef") ) return ok("emitted") @@ -91,16 +90,15 @@ registerReqFFI(EmitRawBytesEventRequest, lib: ptr TestEvtLib): return ok("emitted") # Add/remove worker for the registry-race regression test. -type SetterArgs = tuple - ctx: ptr FFIContext[TestEvtLib] - stop: ptr Atomic[bool] - target: ptr CallbackData +type SetterArgs = + tuple[ + ctx: ptr FFIContext[TestEvtLib], stop: ptr Atomic[bool], target: ptr CallbackData + ] proc setterThreadBody(args: SetterArgs) {.thread.} = while not args.stop[].load(): - let id = addEventListener( - args.ctx[].eventRegistry, "message_sent", captureCb, args.target - ) + let id = + addEventListener(args.ctx[].eventRegistry, "message_sent", captureCb, args.target) discard removeEventListener(args.ctx[].eventRegistry, id) suite "dispatchFFIEventCbor": @@ -111,9 +109,7 @@ suite "dispatchFFIEventCbor": setupCallbackData(rsp) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, "message_sent", captureCb, addr evt - ) + discard addEventListener(ctx[].eventRegistry, "message_sent", captureCb, addr evt) check sendRequestToFFIThread( ctx, EmitCborEventRequest.ffiNewReq(captureCb, addr rsp) @@ -135,9 +131,7 @@ suite "dispatchFFIEvent with seq[byte]": setupCallbackData(rsp) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, "raw_bytes", captureCb, addr evt - ) + discard addEventListener(ctx[].eventRegistry, "raw_bytes", captureCb, addr evt) check sendRequestToFFIThread( ctx, EmitRawBytesEventRequest.ffiNewReq(captureCb, addr rsp) @@ -161,9 +155,8 @@ when not defined(gcRefc): withPool(ctx): # Seed an initial listener so the first dispatch has a target. - discard addEventListener( - ctx[].eventRegistry, "message_sent", captureCb, addr evt - ) + discard + addEventListener(ctx[].eventRegistry, "message_sent", captureCb, addr evt) const NumSetterThreads = 4 const NumDispatchIters = 200 @@ -209,9 +202,8 @@ suite "registry lock held during invocation": st.entered.store(false) st.exited.store(false) - let id = addEventListener( - ctx[].eventRegistry, "message_sent", slowEventCb, addr st - ) + let id = + addEventListener(ctx[].eventRegistry, "message_sent", slowEventCb, addr st) check id != 0'u64 check sendRequestToFFIThread( @@ -250,8 +242,7 @@ suite "liveness events": waitCallback(evt) check evt.retCode == RET_OK - let decoded = - cborDecode(callbackBytes(evt), EventEnvelope[NotRespondingEvent]) + let decoded = cborDecode(callbackBytes(evt), EventEnvelope[NotRespondingEvent]) check decoded.isOk() check decoded.value.eventType == NotRespondingEventName @@ -259,9 +250,8 @@ suite "liveness events": setupCallbackData(evt) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, RespondingEventName, captureCb, addr evt - ) + discard + addEventListener(ctx[].eventRegistry, RespondingEventName, captureCb, addr evt) onResponding(ctx) @@ -289,9 +279,7 @@ suite "event thread drains queued events": withPool(ctx): const QueuedEvtName = "queued_evt" - discard addEventListener( - ctx[].eventRegistry, QueuedEvtName, captureCb, addr evt - ) + discard addEventListener(ctx[].eventRegistry, QueuedEvtName, captureCb, addr evt) # `tryEnqueueEvent` takes ownership of both buffers on success; the # event thread c_frees them after dispatch returns. diff --git a/tests/unit/test_event_thread.nim b/tests/unit/test_event_thread.nim index 51f8829..1b5f252 100644 --- a/tests/unit/test_event_thread.nim +++ b/tests/unit/test_event_thread.nim @@ -130,9 +130,8 @@ suite "event delivery is asynchronous": setupCallbackData(rsp) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, "latch", captureThreadIdCb, addr evt - ) + discard + addEventListener(ctx[].eventRegistry, "latch", captureThreadIdCb, addr evt) check sendRequestToFFIThread( ctx, CaptureFfiTidRequest.ffiNewReq(captureCb, addr rsp) @@ -164,9 +163,7 @@ suite "FFI thread independence": setupCallbackData(rsp) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, "latch", slowSleepCb, nil - ) + discard addEventListener(ctx[].eventRegistry, "latch", slowSleepCb, nil) check sendRequestToFFIThread( ctx, EmitLatchEvent.ffiNewReq(captureCb, addr rsp, 0) @@ -178,8 +175,7 @@ suite "FFI thread independence": # chronos's `Moment` — std/times exports a `milliseconds` that # shadows chronos's at this generic-instantiation site. let started = Moment.now() - check sendRequestToFFIThread(ctx, PingEvent.ffiNewReq(captureCb, addr rsp)) - .isOk() + check sendRequestToFFIThread(ctx, PingEvent.ffiNewReq(captureCb, addr rsp)).isOk() waitCallback(rsp) let elapsed = Moment.now() - started @@ -212,8 +208,7 @@ when not defined(gcRefc): # Wedge long enough to cross at least one tick boundary. gBlockingEnabled.store(true) let wedgeMs = - (EventThreadTickInterval + FFIHeartbeatStaleThreshold).milliseconds.int + - 1500 + (EventThreadTickInterval + FFIHeartbeatStaleThreshold).milliseconds.int + 1500 check sendRequestToFFIThread( ctx, BlockingRequest.ffiNewReq(captureCb, addr rsp, wedgeMs) ) @@ -277,9 +272,7 @@ suite "queue overflow": setupCallbackData(rejected) withPool(ctx): - discard addEventListener( - ctx[].eventRegistry, "latch", backpressureCb, addr bp - ) + discard addEventListener(ctx[].eventRegistry, "latch", backpressureCb, addr bp) discard addEventListener( ctx[].eventRegistry, NotRespondingEventName, captureCb, addr notif ) diff --git a/tests/unit/test_ffi_context.nim b/tests/unit/test_ffi_context.nim index ec28ccb..2e28591 100644 --- a/tests/unit/test_ffi_context.nim +++ b/tests/unit/test_ffi_context.nim @@ -324,7 +324,6 @@ suite "sendRequestToFFIThread": check d.retCode == RET_OK check cborDecode(callbackBytes(d), string).value == "pong:" & msg - type SimpleLib = object value: int @@ -372,7 +371,6 @@ suite "ffiCtor macro": check SimpleLibFFIPool.destroyFFIContext(ctx).isOk() - type SendConfig {.ffi.} = object message: string diff --git a/tests/unit/test_serial.nim b/tests/unit/test_serial.nim index 883df75..b3e1476 100644 --- a/tests/unit/test_serial.nim +++ b/tests/unit/test_serial.nim @@ -299,8 +299,14 @@ suite "CBOR boundaries": check cborDecode(bytes, uint64).value == v test "float32 finite values incl. ±FLT_MAX": - for v in [float32(0.0), float32(-0.0), float32(1.5), float32(-1.5), - float32(3.4028235e38), float32(-3.4028235e38)]: + for v in [ + float32(0.0), + float32(-0.0), + float32(1.5), + float32(-1.5), + float32(3.4028235e38), + float32(-3.4028235e38), + ]: let bytes = cborEncode(v) check cborDecode(bytes, float32).value == v @@ -408,8 +414,8 @@ suite "CBOR round-trips": check back.value == o test "three-level struct nesting": - let d = DeepInner(tag: "outer", - nested: Nested(label: "mid", point: Point(x: 11, y: 22))) + let d = + DeepInner(tag: "outer", nested: Nested(label: "mid", point: Point(x: 11, y: 22))) let bytes = cborEncode(d) let back = cborDecode(bytes, DeepInner) check back.isOk