fix(rust-backend): borrow the argument that lands in a &mut slice slot - #3405
Merged
Conversation
Refs #3402 #3403 rewrote a written `[]T` parameter to `&mut [T]` and never rewrote the argument. Adversarial review measured the gap: of 608 call-site arguments landing in a slice parameter position across the generated corpus, zero gained a borrow. So `tritwise_and(a, b, temp, len)` read expected `&mut [i32]`, found `[i32; 27]` with a correct signature and an uncorrected call. A call site now borrows the argument that lands in a marked slot. It does NOT borrow one that is already a `&mut [T]` parameter of the calling function: passing a borrow onward is a reborrow, and a second `&mut` is an error. That distinction cannot be made from the argument text alone, which is why `current_mut_slice_params` exists; `collect_param_names` supplies the map from argument POSITION to the parameter NAME that `written_slice_params` is keyed by. Measured over all 650 specs against master: total diagnostics 3773 -> 3745 move/borrow (E0382 + E0596) 33 files / 143 -> 24 / 117 rustc accepts 430 -> 430 specs/isa/ternary_bitwise 12 errors -> 3, across both halves 41 files' output changed, 7 strictly better, 5 show a higher count of one class -- all E0615 on `data.len`, already verified as blaming byte-identical lines reached only because an earlier error no longer aborts the compile (#3402). The corpus value of this half is two diagnostics. It is worth having regardless: passing a local buffer into a kernel is exactly the shape a ported hand-written kernel needs, and without it the parameter fix is unusable from any caller that does not already hold a borrow. Writing the test surfaced a third, separate defect: `var tmp : [4]i32 = undefined;` lowers to `let mut tmp: [i32; 4];` with no initialiser, so E0381 fires before the call is type-checked at all. The fixture uses `[_]i32{0, 0, 0, 0}` so the test measures the rule it exists for. Mutation-checked twice: removing the reborrow guard fails three tests, removing the borrow fails two. FROZEN_HASH updated in the same commit, as M5 requires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gHashTag
enabled auto-merge (squash)
September 7, 2026 19:37
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Contributor
This was referenced Sep 7, 2026
Closed
gHashTag
added a commit
that referenced
this pull request
Sep 7, 2026
…rom now on (#3416) Closes #3415 A seal records four `gen_hash_*` fields -- the hashes of what each backend emitted. Nothing checked them. Measured on master: 109 stale gen_hash_rust, 7 stale gen_hash_zig, 0 C, 0 Verilog, and 0 stale spec_hash. All 116 were left by five merged backend repairs -- #3401, #3403, #3405, #3407, #3411 -- every one of them mine. None touched a spec, so spec_hash stayed correct and every coverage and staleness check in the repository stayed green while a sixth of the Rust seals described output the compiler no longer produces. The existing checks cover the other half. check_seal_coverage.py asks whether a seal describes a spec that exists, unchanged at SOURCE. `Seal Staleness Warning` is about the NMSE manifest and FROZEN_HASH, unrelated to .trinity/seals, and exits 0 by design. `t27c seal --verify` answers this question exactly -- exit 1 with a precise MISMATCH line, exit 0 on a current seal, both used as controls here -- and nothing called it across the corpus. The refresh took three attempts and each failure was informative. `--save` fixed 116 -> 58, not 0, because it writes to .trinity/seals/<module>.json, one name, while 1313 seals cover 728 distinct specs and 547 specs carry more than one seal file. The remaining 58 duplicates are rewritten in place. `sealed_at` is left alone in those: rewriting a hash the tool itself just computed is not a fresh certification event, and moving the timestamp would claim one. gen_hash=none is a different debt and is counted apart -- 169 seals record it for at least one backend, and --save refuses to overwrite them ("4 of 4 backends rejected it"). Conflating the two is how the 116 stayed invisible. tools/check_seal_currency.py now asks the question. Its --self-check plants a wrong hash on a scratch tree and requires exactly that seal to be reported, because a zero from a check that cannot see is indistinguishable from a healthy zero. It exits 2 when t27c is absent rather than 0: a check that could not run has not passed. Filed, not guessed: some duplicate seals are not named after a module at all. specs/tri/utils/logger.t27 carries `"[]const u8".json`, `utils_"[]const u8".json` and `utils_TriLogger.json` -- a seal named after a type string. After this: 1318 seals scanned, 1055 current, 94 whose spec is gone, 169 sealed with none, 0 stale. Co-authored-by: lab <lab@example.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #3402 — the second half of #3403, found by adversarial review of it.
The gap
#3403 rewrote a written
[]Tparameter to&mut [T]and never rewrote the argument. The reviewer measured it exactly: of 608 call-site arguments landing in a slice parameter position across the generated corpus, 0 gained a borrow. SoA correct signature reached by an uncorrected call.
The rule has two branches, so the test has two cases
A call site now borrows the argument that lands in a marked slot:
and does not borrow one that is already a
&mut [T]parameter of the calling function:That distinction cannot be made from the argument text alone, which is what
current_mut_slice_paramsis for.collect_param_namessupplies the missing map: a call site knows argument positions,written_slice_paramsis keyed by parameter names.Measured, all 650 specs against master
specs/isa/ternary_bitwise.t27, errors41 files’ output changed; 7 strictly better; 5 show a higher count of one class — all
E0615ondata.len, already verified in #3403 as blaming byte-identical lines reached only because an earlier error no longer aborts the compile. That defect is filed in #3402.The corpus value of this half is two diagnostics, and I would rather say so than dress it up. It is worth having anyway: passing a local buffer into a kernel is exactly the shape a ported hand-written kernel needs, and without it the parameter fix is unusable from any caller that does not already hold a borrow.
A third defect, surfaced by writing the test
var tmp : [4]i32 = undefined;lowers tolet mut tmp: [i32; 4];with no initialiser, soE0381fires before the call is type-checked at all. The first draft of the runtime test failed on that and would have been measuring the wrong thing; the fixture now uses[_]i32{0, 0, 0, 0}.Tests
Two new, 15 green in the file. One checks both branches of the rule textually, the other compiles and runs and asserts the local array actually receives the write.
Mutation-checked twice, one variable each:
&mutbreaks it too);bootstrap/stage0/FROZEN_HASHupdated in the same commit, as M5 requires.