Migrate to Zig 0.15.1#46
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughUpdates CI to Zig 0.15.1 and adds GDB/stacktrace example test steps; migrates example build API to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Elixir Test
participant VM as Elixir VM
participant NIF as Zig NIF (exported)
participant Beam as beam.zig
Test->>VM: call exported function (e.g., StrInt.make)
VM->>NIF: invoke exported(env, argc, args) (callconv .c)
NIF->>Beam: build/raise error (may request stack dump)
alt KINDA_DUMP_STACK_TRACE set
Note right of Beam #f8f9fb: Beam dumps stack trace (env-driven)
Beam->>Beam: dumpStackTrace(...)
end
Beam-->>NIF: return `ERL_NIF_TERM` error
NIF-->>VM: return error term
VM-->>Test: raise `Kinda.CallError` (message-only)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/kinda.zig (1)
13-13: Use consistent type names for clarity.The type annotation declares
std.array_list.AlignedManaged(u8, null)but line 13 initializes withstd.array_list.Managed(u8). While these are equivalent (null alignment defaults to Managed behavior), using different names is inconsistent.Apply this diff for consistency:
- storage: std.array_list.AlignedManaged(u8, null) = std.array_list.Managed(u8).init(beam.allocator), + storage: std.array_list.Managed(u8) = std.array_list.Managed(u8).init(beam.allocator),- storage: std.array_list.AlignedManaged(u8, null), + storage: std.array_list.Managed(u8),Also applies to: 19-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/kinda.zig(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: otp25.0-ex1.18.0 / ubuntu-22.04
- GitHub Check: otp25.0-ex1.16.2 / ubuntu-22.04
- GitHub Check: otp25.0-ex1.13.0 / ubuntu-22.04
- GitHub Check: otp24.2-ex1.13.0 / ubuntu-22.04
🔇 Additional comments (4)
src/kinda.zig (4)
1-3: LGTM! Public API exposure aligns with migration.The public exports of
beamanderl_nif.cenable downstream usage of these modules, consistent with the Zig 0.15.1 migration goals.
117-117: LGTM! Correct migration to Managed type.The change from
std.ArrayList(u8)tostd.array_list.Managed(u8)aligns with Zig 0.15.1 API changes, and resource cleanup is properly handled withdefer buffer.deinit().
119-122: LGTM! Format specifier updated correctly.The change from
"{?}"to"{any}"aligns with Zig 0.15.1's formatting conventions. The{any}specifier is appropriate for general type formatting in the dump function.
310-310: LGTM! Correct format specifier for integer index.The change from
"{?}"to"{d}"correctly formats the argument index as a decimal integer. This improves error message clarity when argument fetching fails.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores