Skip to content

Commit 4bcb6c0

Browse files
Haofeiclaude
andcommitted
feat(lang): scalar field assignment through mut parameters (Step 3, SH-013)
`m.field = expr` on a `mut` parameter now works and propagates to the caller on every backend, removing the "List<Int> as a mutable cell" smell that self-hosting the Mailbox forced (SH-007). - Checker: a `mut` parameter (new AssignBinding::MutParam) permits field/index assignment (still rejects rebinding the parameter itself). - VM: CallKnown carries the callee's mut-param positions; on frame completion (every return path: Return, tier-0, native, `?`-early, fall-off-end) each mut arg's final value is written back to the caller's register (apply_mut_writeback), matching AOT's &mut semantics. Backward compatible: empty mut_args is a no-op; List-based propagation is unchanged. - Inliner skips mut-arg calls (they need the write-back). Also noted: core already ships `Counter` (mut-scalar container). New test backends_agree_on_mut_param_field_assignment (5-way). Full gate green: feature differential 22/22, vm 112, corpus, checker 212/149, clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 816d8fc commit 4bcb6c0

4 files changed

Lines changed: 125 additions & 8 deletions

File tree

docs/rss-selfhost-ledger.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,27 @@ Status: open | decided | done
282282
within noise; was 345 vs 326). Telemetry `considered: 0` after warmup.
283283
- **Tests:** feature differential 21/21 (behavior-neutral).
284284
- **Status:** done.
285+
286+
### SH-013 — scalar field assignment through a `mut` parameter (fixed)
287+
288+
- **Tool:** Mailbox<T> (the List<Int>-as-cell smell, SH-007)
289+
- **Symptom:** `m.count = m.count + 1` on a `mut` param was rejected (RS0311), so
290+
the mailbox held mutable scalars in 1-element lists and recomputed `count`.
291+
- **Root cause:** (1) the checker rejected any assignment rooted in a parameter;
292+
(2) the VM copies `CallKnown` args into the callee window with no write-back, so
293+
even if allowed, scalar field mutations wouldn't propagate (only `List` fields
294+
did, via their shared `RefCell`). AOT already had `&mut` semantics.
295+
- **Classification:** language (checker) + VM (call semantics).
296+
- **Decision (DONE):**
297+
- Checker: a `mut` parameter (`AssignBinding::MutParam`) allows field/index
298+
assignment (not bare rebinding).
299+
- VM: `CallKnown` carries the callee's `mut`-param positions; when the frame
300+
completes (any return path), each `mut` arg's final value is written back to
301+
the caller's register (`apply_mut_writeback`), matching AOT's `&mut`.
302+
- Backward compatible: empty `mut_args` ⇒ no-op; List-based code already
303+
propagated and is unchanged.
304+
- **Note:** core already ships `Counter` (`Counter.new/add/value`, a `mut`-scalar
305+
container) — the stdlib alternative the review suggested already exists.
306+
- **Tests:** `backends_agree_on_mut_param_field_assignment` (5-way). Full gate
307+
green: feature differential 22/22; vm 112; corpus; checker 212/149.
308+
- **Status:** done.

src/analyzer.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4897,6 +4897,9 @@ enum AssignBinding {
48974897
MutLocal,
48984898
ImmutableLocal,
48994899
Param,
4900+
/// A `mut` parameter: not reassignable itself, but its fields/elements may be
4901+
/// updated in place (the mutation propagates to the caller, matching `&mut`).
4902+
MutParam,
49004903
}
49014904

49024905
#[derive(Clone)]
@@ -4929,11 +4932,12 @@ impl<'a> AssignChecker<'a> {
49294932
self.scopes.clear();
49304933
self.push_scope();
49314934
for param in &function.params {
4932-
self.insert(
4933-
param.name.clone(),
4934-
AssignBinding::Param,
4935-
Some(type_ref_name(&param.ty)),
4936-
);
4935+
let binding = if param.effect == Some(DataEffect::Mut) {
4936+
AssignBinding::MutParam
4937+
} else {
4938+
AssignBinding::Param
4939+
};
4940+
self.insert(param.name.clone(), binding, Some(type_ref_name(&param.ty)));
49374941
}
49384942
self.block(&function.body);
49394943
self.pop_scope();
@@ -5172,9 +5176,9 @@ impl<'a> AssignChecker<'a> {
51725176
format!("`{name}` is an immutable binding"),
51735177
format!("Declare `{name}` with `let mut` to allow reassignment."),
51745178
),
5175-
Some(AssignBinding::Param) => (
5179+
Some(AssignBinding::Param | AssignBinding::MutParam) => (
51765180
format!("`{name}` is a parameter, not a reassignable local"),
5177-
"Parameters are not reassignable. Bind a `let mut` local, or mutate through a `mut` parameter."
5181+
"Parameters are not reassignable (even `mut` ones): a `mut` parameter's fields/elements may be updated, but the parameter binding itself can't be rebound. Bind a `let mut` local instead."
51785182
.to_string(),
51795183
),
51805184
None => (
@@ -5244,6 +5248,9 @@ impl<'a> AssignChecker<'a> {
52445248
};
52455249
match self.resolve(root) {
52465250
Some(AssignBinding::MutLocal) => {}
5251+
// A `mut` parameter's fields/elements may be updated in place (the
5252+
// mutation propagates to the caller, like `&mut`).
5253+
Some(AssignBinding::MutParam) => {}
52475254
Some(AssignBinding::ImmutableLocal) => {
52485255
self.diagnostics.push(invalid_assignment_diagnostic(
52495256
span.clone(),

src/reg_vm/mod.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,9 +592,12 @@ fn native_inline_leaf_calls(unit: &RegUnit, func: &RegFunction) -> Option<(Vec<R
592592
dst,
593593
function,
594594
args,
595+
mut_args,
595596
} => {
596597
let callee = unit.functions.get(*function)?;
597-
if !native_callee_inlinable(callee, args.len()) {
598+
// Calls with `mut` args need a write-back at return; don't inline
599+
// them (native-inlinable callees are side-effect-free anyway).
600+
if !mut_args.is_empty() || !native_callee_inlinable(callee, args.len()) {
598601
return None;
599602
}
600603
let base = next_reg;
@@ -1676,6 +1679,12 @@ enum RegInstr {
16761679
dst: Reg,
16771680
function: usize,
16781681
args: Vec<Reg>,
1682+
/// Argument positions passed with `mut` (the callee's `mut` params). After
1683+
/// the call returns, each such argument's (possibly mutated) value is
1684+
/// written back to the caller's argument register, so a `mut` parameter's
1685+
/// field/element mutations propagate to the caller — matching AOT's
1686+
/// `&mut` semantics.
1687+
mut_args: Vec<usize>,
16791688
},
16801689
/// `spawn f(args)` / `async let`: start `function` as a new concurrent task
16811690
/// and put a Task handle in `dst` (the spawning task keeps running).
@@ -3733,10 +3742,12 @@ impl RegLowerer<'_> {
37333742
// the generics before the lookup — otherwise a generic *function*
37343743
// call falls through and is mis-lowered as a struct construction.
37353744
if let Some(function) = self.function_ids.get(type_root_name(name)).copied() {
3745+
let mut_args = self.user_mut_arg_positions(name);
37363746
self.emit(RegInstr::CallKnown {
37373747
dst,
37383748
function,
37393749
args: arg_regs,
3750+
mut_args,
37403751
});
37413752
} else if self.is_native_function(None, name) {
37423753
let mut_args = self.native_mut_arg_positions(None, name);
@@ -5038,10 +5049,13 @@ impl RegLowerer<'_> {
50385049
return Ok(dst);
50395050
}
50405051
if let Some(function) = self.function_ids.get(&qualified_key).copied() {
5052+
let mut_args =
5053+
self.native_mut_arg_positions(Some(namespace_root), name_root);
50415054
self.emit(RegInstr::CallKnown {
50425055
dst,
50435056
function,
50445057
args: arg_regs,
5058+
mut_args,
50455059
});
50465060
return Ok(dst);
50475061
}
@@ -5106,6 +5120,12 @@ impl RegLowerer<'_> {
51065120
.unwrap_or_default()
51075121
}
51085122

5123+
/// `mut` parameter positions of a user function, so a `CallKnown` can write
5124+
/// the mutated arguments back to the caller (matching AOT's `&mut` params).
5125+
fn user_mut_arg_positions(&self, name: &str) -> Vec<usize> {
5126+
self.native_mut_arg_positions(None, name)
5127+
}
5128+
51095129
fn variant_match(&mut self, value: &HirExpr, arms: &[HirMatchArm]) -> Result<bool, EvalError> {
51105130
if arms.is_empty()
51115131
|| !arms
@@ -5842,6 +5862,11 @@ struct Frame {
58425862
/// Absolute register in the caller that receives this frame's return value.
58435863
/// `usize::MAX` marks a driver root (its value is returned out of `run_frame`).
58445864
ret_dst: usize,
5865+
/// `mut`-argument write-backs to perform when this frame completes:
5866+
/// `(caller_abs_reg, this_frame_abs_reg)` pairs. The caller register receives
5867+
/// the parameter's final (possibly mutated) value, so `mut` params propagate.
5868+
/// Empty for the overwhelmingly common no-`mut`-arg call (then a no-op).
5869+
mut_writeback: Vec<(usize, usize)>,
58455870
}
58465871

58475872
/// Result of driving a task's call stack one slice at a time.
@@ -6448,6 +6473,7 @@ impl RegVm {
64486473
dst,
64496474
function: callee_id,
64506475
args,
6476+
mut_args,
64516477
} = instr
64526478
{
64536479
let callee = Rc::clone(&unit.functions[*callee_id]);
@@ -6458,6 +6484,11 @@ impl RegVm {
64586484
self.set_reg(next_base + index, value);
64596485
}
64606486
let result = self.run_frame(unit, callee, next_base)?;
6487+
// Propagate `mut` parameters back to the caller's argument regs.
6488+
for &pos in mut_args {
6489+
let value = self.reg(next_base + pos).clone();
6490+
self.set_reg(base + args[pos], value);
6491+
}
64616492
self.set_reg(base + *dst, result);
64626493
continue;
64636494
}
@@ -7022,6 +7053,16 @@ impl RegVm {
70227053
self.written[index] = true;
70237054
}
70247055

7056+
/// Propagate a completing frame's `mut` parameters back to the caller: each
7057+
/// `(caller_reg, callee_reg)` copies the parameter's final value out. A no-op
7058+
/// for the common call with no `mut` args (empty `mut_writeback`).
7059+
fn apply_mut_writeback(&mut self, frame: &Frame) {
7060+
for &(caller_reg, callee_reg) in &frame.mut_writeback {
7061+
let value = self.reg(callee_reg).clone();
7062+
self.set_reg(caller_reg, value);
7063+
}
7064+
}
7065+
70257066
#[inline(always)]
70267067
fn take_reg(&mut self, index: usize) -> VmValue {
70277068
assert!(
@@ -7055,6 +7096,7 @@ impl RegVm {
70557096
ip: 0,
70567097
base,
70577098
ret_dst: usize::MAX,
7099+
mut_writeback: Vec::new(),
70587100
});
70597101
match self.drive(unit, floor)? {
70607102
Outcome::Completed(value) => Ok(value),
@@ -7095,6 +7137,7 @@ impl RegVm {
70957137
ip: 0,
70967138
base: 0,
70977139
ret_dst: usize::MAX,
7140+
mut_writeback: Vec::new(),
70987141
}];
70997142
self.tasks.insert(
71007143
tid,
@@ -7415,6 +7458,7 @@ impl RegVm {
74157458
&& let Some(value) = self.try_native(&func, base)
74167459
{
74177460
let frame = self.frames.pop().expect("active frame");
7461+
self.apply_mut_writeback(&frame);
74187462
if self.frames.len() == floor {
74197463
return Ok(Outcome::Completed(value));
74207464
}
@@ -7429,6 +7473,7 @@ impl RegVm {
74297473
if self.jit_enabled && ip == 0 && self.is_jit_eligible(&func) {
74307474
let value = self.run_jit(unit, &func, base)?;
74317475
let frame = self.frames.pop().expect("active frame");
7476+
self.apply_mut_writeback(&frame);
74327477
if self.frames.len() == floor {
74337478
return Ok(Outcome::Completed(value));
74347479
}
@@ -7447,6 +7492,7 @@ impl RegVm {
74477492
PureStep::Next => {}
74487493
PureStep::Return(value) => {
74497494
let frame = self.frames.pop().expect("active frame");
7495+
self.apply_mut_writeback(&frame);
74507496
if self.frames.len() == floor {
74517497
return Ok(Outcome::Completed(value));
74527498
}
@@ -7462,13 +7508,21 @@ impl RegVm {
74627508
dst,
74637509
function: callee_id,
74647510
args,
7511+
mut_args,
74657512
} => {
74667513
let callee = Rc::clone(&unit.functions[*callee_id]);
74677514
self.prepare_frame(next_base, callee.regs);
74687515
for (index, reg) in args.iter().enumerate() {
74697516
let value = self.reg(base + *reg).clone();
74707517
self.set_reg(next_base + index, value);
74717518
}
7519+
// `mut` args: when this frame completes, write each
7520+
// parameter's final value back to the caller's register
7521+
// so mutations propagate (caller_abs_reg, callee_abs_reg).
7522+
let mut_writeback = mut_args
7523+
.iter()
7524+
.map(|&pos| (base + args[pos], next_base + pos))
7525+
.collect();
74727526
// Stackless call: save our resume point, push the callee, and
74737527
// re-enter the driver loop instead of recursing on the host
74747528
// stack — so an `await` deep in this chain can later suspend it.
@@ -7478,6 +7532,7 @@ impl RegVm {
74787532
ip: 0,
74797533
base: next_base,
74807534
ret_dst: base + *dst,
7535+
mut_writeback,
74817536
});
74827537
continue 'frames;
74837538
}
@@ -7839,6 +7894,7 @@ impl RegVm {
78397894
// whole stackless driver.
78407895
let err_value = value_err(error);
78417896
let frame = self.frames.pop().expect("active frame");
7897+
self.apply_mut_writeback(&frame);
78427898
if self.frames.len() == floor {
78437899
return Ok(Outcome::Completed(err_value));
78447900
}
@@ -7867,6 +7923,7 @@ impl RegVm {
78677923
// Fell off the end of the function body without an explicit `Return`.
78687924
// Lowering always appends one, so this is a defensive `Unit` return.
78697925
let frame = self.frames.pop().expect("active frame");
7926+
self.apply_mut_writeback(&frame);
78707927
if self.frames.len() == floor {
78717928
return Ok(Outcome::Completed(VmValue::Unit));
78727929
}

tests/backend_differential.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,35 @@ fn backends_agree_on_manifest_inspector() {
543543
common::differential::assert_backends_agree("selfhost-manifest-inspector.rss", source, &[fixture]);
544544
}
545545

546+
/// Scalar field assignment through a `mut` parameter must propagate to the caller
547+
/// on every backend (VM write-back == AOT `&mut`). Regression for ledger SH-013.
548+
#[test]
549+
fn backends_agree_on_mut_param_field_assignment() {
550+
let source = "features: local\n\
551+
\n\
552+
struct Tally {\n\
553+
n: Int\n\
554+
}\n\
555+
\n\
556+
fn bump(c: mut Tally) -> Unit {\n\
557+
c.n = c.n + 1\n\
558+
}\n\
559+
\n\
560+
fn add(c: mut Tally, amount: Int) -> Unit {\n\
561+
c.n = c.n + amount\n\
562+
}\n\
563+
\n\
564+
fn main() -> Unit {\n\
565+
let mut c = Tally(n: 0)\n\
566+
bump(c: mut c)\n\
567+
bump(c: mut c)\n\
568+
add(c: mut c, amount: 10)\n\
569+
Log.write(message: read String.from_int(value: c.n))\n\
570+
return Unit\n\
571+
}\n";
572+
common::differential::assert_backends_agree("mut-param-field.rss", source, &[]);
573+
}
574+
546575
/// A generic collection implemented in RSS itself: `benchmark/selfhost_mailbox.rss`
547576
/// (a fixed-capacity `Mailbox<T>` over parallel lists, with oldest-first and
548577
/// source-filtered takes). Regression for the generic-call lowering fix (a generic

0 commit comments

Comments
 (0)