diff --git a/bootstrap/src/compiler.rs b/bootstrap/src/compiler.rs index 656162ddf3..dcdefefc3b 100644 --- a/bootstrap/src/compiler.rs +++ b/bootstrap/src/compiler.rs @@ -21944,6 +21944,24 @@ fn parse_int_value(s: &str) -> Option { /// The argument must be a bare identifier naming a slice parameter of the /// caller. Anything else -- a literal, an index, a call -- is not a parameter /// being threaded through and is left alone rather than guessed at. +/// Each function's parameter names, in order. A call site knows argument +/// POSITIONS; `written_slice_params` is keyed by parameter NAME, and this is +/// the map between them. +fn collect_param_names( + node: &Node, + out: &mut std::collections::HashMap>, +) { + if node.kind == NodeKind::FnDecl { + out.insert( + node.name.clone(), + node.params.iter().map(|(n, _)| n.clone()).collect(), + ); + } + for c in &node.children { + collect_param_names(c, out); + } +} + fn collect_written_slice_params( ast: &Node, ) -> std::collections::HashMap> { @@ -23782,6 +23800,13 @@ pub struct RustCodegen { /// being threaded into a callee that writes them. Emitted as `&mut [T]`. written_slice_params: std::collections::HashMap>, + /// Parameter names by position, per function. Pairs with the map above so a + /// call site can ask "is argument 2 a `&mut [T]` slot?". + param_names: std::collections::HashMap>, + /// The slice parameters of the function currently being emitted that are + /// themselves `&mut [T]`. Passing one of those on is a reborrow and must + /// NOT get another `&mut`. + current_mut_slice_params: std::collections::HashSet, /// True while the initialiser of a `pub const` is being emitted. Rust's /// float methods are not `const fn`, so a builtin rewrite is invalid there /// in any spelling: `(x).sqrt()` is E0015 where the bare `sqrt(x)` was @@ -23851,6 +23876,8 @@ impl RustCodegen { declared_fns: std::collections::HashSet::new(), in_const_init: false, written_slice_params: std::collections::HashMap::new(), + param_names: std::collections::HashMap::new(), + current_mut_slice_params: std::collections::HashSet::new(), bool_vars: std::collections::HashSet::new(), bool_module_vars: std::collections::HashSet::new(), bool_fields: std::collections::HashSet::new(), @@ -23972,6 +23999,8 @@ impl RustCodegen { // Whole-tree, because the fixpoint below follows calls across functions. self.written_slice_params = collect_written_slice_params(ast); + self.param_names.clear(); + collect_param_names(ast, &mut self.param_names); // Same pre-pass, for the integer widths used by `infer_int_type`: // callee return types and module-level constants are both visible from @@ -24274,6 +24303,7 @@ impl RustCodegen { // Deliberately narrow: only the written-into slices move to `&mut [T]`. // A read-only `Vec` parameter is valid Rust that compiles today, and // rewriting it to `&[T]` would change call sites for no defect. + let mut mut_slices: Vec = Vec::new(); let empty = std::collections::HashSet::new(); let written = self .written_slice_params @@ -24321,6 +24351,7 @@ impl RustCodegen { // with itself. A parameter the fixpoint does not mark is left // EXACTLY as it was, which is the no-op this comment promises. if let (true, true, Some(elem)) = (is_slice, written.contains(n), elem) { + mut_slices.push(n.clone()); format!("{}: &mut [{}]", rust_ident(n), elem) } else { format!("{}: {}", rust_ident(n), rust_ty) @@ -24354,6 +24385,7 @@ impl RustCodegen { used.iter().map(|s| s.as_str()).collect::>().join(", ") ) }; + self.current_mut_slice_params = mut_slices.into_iter().collect(); self.write(&format!( "pub fn {}{}({}) -> {} {{", fn_name, generics, params_str, ret_type @@ -25520,11 +25552,38 @@ impl RustCodegen { } } NodeKind::ExprCall => { - let args: Vec = node + let mut args: Vec = node .children .iter() .map(|c| self.expr_to_rust(c)) .collect(); + // A parameter that became `&mut [T]` needs its ARGUMENT + // borrowed. Rewriting the parameter and not the argument left + // `tritwise_and(a, b, temp, len)` reading + // "expected `&mut [i32]`, found `[i32; 27]`" -- the signature + // was right and the call was not. + // + // A caller passing on its OWN `&mut [T]` parameter is + // reborrowing and must not get a second `&mut`; that is what + // `current_mut_slice_params` is for. It is also why this cannot + // be done by looking at the argument text alone. + if let Some(callee_params) = self.param_names.get(&node.name) { + if let Some(callee_written) = self.written_slice_params.get(&node.name) { + for (i, a) in args.iter_mut().enumerate() { + let Some(pname) = callee_params.get(i) else { + continue; + }; + if !callee_written.contains(pname) { + continue; + } + if self.current_mut_slice_params.contains(a.as_str()) { + continue; + } + *a = format!("&mut {}", a); + } + } + } + let args = args; if let Some(built) = Self::zig_builtin_to_rust(&node.name, &args) { return built; } diff --git a/bootstrap/stage0/FROZEN_HASH b/bootstrap/stage0/FROZEN_HASH index 98ad3eb721..989743a854 100644 --- a/bootstrap/stage0/FROZEN_HASH +++ b/bootstrap/stage0/FROZEN_HASH @@ -1 +1 @@ -7ddf8caecb91afe9e6a67ec04e7222256a3938ea19916f80122e34df27dbbe86 bootstrap/src/compiler.rs +ff93a9bf0dc2ba784e27d8154ebc90666dac834653236caa5e39fa159221811c bootstrap/src/compiler.rs diff --git a/bootstrap/tests/backend_behaviour.rs b/bootstrap/tests/backend_behaviour.rs index d432dc3237..39182134d8 100644 --- a/bootstrap/tests/backend_behaviour.rs +++ b/bootstrap/tests/backend_behaviour.rs @@ -444,3 +444,59 @@ fn only_the_written_slice_parameter_changes() { assert!(text.contains("b: Vec"), "read-only slice must be untouched, got:\n{text}"); assert!(text.contains("c: [i32; 3]"), "fixed array untouched, got:\n{text}"); } + +/// The argument half. Rewriting the PARAMETER to `&mut [T]` and leaving the +/// argument alone gave `tritwise_and(a, b, temp, len)` reading +/// "expected `&mut [i32]`, found `[i32; 27]`" -- the signature was right and +/// the call was not. A caller passing on its OWN `&mut [T]` parameter is +/// reborrowing and must NOT get a second `&mut`, so both shapes are here. +const SPEC_OUT_PARAM_CALLERS: &str = r#" +module callers { + fn fill(buf: []i32, n: usize) -> void { + var i : usize = 0; + while (i < n) { + buf[i] = 3; + i = i + 1; + } + } + fn via_local(n: usize) -> i32 { + // NOT `= undefined`: that lowers to `let mut tmp: [i32; 4];` with no + // initialiser and rustc answers E0381 before it ever type-checks the + // call, which is a different defect and would make this test measure it + // instead of the one it is here for. + var tmp : [4]i32 = [_]i32{0, 0, 0, 0}; + fill(tmp, n); + return tmp[0]; + } + fn via_param(buf: []i32, n: usize) -> void { + fill(buf, n); + } +} +"#; + +#[test] +fn a_local_array_is_borrowed_at_the_call_and_a_parameter_is_not() { + let Some(text) = rust_text(SPEC_OUT_PARAM_CALLERS, "rust-callsite-borrow") else { + return; + }; + assert!( + text.contains("fill(&mut tmp, n)"), + "a local array must be borrowed at the call, got:\n{text}" + ); + assert!( + text.contains("fill(buf, n)") && !text.contains("fill(&mut buf, n)"), + "passing on a &mut [T] parameter is a reborrow, not a second borrow, got:\n{text}" + ); +} + +#[test] +fn a_local_array_out_parameter_round_trips_at_runtime() { + let Some(out) = rust_says( + SPEC_OUT_PARAM_CALLERS, + "fn main(){ println!(\"{}\", via_local(4)); }\n", + "rust-callsite-run", + ) else { + return; + }; + assert_eq!(out, "3", "the local array must actually receive the write"); +} diff --git a/docs/now/2026-09-08-the-signature-was-right-and-the-call-was-not.md b/docs/now/2026-09-08-the-signature-was-right-and-the-call-was-not.md new file mode 100644 index 0000000000..06d9d5a42b --- /dev/null +++ b/docs/now/2026-09-08-the-signature-was-right-and-the-call-was-not.md @@ -0,0 +1,10 @@ +# NOW -- The signature was right and the call was not (2026-09-08) + +## The signature was right and the call was not (Refs #3402) + +- #3403 rewrote a written `[]T` PARAMETER to `&mut [T]` and never rewrote the ARGUMENT. Adversarial review measured it: of **608** call-site arguments landing in a slice parameter position across the generated corpus, **0** gained a borrow. `tritwise_and(a, b, temp, len)` therefore read "expected `&mut [i32]`, found `[i32; 27]`" -- a correct signature reached by an uncorrected call. +- Now a call site borrows the argument that lands in a marked slot, and does NOT borrow one that is already a `&mut [T]` parameter of the caller: passing a borrow onward is a reborrow, and a second `&mut` is an error. Both shapes are in one test, because a rule with two branches needs a case for each. +- Measured, all 650 specs against master: total diagnostics **3773 -> 3745**; move/borrow **33 files / 143 -> 24 / 117**; rustc acceptance unchanged at **430**. `specs/isa/ternary_bitwise.t27` alone goes **12 -> 3** errors across the two changes. 41 files' output changed, 7 strictly better, 5 show a higher count of one class and all 5 are E0615 on `data.len`, previously verified as REVEALED against byte-identical lines. +- The corpus value of this half is 2 diagnostics. It is worth having anyway: passing a local buffer to 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 test caught a THIRD defect while being written: `var tmp : [4]i32 = undefined;` lowers to `let mut tmp: [i32; 4];` with no initialiser, so E0381 fires before the call is ever type-checked. The fixture was changed to `[_]i32{0,0,0,0}` so the test measures the rule it is for and not that one. +- Mutation-checked twice: removing the reborrow guard fails 3 tests, removing the borrow itself fails 2.