| name | rust-unsafe |
| description | `unsafe` conventions in the Rocky engine — SAFETY comment rules, the legitimate unsafe categories (a memory-map, a repr(transparent) cast, and serialised test env-var mutation), and when to push back on new unsafe. Use when auditing, reviewing, or adding any `unsafe` block/function in the engine crates. |
unsafe in the Rocky engine
Reality check
Rocky keeps unsafe minimal and boring on purpose. It falls into two buckets — get the live list with rg -n '\bunsafe\b' crates/ from engine/ rather than trusting a hardcoded count (the set drifts as tests are added):
Production unsafe — two sites, both in rocky-core:
| Site | What's unsafe | Why it's justified |
|---|
rocky-core/src/mmap.rs | memmap2::Mmap::map(&file) for project files | Reading many SQL/TOML files during compile wants mmap for throughput; the mapped bytes are read-only project source — worst case is a garbled read → parse error, not UB. |
rocky-core/src/column_map.rs | &*(s as *const str as *const CiStr) — cast to a #[repr(transparent)] case-insensitive str newtype | Layout-compatible by repr(transparent); the borrow never outlives the input str. |
Test-only unsafe — the majority of sites: std::env::{set_var, remove_var} inside #[cfg(test)] modules across many crates (config.rs, models.rs, pipes.rs, object_store.rs, rocky-observe, rocky-server/src/lsp.rs, several rocky-cli commands, and integration tests). Env mutation became unsafe in the 2024 edition because it races with concurrent reads; every one is serialised under a module ENV_LOCK (or a single-threaded test) and restores state before returning.
There is no FFI unsafe. The duckdb crate, jsonwebtoken, and rsa all wrap their C / crypto surfaces in safe APIs — Rocky calls those and never reaches into duckdb_sys or similar. If you find yourself writing an FFI binding from scratch, stop and check whether the upstream crate already has a safe wrapper.
Keep this list short. Every new unsafe site is a reviewer tax and a future soundness bug waiting to happen.
The SAFETY: comment rule
Every unsafe block, function, impl, or trait must be accompanied by a SAFETY: comment that states the invariant the caller/author is relying on. This is non-negotiable and matches the convention across the Rocky codebase.
The comment format:
unsafe { ... }
Two real examples from the codebase (rg "SAFETY:" engine/crates/ to see them live):
let mmap = unsafe { memmap2::Mmap::map(&file)? };
unsafe { std::env::set_var("ROCKY_TEST_VAR", "hello_world") };
What a good SAFETY: comment does:
- States the invariant the
unsafe call depends on (not the API's contract — the thing the caller is promising).
- Justifies why that invariant holds here (data shape, locking, single-threaded context, read-only guarantee, etc.).
- Mentions the fallback if the invariant is violated, when possible (e.g. "worst case is a parse error, not UB").
What a bad SAFETY: comment looks like:
// SAFETY: this is safe — useless; delete and re-write.// SAFETY: the docs say so — where? Cite the exact contract.// SAFETY: tested — tests don't prove memory safety.- No comment at all — blocks review.
Module-level safety docs
If an entire module's purpose is to wrap unsafe primitives behind a safe interface, document that at the module level using //! # Safety. rocky-core/src/mmap.rs is the reference:
Follow this shape whenever you introduce a new module containing unsafe. The doc-level section complements the inline SAFETY: comments — the module-level explains why this module needs unsafe at all, the inline explains why each specific site is sound.
Public unsafe fn declarations also need a # Safety section in their doc comment — see the rust-doc skill.
When to push back on new unsafe
Before merging a PR that adds new unsafe, ask:
- Is there a safe alternative? Most of the time there is —
std::cell::UnsafeCell → RefCell, raw pointer arithmetic → slice::split_at, manual bit-twiddling → bytemuck, hand-rolled FFI → a safe crate wrapper.
- Is the performance win measured?
mmap.rs justifies itself with "50k+ files"; speculative performance claims are not enough.
- Is the invariant stable? An invariant that holds today but could be invalidated by an unrelated refactor is a soundness bomb. If you can't describe a test or type-level property that will break loudly when the invariant fails, reconsider.
- Who else reads this?
rocky-core is the library that everything else links against. A bug there is a bug everywhere — the bar for new unsafe in rocky-core is higher than in a leaf adapter crate.
If you can't answer all four, leave the safe implementation in place and open a perf issue instead.
Linting and auditing
grep surface
rg -n '\bunsafe\b' crates/
rg -n 'SAFETY:' crates/
rg -nB1 '\bunsafe\s*\{' crates/ | rg -v 'SAFETY:'
If the last command shows a site that doesn't have a SAFETY: comment within the surrounding 1-2 lines, that's a bug to fix before merge.
Lints
Rocky runs cargo clippy --all-targets -- -D warnings. The relevant clippy lints for unsafe are:
clippy::undocumented_unsafe_blocks — fires on unsafe { ... } without a SAFETY: comment. Not in the default set, so it currently doesn't enforce the rule at CI level — but it's the right lint to consider if you want to make the rule a machine-checkable policy. (Adding it to [workspace.lints] is a policy change — see the rust-clippy-triage skill for the rule on workspace lint changes.)clippy::multiple_unsafe_ops_per_block — fires if one unsafe { ... } block does more than one unsafe operation. Splitting them makes each SAFETY: comment tighter.
Related skills
rust-doc — public unsafe fn needs a # Safety section in its doc comment.rust-clippy-triage — how to react when an unsafe-related lint fires, and why workspace-level lint changes need Hugo review.rust-style — the wildcard-match rule is load-bearing around unsafe too: every enum variant you forget is one you're silently assuming doesn't exist.
