Merge pull request #207 from ausimian/claude/nif-static-analysis-tools-t3rmhg

Add cppcheck static analysis for the NIF

Author: ausimian

.github/workflows/ci.yml

@@ -200,6 +200,26 @@ jobs:
       - name: fast_kernels_full
         run: mix test --only fast_kernels_full
 
+  # C++ static analysis of the NIF sources with cppcheck. Kept separate
+  # from the precommit lane on purpose: the `cppcheck` Makefile target
+  # needs neither a built libmlx nor the BEAM toolchain (it analyses our
+  # own c_src/ tree and suppresses the third-party headers it can't see),
+  # so this runs on a bare checkout and finishes in seconds. Runs on
+  # macOS to match the platform the NIF actually ships on, and invokes
+  # the same `make cppcheck` a developer runs locally.
+  cppcheck:
+    name: cppcheck (NIF static analysis)
+    if: github.event_name != 'push' || github.ref_type == 'branch'
+    runs-on: macos-14
+    steps:
+      - uses: actions/checkout@v6
+
+      - name: Install cppcheck
+        run: brew install cppcheck
+
+      - name: Run cppcheck
+        run: make cppcheck
+
   # ASan CI deferred: requires OTP built with --enable-sanitizers=address
   # (macOS SIP blocks DYLD_INSERT_LIBRARIES, and late-loaded libasan
   # fails). See Makefile and RELEASE.md for details.

MAINTAINING.md

@@ -191,6 +191,20 @@ so changing the attribute is the entire pin.
 
 ## Local debugging
 
+### Static analysis of the NIF (cppcheck)
+
+```sh
+brew install cppcheck   # one-time
+make cppcheck
+```
+
+Runs cppcheck over `c_src/` and exits non-zero on any finding. It needs
+neither a built `libmlx` nor the BEAM toolchain, so it works on a bare
+checkout in seconds — the same target CI runs (`.github/workflows/ci.yml`,
+the `cppcheck` job). See the `cppcheck` target in the `Makefile` for the
+enabled checks and suppressions; use inline `// cppcheck-suppress <id>`
+for one-off false positives.
+
 ### Build MLX in isolation
 
 ```sh

Makefile

@@ -47,10 +47,48 @@ ifeq ($(EMILY_ASAN),1)
     LDFLAGS  += -fsanitize=address
 endif
 
-.PHONY: all clean bench-native
+.PHONY: all clean bench-native cppcheck
 
 all: $(NIF_SO) $(METALLIB)
 
+# ------------------------------------------------------------------
+# cppcheck: static analysis of the first-party NIF sources.
+#
+# Deliberately self-contained: it does NOT need libmlx built or the
+# MLX_INCLUDE_DIR / FINE_INCLUDE_DIR / ERTS_INCLUDE_DIR env that the
+# real compile relies on, so a developer can just run `make cppcheck`
+# from the repo root without a full NIF build (and CI can run it on a
+# bare checkout). cppcheck degrades gracefully on the third-party
+# headers it can't see — we only feed it our own `-Ic_src` tree and
+# suppress the unavoidable missing-include notices for <mlx/...>,
+# <fine.hpp>, <erl_nif.h>, etc. Findings are therefore scoped to code
+# we actually own.
+#
+# `passedByValueCallback` is suppressed on purpose: every NIF entry
+# point takes its container/aggregate args (std::vector, std::string,
+# std::tuple) by value because Fine's FINE_NIF macro decodes each BEAM
+# term into a value and passes it in — the signature is dictated by the
+# binding, not a stray copy. (Plain helpers still use const& where they
+# should.) Use inline `// cppcheck-suppress <id>` for one-off cases.
+#
+# Install: `brew install cppcheck`.
+# ------------------------------------------------------------------
+CPPCHECK       ?= cppcheck
+CPPCHECK_JOBS  ?= $(shell sysctl -n hw.ncpu 2>/dev/null || echo 4)
+CPPCHECK_FLAGS := --enable=warning,performance,portability \
+                  --std=c++20 --language=c++ \
+                  --inline-suppr \
+                  --error-exitcode=1 \
+                  --quiet -j $(CPPCHECK_JOBS) \
+                  -Ic_src \
+                  --suppress=missingInclude \
+                  --suppress=missingIncludeSystem \
+                  --suppress=unmatchedSuppression \
+                  --suppress=passedByValueCallback
+
+cppcheck:
+	$(CPPCHECK) $(CPPCHECK_FLAGS) $(SOURCES)
+
 # ------------------------------------------------------------------
 # bench-native: standalone C++ microbenchmarks under bench/native/.
 #

c_src/emily/program.hpp

@@ -53,7 +53,10 @@ inline int64_t index_of(int64_t r) { return r & kIndexMask; }
 class Program;
 
 struct CompiledInstr {
-  Opcode opcode;
+  // Value-initialized so a default-constructed instr has a defined opcode
+  // (the other members are containers that self-initialize). Every real
+  // construction aggregate-initializes all fields, overriding this.
+  Opcode opcode{};
   std::vector<int64_t> operands;             // packed refs
   std::vector<std::vector<int64_t>> iattrs;  // integer attrs (shapes/axes/dtype codes)
   // Nested programs an instruction carries (empty for all but control