|
| 1 | +# Verify Rust Standard Library — Code Review Guidelines |
| 2 | + |
| 3 | +## Purpose & Scope |
| 4 | + |
| 5 | +This repository is a fork of the Rust standard library used exclusively for formal verification. Verification targets include memory safety, undefined behavior, and functional correctness, depending on the specific challenge. PRs fall into four categories: **solving challenges**, **proposing new challenges**, **proposing new tools**, and **maintenance**. |
| 6 | + |
| 7 | +## Approved Verification Tools |
| 8 | + |
| 9 | +Only these tools are accepted: **Kani**, **VeriFast**, **Flux**, and **ESBMC (GOTO-Transcoder)**. Flag any PR that uses a tool not on this list — it must go through a separate tool application process first. |
| 10 | + |
| 11 | +--- |
| 12 | + |
| 13 | +## General Rules (All PRs) |
| 14 | + |
| 15 | +- The contribution must be automated and pass as part of PR CI checks. |
| 16 | +- Changes must not alter the runtime logic of the standard library unless the change is proposed and incorporated upstream into the official Rust standard library. |
| 17 | +- All contributors must be properly credited. By submitting, contributors confirm they can use, modify, copy, and redistribute their contribution. |
| 18 | +- PRs should reference the relevant tracking issue (e.g., `Resolves #ISSUE-NUMBER`). |
| 19 | +- Merging requires approval from at least two committee members listed in `.github/pull_requests.toml`. |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## Challenge Solutions (Highest Priority) |
| 24 | + |
| 25 | +These PRs solve open verification challenges. Apply the strictest review criteria: |
| 26 | + |
| 27 | +### Success Criteria Compliance |
| 28 | +- Verify the PR meets **every** success criterion listed in the specific challenge description. Partial solutions should be flagged. |
| 29 | +- Check the challenge page at `doc/src/challenges/` or the [challenge book](https://model-checking.github.io/verify-rust-std/challenges/) for the exact requirements. |
| 30 | + |
| 31 | +### Formal Verification Rigor |
| 32 | +- **No vacuous proofs.** Ensure harnesses and contracts actually exercise the property under verification. A proof that passes trivially (e.g., with an unsatisfiable precondition or an empty harness body) is unacceptable. |
| 33 | +- **No unjustified assumptions.** Every `#[kani::assume]`, `requires`, or equivalent precondition must be justified by the function's documented safety contract or API invariants. Flag assumptions that over-constrain inputs to make verification pass. |
| 34 | +- **Contracts must reflect documented safety conditions.** Cross-reference preconditions and postconditions against the function's `# Safety` doc comments and the [official std library documentation](https://doc.rust-lang.org/std/). |
| 35 | +- **Harnesses must cover meaningful input ranges**, not just trivial or degenerate cases. |
| 36 | +- **Verification must actually run in CI** using one of the approved tools. Check that the relevant workflow (kani.yml, verifast.yml, flux.yml, goto-transcoder.yml) executes the new proofs. |
| 37 | + |
| 38 | +### Code Quality for the Rust Standard Library |
| 39 | +- Code should be **idiomatic Rust** and match the style of the surrounding standard library code. |
| 40 | +- Unsafe code blocks must have a `// SAFETY:` comment explaining why the usage is sound. |
| 41 | +- Use `#[inline]` only on public, small, non-generic functions. Do not add `#[inline]` to generic methods or trait methods without default implementations. Avoid `#[inline(always)]` unless justified by benchmarks. |
| 42 | +- Contracts and harness code should be **readable and well-documented** so that someone unfamiliar can understand what property is being verified and why. |
| 43 | +- Verification annotations (contracts, harnesses, proof attributes) should be clearly separated from the library's functional code using `cfg` attributes (e.g., `#[cfg(kani)]`) so they don't affect normal compilation. |
| 44 | +- Do not introduce new public API surface unless the challenge explicitly requires it. |
| 45 | + |
| 46 | +--- |
| 47 | + |
| 48 | +## New Challenge Proposals |
| 49 | + |
| 50 | +- Must follow the template at `doc/src/challenge_template.md`. |
| 51 | +- Must include a tracking issue created with the challenge issue template. |
| 52 | +- Must add an entry in `doc/src/SUMMARY.md`. |
| 53 | +- Success criteria must be specific, measurable, and achievable with the currently approved tools. |
| 54 | +- Flag vague or overly broad success criteria. |
| 55 | + |
| 56 | +--- |
| 57 | + |
| 58 | +## New Tool Applications |
| 59 | + |
| 60 | +- Must be submitted as a separate issue using the "Tool Application" template before any PR using the tool. |
| 61 | +- The PR must include a new CI workflow that runs the tool against the standard library. |
| 62 | +- The PR must add a new entry to the "Approved Tools" section of the book. |
| 63 | + |
| 64 | +--- |
| 65 | + |
| 66 | +## Maintenance PRs |
| 67 | + |
| 68 | +- Repository updates, CI fixes, documentation improvements, and dependency bumps. |
| 69 | +- Should not change verification semantics or weaken existing proofs. |
| 70 | +- Flag any maintenance PR that removes or weakens existing contracts, harnesses, or proof obligations. |
| 71 | + |
| 72 | +--- |
| 73 | + |
| 74 | +## Common Red Flags to Watch For |
| 75 | + |
| 76 | +- Harness with no assertions or only trivial assertions. |
| 77 | +- `kani::assume(false)` or preconditions that make the proof vacuously true. |
| 78 | +- Assumptions that are stricter than what the function's safety docs require. |
| 79 | +- Contracts that don't match the `# Safety` section of the function being verified. |
| 80 | +- Verification code that is not gated behind a tool-specific `cfg` (e.g., `#[cfg(kani)]`). |
| 81 | +- Changes to `library/` source files that alter runtime behavior without upstream justification. |
| 82 | +- Missing or incorrect tracking issue references. |
| 83 | +- Use of a verification tool not in the approved list. |
| 84 | +- Large, hard-to-review PRs that should be split into smaller logical units. |
0 commit comments