38 KiB

Raw Blame History

Zig Code Review Reference (Zig 0.16.0)

Systematic code review checklist organized by detection confidence level. Work through sections in order: ALWAYS FLAG -> FLAG WITH CONTEXT -> SUGGEST. For version-specific upgrade issues, load Zig 0.16 Release Notes Migration Reference first.

Quick Reference Card

Scan Order

ALWAYS FLAG - Mechanical checks, objectively wrong
FLAG WITH CONTEXT - Requires understanding function semantics
SUGGEST - Style suggestions for review feedback

Most Common Issues

Detect	Issue	Risk	Fix
file/process/net/time/random API without `io`	Old 0.15 API or bad 0.16 design	Compile error or wrong runtime ownership	Add/pass/store `std.Io`
`std.fs.cwd()` / `std.fs.File` / `std.fs.Dir`	File-system API moved	Compile error	Use `std.Io.Dir` / `std.Io.File`
`std.process.Child.run` / `Child.init`	Process API changed	Compile error	Use `std.process.run(gpa, io, ...)` / `spawn(io, ...)`
`std.Thread.Mutex` in I/O path	Blocking sync bypasses Io backend	Cancelation/perf/runtime bugs	Use `std.Io.Mutex` and pass `io`
`@Type(`	Removed builtin	Compile error	Use `@Int`, `@Struct`, `@Union`, `@Enum`, etc.
`@cImport` for new bindings	Deprecated path	Future breakage / translate-c drift	Use `b.addTranslateC(...)`
`writer(&buf)` without `.flush()`	Missing flush	Data loss	Add `try w.flush()`
`.{}` on container type	Wrong init	Compile error	Use `.empty` or `.init`
`root_source_file` in build.zig	Old build API	Compile error	Use `root_module = b.createModule(...)`
`"{}"` with custom format method	Old format spec	Compile error	Use `"{f}"` (see 1.4)
`getOrPut` then `ensure*`	Exception safety	Orphan entries	Reserve first, mutate with `*AssumeCapacity`

Guaranteed Bug Patterns

Detect	Risk	Verify
`@intCast(val)` without bounds check	Runtime panic	No `std.math.cast` or prior validation
`.?` unwrap	Runtime panic	Not guarded by `if` or `orelse`
`catch unreachable` on alloc/create	Runtime panic	Allocation can fail
`return &local_var`	Dangling pointer	Returns address of stack variable
`&packed_struct.field`	Undefined behavior	Pointer to packed struct field
pointer field in `packed struct`/`packed union`	Forbidden in 0.16	Compile error / invalid layout
`DebugAllocator` without `defer.*deinit()`	No leak detection	Missing cleanup
build.zig without `standardTargetOptions`	No cross-compile	Missing standard options

Context-Dependent Checks

Detect	When to Flag	Section
`anyerror` return type	Library public API	2.7
Loop allocations with non-freeing allocator	Arena without reset	2.8
`[*]T` pointer type	Pure Zig code (not FFI)	2.10
Regular `for` on `@typeInfo` fields	Need comptime iteration	2.11
`ArenaAllocator` without `.reset()`	Long-running service	2.9
Multi-step alloc without errdefer	Partial construction	2.12
Alloc without `defer free` + error returns	Resource leak	2.13

Style Quick Checks

Detect	Suggestion	Section
Imports not grouped	std → third-party → local	3.8
Global allocator variable	Accept as parameter	3.9
`allocPrint` for bounded strings	Use `bufPrint` with stack buffer	3.10
Runtime constant lookup	Comptime lookup table	3.11
`expectEqual` with strings/slices	Use `expectEqualStrings`/`expectEqualSlices`	3.14

Review Workflow

Run zig fmt — catches whitespace, trailing commas, basic style
Scan for ALWAYS FLAG patterns — removed features, old APIs, guaranteed bugs
Check 0.16 I/O ownership — std.Io parameter flow, Juicy Main, tests with std.testing.io
Check safety — defer/errdefer usage, reserve-first pattern, memory poisoning
Check context-dependent issues — allocator naming, error handling, loop resources
Review style — naming conventions, struct init, imports, documentation
Verify build.zig — standardTargetOptions, standardOptimizeOption, root_module, addTranslateC

1. ALWAYS FLAG (100% Confidence)
2. FLAG WITH CONTEXT (High Confidence)
3. SUGGEST (Advisory)

1. ALWAYS FLAG (100% Confidence)

Objective errors that cause compilation failures or guaranteed runtime bugs.

1.1 Removed Language Features

Detect	Replacement	Risk
`usingnamespace`	Explicit re-exports	Compile error
`async`/`await` keywords	Removed entirely	Compile error
`@fence()`	Stronger atomic orderings or RMW operations	Compile error
`@setCold(true/false)`	`@branchHint(.cold)`	Compile error
`@setAlignStack()`	`callconv(.withStackAlign(...))`	Compile error
`@Type(...)`	Specific 0.16 type builtin such as `@Int`/`@Struct`	Compile error
`std.Thread.Pool`	Application scheduler or `std.Io.Group`/`io.async` where semantically correct	Compile error
`std.Thread.Mutex.Recursive`	Redesign locking or use explicit state	Compile error
`std.BoundedArray`	`ArrayList.initBuffer()`	Compile error

Wrong:

pub usingnamespace @import("other.zig");

Right:

const other = @import("other.zig");
pub const foo = other.foo;

Wrong:

fn coldPath() void {
    @setCold(true);
}

Right:

fn coldPath() void {
    @branchHint(.cold);  // Must be first statement
}

Wrong:

var stack = try std.BoundedArray(i32, 8).fromSlice(initial);

Right:

var buffer: [8]i32 = undefined;
var stack = std.ArrayList(i32).initBuffer(&buffer);
try stack.appendSliceBounded(initial);

1.2 Changed Syntax (0.14+)

Detect	New	Risk
`@export(foo, opts)`	`@export(&foo, opts)`	Compile error
`.Int`, `.Struct` in @typeInfo	`.int`, `.@"struct"`	Compile error
`.One`, `.Slice`, `.Many`	`.one`, `.slice`, `.many`	Compile error
`sentinel = &val`	`sentinel_ptr = &val`	Compile error
Inline asm clobbers `"rcx"`	`.{ .rcx = true }`	Compile error
`callconv(.C)`	`callconv(.c)`	Compile error
`callconv(.Stdcall)`	`callconv(.x86_stdcall)`	Compile error

Wrong:

@export(foo, .{ .name = "bar" });

Right:

@export(&foo, .{ .name = "bar" });

Wrong:

switch (@typeInfo(T)) {
    .Int => {},
    .Struct => {},
    .Pointer => |p| if (p.size == .One) {},
}

Right:

switch (@typeInfo(T)) {
    .int => {},
    .@"struct" => {},
    .pointer => |p| if (p.size == .one) {},
}

Wrong:

asm volatile ("syscall"
    : [ret] "={rax}" (-> usize),
    : [number] "{rax}" (number),
    : "rcx", "r11"
);

Right:

asm volatile ("syscall"
    : [ret] "={rax}" (-> usize),
    : [number] "{rax}" (number),
    : .{ .rcx = true, .r11 = true }
);

Wrong:

export fn foo() callconv(.C) void {}

Right:

export fn foo() callconv(.c) void {}

1.3 Removed/Renamed APIs

Detect	New	Risk
`root_source_file`	`root_module = b.createModule(...)`	Compile error
`exe.addModule(...)`	`exe.root_module.addImport(...)`	Compile error
`GeneralPurposeAllocator`	`DebugAllocator`	Alias works
`std.mem.page_size`	`std.heap.pageSize()`	Compile error
`BufferedWriter`	Buffer provided to `.writer(&buf)`	Compile error
`CountingWriter`	`std.Io.Writer.Discarding`	Compile error
`GenericWriter/Reader`	`std.Io.Writer/Reader`	Deprecated
`std.ArrayList` (managed)	`std.array_list.Managed`	Eventually removed
`std.ArrayListUnmanaged`	`std.ArrayList`	Unmanaged is now the default
`std.fifo.LinearFifo`	`std.Io.Reader`/`Writer` patterns	Removed
`std.RingBuffer`	`std.Io.Reader`/`Writer` patterns	Removed
`std.ChildProcess`	`std.process.Child`	Compile error
`*std.build.Builder`	`*std.Build`	Compile error
`.{ .path = "..." }`	`b.path("...")`	Compile error

Wrong:

b.addExecutable(.{
    .name = "app",
    .root_source_file = b.path("src/main.zig"),
    .target = target,
    .optimize = optimize,
});

Right:

b.addExecutable(.{
    .name = "app",
    .root_module = b.createModule(.{
        .root_source_file = b.path("src/main.zig"),
        .target = target,
        .optimize = optimize,
    }),
});

Wrong:

exe.addModule("helper", helper_mod);

Right:

exe.root_module.addImport("helper", helper_mod);

1.4 API Signature Changes

Note: For "{}" → "{f}", this is a migration check. For new code forgetting .flush(), see also 2.2.

Detect	New	Risk
`stdout.print(fmt, args)` without flush	Must call `stdout.flush()`	Data loss
`format(self, fmt, opts, writer)`	`format(self, *std.Io.Writer)`	Compile error
`"{}"` for format methods	`"{f}"`	Compile error
`std.process.Child.run` / `Child.init`	`std.process.run(gpa, io, ...)` / `std.process.spawn(io, ...)`	Compile error

Wrong:

const stdout = std.io.getStdOut().writer();
try stdout.print("Hello\n", .{});

Right:

var buf: [4096]u8 = undefined;
var stdout_writer = std.Io.File.stdout().writer(io, &buf);
const stdout = &stdout_writer.interface;
try stdout.print("Hello\n", .{});
try stdout.flush();  // Required

Wrong:

pub fn format(
    self: @This(),
    comptime fmt: []const u8,
    opts: std.fmt.FormatOptions,
    writer: anytype,
) !void { ... }

Right:

pub fn format(self: @This(), writer: *std.Io.Writer) std.Io.Writer.Error!void { ... }

Wrong:

std.debug.print("{}", .{myFormattableType});

Right:

std.debug.print("{f}", .{myFormattableType});

1.5 Container Initialization

Detect	Right	Risk
`var list: ArrayList(T) = .{}`	`.empty`	Deprecated
`var gpa: DebugAllocator(.{}) = .{}`	`.init`	Deprecated
`var map: HashMapUnmanaged(...) = .{}`	`.empty`	Deprecated
`list.append(42)` (old managed API)	`try list.append(allocator, 42)`	Compile error

Wrong:

var list: std.ArrayList(u32) = .{};
var gpa: std.heap.DebugAllocator(.{}) = .{};

Right:

var list: std.ArrayList(u32) = .empty;
var map: std.AutoHashMapUnmanaged(u32, u32) = .empty;
var gpa: std.heap.DebugAllocator(.{}) = .init;
var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator);

ArrayList is now unmanaged by default — allocator is passed per method call, not stored:

Wrong (old managed API):

var list = std.ArrayList(u32).init(allocator);
list.append(42);

Right (unmanaged, allocator per call):

var list: std.ArrayList(u32) = .empty;
try list.append(allocator, 42);
try list.appendSlice(allocator, &.{ 1, 2, 3 });
defer list.deinit(allocator);

1.6 Guaranteed Runtime Bugs

1.6.1 Unchecked @intCast

Detect	`@intCast` without prior bounds check or `std.math.cast`
Risk	Runtime panic on overflow

Wrong:

fn convert(big: u64) u8 {
    return @intCast(big);  // Panics if big > 255
}

Right:

fn convert(big: u64) ?u8 {
    return std.math.cast(u8, big);
}

Verify: Search for @intCast calls, check for prior validation.

1.6.2 Unguarded Optional Unwrap

Detect	`.?` not preceded by `if` check or `orelse`
Risk	Runtime panic if null

Wrong:

fn getName(user: ?*User) []const u8 {
    return user.?.name;
}

Right:

fn getName(user: ?*User) []const u8 {
    return if (user) |u| u.name else "anonymous";
}

Verify: Search for .? usage, confirm null case is handled.

1.6.3 Catch Unreachable on Allocation

Detect	`catch unreachable` after `alloc`/`create` calls
Risk	Runtime panic on OOM

Wrong:

fn createBuffer(allocator: Allocator) *Buffer {
    return allocator.create(Buffer) catch unreachable;
}

Right:

fn createBuffer(allocator: Allocator) !*Buffer {
    return try allocator.create(Buffer);
}

Verify: Search for catch unreachable, check if operation can fail.

1.6.4 Returning Stack Pointer

Detect	`return &` of local variable
Risk	Dangling pointer

Wrong:

fn getBuffer() *[256]u8 {
    var buf: [256]u8 = undefined;
    return &buf;
}

Right:

fn fillBuffer(buf: *[256]u8) void { ... }

// OR return by value if small
fn getBuffer() [256]u8 {
    var buf: [256]u8 = undefined;
    return buf;
}

Verify: Search for return &, check if variable is local.

1.6.5 Self-Referential Struct Copy

Detect	Struct with self-pointer field returned by value
Risk	Dangling internal pointer

Wrong:

const Node = struct {
    self_ptr: *Node,

    fn init() Node {
        var node: Node = undefined;
        node.self_ptr = &node;
        return node;  // self_ptr now dangling
    }
};

Right:

fn init(allocator: Allocator) !*Node {
    const node = try allocator.create(Node);
    node.self_ptr = node;
    return node;
}

Verify: Check structs with pointer fields pointing to self.

1.6.6 Division by Runtime Variable

Detect	`/` or `%` with non-constant divisor
Risk	Runtime panic if zero

Wrong:

fn average(sum: u32, count: u32) u32 {
    return sum / count;
}

Right:

fn average(sum: u32, count: u32) ?u32 {
    if (count == 0) return null;
    return sum / count;
}

Verify: Search for division, check if divisor is validated.

1.6.7 Inactive Union Field Access

Detect	Direct union field access without tag check
Risk	Undefined behavior

Wrong:

fn getInt(v: Value) i64 {
    return v.int;  // UB if v is not .int
}

Right:

fn getInt(v: Value) ?i64 {
    return switch (v) {
        .int => |i| i,
        else => null,
    };
}

Verify: Check union field access, ensure tag is verified first.

1.6.8 Unchecked @enumFromInt

Detect	`@enumFromInt` without range validation
Risk	Runtime panic if invalid

Wrong:

fn parseColor(byte: u8) Color {
    return @enumFromInt(byte);
}

Right:

fn parseColor(byte: u8) ?Color {
    return std.meta.intToEnum(Color, byte) catch null;
}

Verify: Search for @enumFromInt, check for validation.

1.7 Memory Safety Violations

1.7.1 Missing DebugAllocator Deinit

Detect	`DebugAllocator` without `defer.*deinit()`
Risk	No leak detection in debug builds

Wrong:

pub fn main() !void {
    var gpa: std.heap.DebugAllocator(.{}) = .init;
    const allocator = gpa.allocator();
    // Missing gpa.deinit()
}

Right:

pub fn main() !void {
    var gpa: std.heap.DebugAllocator(.{}) = .init;
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();
}

Verify: Search for DebugAllocator, check for defer.*deinit().

1.7.2 @ptrCast Size Mismatch

Detect	`@ptrCast` between types of different sizes
Risk	Memory corruption

Wrong:

fn dangerous(ptr: *u32) *u64 {
    return @ptrCast(ptr);  // Reading u64 from u32 space
}

Right:

fn reinterpret(ptr: *u32) *[4]u8 {
    return @ptrCast(ptr);  // Same size
}

Verify: Check @ptrCast source and target sizes match.

1.7.3 Packed Struct Field Pointer

Detect	`&packed_struct.field`
Risk	Unaligned access UB

Wrong:

const Packet = packed struct { flags: u4, len: u12, data: u16 };

fn getLen(pkt: *Packet) *u12 {
    return &pkt.len;  // Unaligned pointer
}

Right:

fn getLen(pkt: *const Packet) u12 {
    return pkt.len;  // Copy value
}

Verify: Search for & on packed struct field access.

1.7.4 Disabled Runtime Safety

Detect	`@setRuntimeSafety(false)`
Risk	All safety checks removed

Wrong:

fn fastPath(data: []u8) void {
    @setRuntimeSafety(false);
}

Right:

fn fastPath(data: []u8) void {
    // Use ReleaseFast build mode for controlled optimization
}

Verify: Search for @setRuntimeSafety(false).

1.8 Build System Anti-patterns

Detect	Problem	Risk
Missing `b.standardTargetOptions()`	No cross-compilation	Build inflexibility
Missing `b.standardOptimizeOption()`	No release builds	Build inflexibility

Wrong:

pub fn build(b: *std.Build) void {
    const exe = b.addExecutable(.{
        .name = "app",
        .root_module = b.createModule(.{
            .root_source_file = b.path("src/main.zig"),
            // Missing target and optimize
        }),
    });
}

Right:

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const exe = b.addExecutable(.{
        .name = "app",
        .root_module = b.createModule(.{
            .root_source_file = b.path("src/main.zig"),
            .target = target,
            .optimize = optimize,
        }),
    });
}

Verify: Check build.zig for standardTargetOptions and standardOptimizeOption.

2. FLAG WITH CONTEXT (High Confidence)

Genuine issues when preconditions are satisfied.

2.1 Exception Safety Bugs

Detect	`getOrPut` followed by `ensureCapacity` in same function
Risk	Orphan entries on allocation failure

Preconditions:

Function inserts into multiple growable containers
Mutation precedes reservation

If met: Reorder to reserve-first pattern.

Wrong:

const gop = try state.table.getOrPut(gpa, key);
try state.bytes.ensureUnusedCapacity(gpa, len);  // Failure leaves orphan

Right:

try state.table.ensureUnusedCapacityContext(gpa, 1, ctx);
try state.bytes.ensureUnusedCapacity(gpa, len);

errdefer comptime unreachable;  // Assert no errors after this

const gop = state.table.getOrPutAssumeCapacity(key);
state.bytes.appendSliceAssumeCapacity(data);

Verify: Search for getOrPut followed by ensure in same scope.

2.2 Missing flush() After I/O Write

Note: For migrating old stdout.print() calls, see also 1.4. This section covers new code that forgets .flush().

Detect	`writer(&buf)` or `.writer(&` without subsequent `.flush()`
Risk	Data loss (buffered data not written)

Preconditions:

Code uses a buffered std.Io.Writer
Writer scope ends without flush

If met: Add try writer.interface.flush() before scope exit.

Wrong:

var buf: [4096]u8 = undefined;
var writer = file.writer(io, &buf);
try writer.interface.print("data", .{});
// Missing flush

Right:

var buf: [4096]u8 = undefined;
var writer = file.writer(io, &buf);
try writer.interface.print("data", .{});
try writer.interface.flush();

Verify: Check for writer(& without corresponding .flush().

2.3 Allocator Pointer Comparison

Detect	`alloc1.ptr == alloc2.ptr` or similar
Risk	Undefined for stateless allocators

Preconditions:

Code compares Allocator or Random interface ptr fields

If met: Remove comparison; ptr is undefined for page_allocator, c_allocator.

2.4 Generic Allocator Naming

Detect	Allocator parameter named just `allocator`
Risk	Hidden memory ownership contract

Preconditions:

Function has allocator parameter
Ownership semantics unclear from name

If met: Rename by memory contract:

Name	Contract	Return Data?	Who Frees?
`gpa`	Long-lived	Yes	Caller
`arena`	Request-scoped	Yes	Arena owner
`scratch`	Function-private	Never	Function

Wrong:

fn process(allocator: Allocator) ![]u8 {
    const temp = try allocator.alloc(u8, 100);
    const result = try allocator.dupe(u8, temp);
    allocator.free(temp);
    return result;
}

Right:

fn handleRequest(
    arena: Allocator,   // Response lifetime
    gpa: Allocator,     // Long-lived cache
    scratch: Allocator, // Function temporaries
) !Response { ... }

2.5 Use-After-Free Potential

Detect	`deinit` method without `self.* = undefined;`
Risk	Use-after-free not caught in debug

Preconditions:

Type has deinit method that frees memory
No memory poisoning after deallocation

If met: Add self.* = undefined; at end of deinit.

Wrong:

pub fn deinit(self: *Self, gpa: Allocator) void {
    gpa.free(self.allocatedSlice());
}

Right:

pub fn deinit(self: *Self, gpa: Allocator) void {
    gpa.free(self.allocatedSlice());
    self.* = undefined;
}

Verify: Check deinit methods for memory poisoning.

2.6 Type-Unsafe Index Usage

Detect	Multiple `u32`/`usize` indices for different arrays
Risk	Index type confusion

Preconditions:

Code manages parallel arrays
Same integer type used for different index spaces

If met: Use distinct enum types for indices.

Wrong:

fn getSection(index: u32) *Section { ... }
fn getSymbol(index: u32) *Symbol { ... }

Right:

const SectionIndex = enum(u32) { _ };
const SymbolIndex = enum(u32) { _ };

fn getSection(index: SectionIndex) *Section { ... }
fn getSymbol(index: SymbolIndex) *Symbol { ... }

2.7 Error Handling Selection

Detect	`anyerror` return in public API, or blind `try` propagation
Risk	Callers cannot handle errors specifically

Preconditions:

Library public function returns anyerror
OR: Function uses try where specific handling needed

If met: Define specific error set; use catch with switch for meaningful errors.

Wrong:

pub fn parse(input: []const u8) anyerror!Ast { ... }

Right:

pub const ParseError = error{ UnexpectedToken, InvalidSyntax, OutOfMemory };
pub fn parse(input: []const u8) ParseError!Ast { ... }

2.8 Loop Resource Management

Detect	Allocations inside loop body
Risk	Memory accumulation or premature free

Preconditions:

Loop body allocates memory
Allocator does not actually free (e.g., ArenaAllocator)
OR: Results accumulate across iterations where defer would free too early

If met: Use arena with per-iteration reset, or manage lifetime explicitly.

Zig's defer is block-scoped — it runs at the end of each loop iteration, not at function exit. This means defer free inside a loop works correctly for per-iteration cleanup. The real risks are:

Risk 1: Non-freeing allocator makes defer free a no-op:

Wrong:

// ArenaAllocator.free() is a no-op — memory accumulates
for (items) |item| {
    const data = try arena.alloc(u8, item.size);
    defer arena.free(data);  // Does nothing!
    try process(data);
}

Right:

// Use a scratch arena with per-iteration reset
var scratch = std.heap.ArenaAllocator.init(backing_allocator);
defer scratch.deinit();

for (items) |item| {
    defer _ = scratch.reset(.retain_capacity);
    const data = try scratch.allocator().alloc(u8, item.size);
    try process(data);
}

Risk 2: Accumulating results across iterations where defer would free too early:

Wrong:

var results: std.ArrayList([]u8) = .empty;
for (items) |item| {
    const data = try allocator.dupe(u8, item.name);
    defer allocator.free(data);  // Frees before we're done with it!
    try results.append(allocator, data);
}

Right:

var results: std.ArrayList([]u8) = .empty;
for (items) |item| {
    const data = try allocator.dupe(u8, item.name);
    errdefer allocator.free(data);  // Only free on error
    try results.append(allocator, data);
}

2.9 Allocator Misuse Patterns

FixedBufferAllocator Non-LIFO Free

Detect	Free in non-LIFO order with FixedBufferAllocator
Risk	Unexpected allocation failures

Preconditions:

Using FixedBufferAllocator
Freeing allocations out of order

If met: Use LIFO order or switch to ArenaAllocator.

ArenaAllocator Without Reset

Detect	`ArenaAllocator` in loop without `.reset()`
Risk	Unbounded memory growth

Preconditions:

Long-running service/loop
Arena never reset

If met: Add defer _ = arena.reset(.retain_capacity); in loop.

Wrong:

while (true) {
    const request = try readRequest(arena.allocator());
    try processRequest(request);
    // Missing reset
}

Right:

while (true) {
    defer _ = arena.reset(.retain_capacity);
    const request = try readRequest(arena.allocator());
    try processRequest(request);
}

2.10 Pointer Type Selection

Detect	`[*]T` in pure Zig code (not C FFI)
Risk	No bounds checking

Preconditions:

Code uses [*]T pointer
Not at C FFI boundary

If met: Use slice []T instead.

Wrong:

fn process(data: [*]u8, len: usize) void {
    var i: usize = 0;
    while (i < len) : (i += 1) {
        data[i] = transform(data[i]);
    }
}

Right:

fn process(data: []u8) void {
    for (data) |*byte| {
        byte.* = transform(byte.*);
    }
}

2.11 Comptime Propagation

Detect	Regular `for` iterating `@typeInfo` fields
Risk	Comptime values lost at runtime boundary

Preconditions:

Iterating comptime-known collection
Using field names or other comptime data

If met: Use inline for or mark function inline.

Wrong:

fn printFields(comptime T: type) void {
    for (@typeInfo(T).@"struct".fields) |field| {
        std.debug.print("{s}\n", .{field.name});  // Error
    }
}

Right:

fn printFields(comptime T: type) void {
    inline for (@typeInfo(T).@"struct".fields) |field| {
        std.debug.print("{s}\n", .{field.name});
    }
}

2.12 Missing errdefer for Partial Construction

Detect	Multi-step allocation without errdefer cleanup
Risk	Resource leak on failure

Preconditions:

Function performs multiple fallible allocations
Earlier allocations not cleaned up if later ones fail

If met: Add errdefer after each allocation that must be cleaned up on failure.

Wrong:

pub fn init(gpa: Allocator) !Self {
    const data = try gpa.alloc(u8, 100);
    const more = try gpa.alloc(u8, 200);  // data leaks if this fails
    return .{ .data = data, .more = more };
}

Right:

pub fn init(gpa: Allocator) !Self {
    const data = try gpa.alloc(u8, 100);
    errdefer gpa.free(data);
    const more = try gpa.alloc(u8, 200);
    return .{ .data = data, .more = more };
}

Verify: Search for functions with multiple try allocator.alloc/create calls. Check that each allocation (except the last) has a corresponding errdefer.

2.13 Missing defer for Allocation Cleanup

Detect	Allocation without corresponding `defer free` in function with error/early-return paths
Risk	Memory leak on non-error paths

Preconditions:

Function allocates memory for internal use (not returned to caller)
Function has try, return, or other early-exit paths after allocation
No defer to free the allocation

If met: Add defer allocator.free(...) immediately after allocation.

This is distinct from errdefer (2.12) — errdefer only fires on error. This covers unconditional cleanup of internally-used allocations.

Wrong:

fn process(gpa: Allocator, input: []const u8) !Result {
    const temp = try gpa.alloc(u8, input.len);
    const parsed = try parse(input);  // temp leaks if this fails
    // ... use temp ...
    gpa.free(temp);
    return parsed;
}

Right:

fn process(gpa: Allocator, input: []const u8) !Result {
    const temp = try gpa.alloc(u8, input.len);
    defer gpa.free(temp);
    const parsed = try parse(input);  // temp freed by defer
    // ... use temp ...
    return parsed;
}

Verify: Search for alloc( not followed by defer.*free in functions with try or early returns.

3. SUGGEST (Advisory)

Style and idiom suggestions.

3.1 Unnecessary Self Alias

Detect	`const Self = @This();` used only once
Risk	None (style)

Wrong:

pub const Iterator = struct {
    const Self = @This();  // Used once
    pub fn next(self: *Self) ?Entry { ... }
};

Right:

pub const Iterator = struct {
    pub fn next(self: *@This()) ?Entry { ... }
};

3.2 Unnecessary Named Return Variable

Detect	Variable assigned then immediately returned
Risk	None (style)

Wrong:

const result = Entry{ .key = k, .value = v };
return result;

Right:

return .{ .key = k, .value = v };

3.3 Old Struct Init Syntax

Detect	`TypeName{}` instead of `.{}` with type annotation
Risk	None (style)

Wrong:

var mutex = Mutex{};

Right:

var mutex: Mutex = .{};

3.4 Redundant Naming

Detect	Names like `JsonValue`, `DataManager`, `miscUtils`
Risk	None (style)

Wrong:

const JsonValue = struct { ... };
const JsonParser = struct { ... };

Right:

const json = struct {
    const Value = struct { ... };
    const Parser = struct { ... };
};

3.5 Large Struct Passed by Value

Detect	Struct >16 bytes passed as `self: T` (read-only)
Risk	Unnecessary copy

Wrong:

pub fn format(uri: Uri, writer: *Writer) Writer.Error!void { ... }

Right:

pub fn format(uri: *const Uri, writer: *Writer) Writer.Error!void { ... }

Also applies to payload captures:

// Wrong: if (m.target) |target| { ... }
// Right: if (m.target) |*target| { ... }

3.6 errdefer Error Capture for Debugging

Detect	Complex init/setup function without `errdefer` diagnostics
Risk	Hard to diagnose failures

In complex initialization functions, use errdefer |err| to capture and log the error for debugging context:

Basic:

pub fn init(gpa: Allocator, io: std.Io, path: []const u8) !Self {
    const file = std.Io.Dir.cwd().openFile(io, path, .{}) catch |err| return err;
    // No context if later steps fail
    ...
}

Better:

pub fn init(gpa: Allocator, io: std.Io, path: []const u8) !Self {
    errdefer |err| {
        std.log.err("init failed for '{s}': {}", .{ path, err });
    }
    const file = try std.Io.Dir.cwd().openFile(io, path, .{});
    errdefer file.close(io);
    ...
}

3.7 Style Guide Violations (not caught by zig fmt)

Element	Convention	Example
Types	`TitleCase`	`XmlParser`, `HashMap`
Namespace structs	`snake_case`	`std.json`, `std.mem`
Functions	`camelCase`	`readU32Be`, `parseJson`
Type-returning functions	`TitleCase`	`ArrayList`, `HashMap`
Variables/constants	`snake_case`	`const_name`, `file_path`

Acronyms: Treat as regular words (XmlParser not XMLParser).

File naming:

Type file: TitleCase.zig (e.g., ArrayList.zig)
Namespace file: snake_case.zig (e.g., mem.zig)

3.8 Import Organization

Detect	Imports not grouped or inconsistently ordered
Risk	None (style)

Right:

const std = @import("std");
const builtin = @import("builtin");

const json = @import("json");  // Third-party

const MyModule = @import("my_module.zig");  // Local

3.9 Allocator Design

Detect	Global allocator variable
Risk	Untestable code

Wrong:

var global_allocator: Allocator = undefined;
pub fn process() ![]u8 {
    return global_allocator.alloc(u8, 100);
}

Right:

pub fn process(allocator: Allocator) ![]u8 {
    return allocator.alloc(u8, 100);
}

3.10 Stack vs Heap Allocation

Detect	Heap allocation for comptime-known bounded size
Risk	Unnecessary allocation

Wrong:

fn formatVersion(allocator: Allocator, major: u32, minor: u32) ![]u8 {
    return std.fmt.allocPrint(allocator, "{d}.{d}", .{ major, minor });
}

Right (caller provides buffer):

fn formatVersion(buf: []u8, major: u32, minor: u32) []u8 {
    return std.fmt.bufPrint(buf, "{d}.{d}", .{ major, minor }) catch unreachable;
}

// Call site — buffer outlives the returned slice
var buf: [32]u8 = undefined;
const version = formatVersion(&buf, 1, 2);

3.11 Comptime Optimization

Detect	Runtime loop for constant lookup
Risk	Suboptimal performance

Wrong:

fn isVowel(c: u8) bool {
    for ("aeiouAEIOU") |v| {
        if (c == v) return true;
    }
    return false;
}

Right:

fn isVowel(c: u8) bool {
    const table = comptime blk: {
        var t: [256]bool = .{false} ** 256;
        for ("aeiouAEIOU") |v| t[v] = true;
        break :blk t;
    };
    return table[c];
}

3.12 SIMD Opportunities

Detect	Scalar loop on arrays where SIMD helps
Risk	Suboptimal performance

Scalar:

fn addArrays(a: []const f32, b: []const f32, result: []f32) void {
    for (a, b, result) |av, bv, *rv| {
        rv.* = av + bv;
    }
}

SIMD:

fn addArrays(a: []const f32, b: []const f32, result: []f32) void {
    const Vec = @Vector(8, f32);
    var i: usize = 0;
    while (i + 8 <= a.len) : (i += 8) {
        const va: Vec = a[i..][0..8].*;
        const vb: Vec = b[i..][0..8].*;
        result[i..][0..8].* = va + vb;
    }
    while (i < a.len) : (i += 1) {
        result[i] = a[i] + b[i];
    }
}

Only suggest for hot loops with measurable impact.

3.13 Struct Layout

Detect	`packed` for non-binary data, or poor extern field ordering
Risk	Slower access or wasted padding

Use:

Regular struct for normal use
packed struct for binary protocols/hardware
extern struct for C ABI

Extern field order: Order by size descending to minimize padding.

3.14 Testing Best Practices

Detect	Production allocator in tests, or wrong assertion type
Risk	Missed leaks, false passes

Allocator:

// Wrong: var gpa: std.heap.DebugAllocator(.{}) = .init;
// Right: use std.testing.allocator

Assertions:

// Strings: use expectEqualStrings (not expectEqual)
// Slices: use expectEqualSlices (not expectEqual)

3.15 Documentation

Detect	Public API without doc comments
Risk	Poor discoverability

Focus on: what it does, error conditions, ownership, usage example.

3.16 Stateless Context Pattern

Detect	`self` parameter that's never used
Risk	None (style)

Wrong:

pub fn eql(self: @This(), a: []const u8, b: []const u8) bool {
    _ = self;
    return std.mem.eql(u8, a, b);
}

Right:

pub fn eql(_: @This(), a: []const u8, b: []const u8) bool {
    return std.mem.eql(u8, a, b);
}

File References

SKILL.md - Breaking changes overview
patterns.md - Best practices patterns
std-io.md - New I/O API
std-allocators.md - Allocator naming conventions
style-guide.md - Style conventions

38 KiB Raw Blame History

Zig Code Review Reference (Zig 0.16.0)

Quick Reference Card

Scan Order

Most Common Issues

Guaranteed Bug Patterns

Context-Dependent Checks

Style Quick Checks

Review Workflow

Table of Contents

1. ALWAYS FLAG (100% Confidence)

1.1 Removed Language Features

1.2 Changed Syntax (0.14+)

1.3 Removed/Renamed APIs

1.4 API Signature Changes

1.5 Container Initialization

1.6 Guaranteed Runtime Bugs

1.6.1 Unchecked @intCast

1.6.2 Unguarded Optional Unwrap

1.6.3 Catch Unreachable on Allocation

1.6.4 Returning Stack Pointer

1.6.5 Self-Referential Struct Copy

1.6.6 Division by Runtime Variable

1.6.7 Inactive Union Field Access

1.6.8 Unchecked @enumFromInt

1.7 Memory Safety Violations

1.7.1 Missing DebugAllocator Deinit

1.7.2 @ptrCast Size Mismatch

1.7.3 Packed Struct Field Pointer

1.7.4 Disabled Runtime Safety

1.8 Build System Anti-patterns

2. FLAG WITH CONTEXT (High Confidence)

2.1 Exception Safety Bugs

2.2 Missing flush() After I/O Write

2.3 Allocator Pointer Comparison

2.4 Generic Allocator Naming

2.5 Use-After-Free Potential

2.6 Type-Unsafe Index Usage

2.7 Error Handling Selection

2.8 Loop Resource Management

2.9 Allocator Misuse Patterns

FixedBufferAllocator Non-LIFO Free

ArenaAllocator Without Reset

2.10 Pointer Type Selection

2.11 Comptime Propagation

2.12 Missing errdefer for Partial Construction

2.13 Missing defer for Allocation Cleanup

3. SUGGEST (Advisory)

3.1 Unnecessary Self Alias

3.2 Unnecessary Named Return Variable

3.3 Old Struct Init Syntax

3.4 Redundant Naming

3.5 Large Struct Passed by Value

3.6 errdefer Error Capture for Debugging

3.7 Style Guide Violations (not caught by zig fmt)

3.8 Import Organization

3.9 Allocator Design

3.10 Stack vs Heap Allocation

3.11 Comptime Optimization

3.12 SIMD Opportunities

3.13 Struct Layout

3.14 Testing Best Practices

3.15 Documentation

3.16 Stateless Context Pattern

File References

38 KiB

Raw Blame History