Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ The generated loop is now written from the program's source alone, and the manif

### Fixed

- **Generated Rust builds when a record is updated after one of its fields was read.** `progress = flight.progress` followed by `Flight.update(flight, progress = f(progress))` inside a pair of functions that tail-call each other generated Rust that moved the field out and then moved the whole record, which rustc rejects (E0382). A loop that read a field in a `let` and later returned the whole record failed the same way. Such a field read now moves only when nothing reads that part of the record again and is copied otherwise, and the field read into `f` moves in both spellings (`f(progress)` and `f(flight.progress)`), so a Map `f` updates is not copied.
- **wasm-gc: `Map.set` no longer copies the map, and `Map.remove` no longer changes the map it was given.** `set` copied every bucket unless the compiler could prove the map had no other holder, which it cannot for a map held in a record field such as an answer module's state. A 100 000-entry state map served 2000 `Map.set` requests in 11 s under `aver run --wasm-gc` and 1.9 s on Node 26. `remove` wrote into the map it was given, so a caller that still held that map saw the key gone. Both now write into the map's arrays in place and return a new version. The version they were given stays valid, because a record of what the write replaced is kept with it. The same run now takes 0.4 s under `aver run --wasm-gc` and 0.3 s on Node. Reading an older version again costs one step for each write made since.
- **A program with the generated loop can key its own waits by a type of its own.** The loop keys its wait by `Int`, and a hand-written `Wait.poll` keyed by a sum beside it used to be refused by the Rust door ("this program keys one wait set by 'Int' and another by 'Watch'") and to fail wasm-gc validation. In a program that answers a capability of its own, each such wait, in the entry or in a dependency, now goes through helpers generated for its key type that carry it through an `Int`-keyed wait. It answers the same keys in the same order. Its recording holds the `Int`-keyed wait. The wasm-gc wait ABI is unchanged, and `--target wasip2` runs such waits too. A program that answers no capability still keys all its waits one way.
- **`check` no longer asks a verify block of a function no verify case can call.** A parameter of a capability resource type (`Tcp.Connection`, `Work.Job`, a job kind's handle), or of a tuple, record or sum of the module that always carries one, has no value a case can write, so such a pure branching helper failed `error[missing-verify]` with no way to satisfy it. It is now exempt, the way effectful functions are. A parameter with an empty value (`List`, `Option`, a sum with a resource-free variant) still needs its verify block.
Expand Down
24 changes: 22 additions & 2 deletions src/codegen/rust/from_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3669,6 +3669,22 @@ fn emit_mir_tail_value(expr: &Spanned<MirExpr>, ctx: &MirEmitCtx<'_>) -> Option<
Some(materialize_owned(code, &expr.node, ctx))
}

/// The value of one `let` statement in a loop or trampoline body. A named
/// binding owns its value, so it goes through [`emit_mir_binding_value`]
/// like every other binding: a field read moves only where `field_moves`
/// allows it and clones otherwise, so a record the chain reads a field of
/// stays whole for its later reads and updates. A bare `Int` binding and a
/// discarded value render as they are.
fn emit_mir_statement_value(
let_node: &crate::ir::mir::MirLet,
ctx: &MirEmitCtx<'_>,
) -> Option<String> {
if let_node.binding_name.is_empty() || ctx.bare.is_bare(let_node.binding) {
return emit_mir_expr(&let_node.value, ctx);
}
emit_mir_binding_value(&let_node.value, ctx)
}

/// Emit a `let` binding's value as an OWNED value.
///
/// Naming a read of a borrowed param does not change what a function may do
Expand Down Expand Up @@ -4081,7 +4097,7 @@ fn emit_mir_tco_body(
let mut current = body;
while let MirExpr::Let(spanned_let) = &current.node {
let let_node = &spanned_let.node;
let value = emit_mir_expr(&let_node.value, ctx)?;
let value = emit_mir_statement_value(let_node, ctx)?;
if let_node.binding_name.is_empty() {
lines.push(format!(" {};", value));
} else {
Expand Down Expand Up @@ -4378,6 +4394,10 @@ pub(super) fn emit_mir_mutual_tco_block(
// explicit restricted tags rather than the pre-rewrite whole-function
// facts so match subjects and their literals agree on `i64`.
policy.apply_rewritten_bare_i64(mir_fn, ctx);
// Each arm binds its params by value, so a field read may move out
// of one exactly as in any other body; the arm's updates then see
// which records gave a field up.
policy.apply_field_moves(mir_fn);
let mut arm_ctx = MirEmitCtx::for_fn(ctx, &policy);
// Mutual invariants are `rc_wrapped` for owning reads, but unlike
// self-TCO's `Arc<T>` representation they are extra `&T` trampoline
Expand Down Expand Up @@ -4575,7 +4595,7 @@ fn emit_mir_trampoline_body(
let mut current = body;
while let MirExpr::Let(spanned_let) = &current.node {
let let_node = &spanned_let.node;
let value = emit_mir_expr(&let_node.value, ctx)?;
let value = emit_mir_statement_value(let_node, ctx)?;
if let_node.binding_name.is_empty() {
// Discarded intermediate (`Stmt::Expr` / `_ = effect()`)
// — bare statement, result dropped.
Expand Down
34 changes: 34 additions & 0 deletions src/ir/mir/optimize/own_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ struct RustOwned {
/// Map/Vector params each fn updates in place
/// (`field_moves::in_place_collection_params`).
in_place_params: HashMap<FnId, Vec<bool>>,
/// Locals bound by a `let` whose value is one of the movable field
/// reads above (`progress = flight.progress`). The provenance table
/// holds copies of the binding values, whose addresses the movable set
/// cannot recognise, so these are named here from the body itself.
movable_let_slots: HashMap<FnId, HashSet<u32>>,
}

/// A single visible call edge: `target(args…)` made from `caller`.
Expand Down Expand Up @@ -386,6 +391,11 @@ fn own_param_refine_for_model(mut program: MirProgram, model: OwnershipModel) ->
for (id, f) in program.iter() {
let movable = crate::ir::mir::field_moves::movable_projections(&f.body.node);
if !movable.is_empty() {
let mut slots = HashSet::new();
collect_movable_let_slots(&f.body.node, &movable, &mut slots);
if !slots.is_empty() {
rust_owned.movable_let_slots.insert(*id, slots);
}
rust_owned.movable_projections.insert(*id, movable);
}
}
Expand Down Expand Up @@ -766,6 +776,19 @@ fn slot_owned(
{
return true;
}
// A let-bound field read generated Rust moves out of its record
// (`progress = flight.progress`). It shares the record's backing, which
// is why the alias table flags it, but the move leaves the record
// without it, exactly as a movable field read passed directly.
if !is_param
&& model.owned_carriers_are_cow_protected()
&& rust_owned
.movable_let_slots
.get(&caller)
.is_some_and(|slots| slots.contains(&slot))
{
return true;
}
// Flagged in the caller's table (RULE 1 param or RULE 2 intra-proc
// alias such as a Vector.get handle).
if caller_fn
Expand Down Expand Up @@ -850,6 +873,17 @@ fn collect_pattern_slots(pattern: &MirPattern, out: &mut HashSet<u32>) {
}

/// Collect `slot → binding-RHS` for every `Let` in the body.
/// The `let` locals of a body bound directly to one of its `movable` field
/// reads.
fn collect_movable_let_slots(e: &MirExpr, movable: &HashSet<usize>, out: &mut HashSet<u32>) {
if let MirExpr::Let(l) = e
&& movable.contains(&(&l.node.value.node as *const MirExpr as usize))
{
out.insert(l.node.binding.0);
}
walk_children(e, &mut |c| collect_movable_let_slots(c, movable, out));
}

fn collect_let_bindings(e: &MirExpr, out: &mut HashMap<u32, Spanned<MirExpr>>) {
if let MirExpr::Let(l) = e {
out.entry(l.node.binding.0)
Expand Down
6 changes: 3 additions & 3 deletions src/self_host/aver_generated/domain/eval/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ fn __mutual_tco_trampoline_1(
}
__MutualTco1::EvalMatch(mut v @ _, mut arms @ _, mut env @ _) => {
crate::cancel_checkpoint();
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco1::EvalExpr(arm.body.clone(), crate::aver_generated::domain::eval::store::mergeBindings(bindings, env)) }, Err(_) => { __MutualTco1::EvalMatch(v, rest, env) } })
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco1::EvalExpr(arm.body, crate::aver_generated::domain::eval::store::mergeBindings(bindings, env)) }, Err(_) => { __MutualTco1::EvalMatch(v, rest, env) } })
}
};
}
Expand Down Expand Up @@ -1208,7 +1208,7 @@ fn __mutual_tco_trampoline_2(
mut env @ _,
) => {
crate::cancel_checkpoint();
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco2::EvalTailExprSlot(selfId, arm.body.clone(), slotCount, crate::aver_generated::domain::eval::core::mergeBindingsSlot(&bindings, &arm.bindingSlots, &env)) }, Err(_) => { __MutualTco2::EvalTailMatchSlot(selfId, v, rest, slotCount, env) } })
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco2::EvalTailExprSlot(selfId, arm.body, slotCount, crate::aver_generated::domain::eval::core::mergeBindingsSlot(&bindings, &arm.bindingSlots, &env)) }, Err(_) => { __MutualTco2::EvalTailMatchSlot(selfId, v, rest, slotCount, env) } })
}
};
}
Expand Down Expand Up @@ -1984,7 +1984,7 @@ fn __mutual_tco_trampoline_3(
}
__MutualTco3::EvalMatchSlot(mut v @ _, mut arms @ _, mut env @ _) => {
crate::cancel_checkpoint();
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco3::EvalExprSlot(arm.body.clone(), crate::aver_generated::domain::eval::core::mergeBindingsSlot(&bindings, &arm.bindingSlots, &env)) }, Err(_) => { __MutualTco3::EvalMatchSlot(v, rest, env) } })
aver_list_match!(arms, [] => { return Err(AverStr::from("no matching arm")) }, [arm, rest] => match crate::aver_generated::domain::match_mod::matchPattern(&arm.pattern, &v) { Ok(bindings @ _) => { __MutualTco3::EvalExprSlot(arm.body, crate::aver_generated::domain::eval::core::mergeBindingsSlot(&bindings, &arm.bindingSlots, &env)) }, Err(_) => { __MutualTco3::EvalMatchSlot(v, rest, env) } })
}
};
}
Expand Down
80 changes: 80 additions & 0 deletions tests/fixtures/rust_update_after_field_move/main.av
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
module Main
intent = "A record update after a field read of the field it replaces, in plain bodies, loops and mutual tail calls."
effects [Args.get, Console.print]

record Flight
progress: Map<Int, Int>
name: String

fn grow(progress: Map<Int, Int>, key: Int) -> Map<Int, Int>
? "One more key."
Map.set(progress, key, key)

verify grow
grow({}, 1) => {1 => 1}

fn bumpLet(flight: Flight, key: Int) -> Flight
? "Reads the field in a let, then replaces it."
progress = flight.progress
Flight.update(flight, progress = grow(progress, key))

verify bumpLet
bumpLet(Flight(progress = {}, name = "a"), 1) => Flight(progress = {1 => 1}, name = "a")

fn bumpInline(flight: Flight, key: Int) -> Flight
? "Reads the field inside the update that replaces it."
Flight.update(flight, progress = grow(flight.progress, key))

verify bumpInline
bumpInline(Flight(progress = {}, name = "a"), 1) => Flight(progress = {1 => 1}, name = "a")

fn plain(flight: Flight, left: Int) -> Flight
? "Calls both bumps left times."
match left <= 0
true -> flight
false -> plain(bumpInline(bumpLet(flight, left), left + 1000), left - 1)

verify plain
plain(Flight(progress = {}, name = "a"), 0) => Flight(progress = {}, name = "a")

fn selfKeep(flight: Flight, left: Int) -> Flight
? "A loop whose let reads a field, then hands on the whole record."
progress = flight.progress
match left <= 0
true -> flight
false -> selfKeep(Flight.update(flight, name = "n{Map.len(progress)}"), left - 1)

verify selfKeep
selfKeep(Flight(progress = {}, name = "a"), 1) => Flight(progress = {}, name = "n0")

fn ping(flight: Flight, left: Int) -> Flight
? "One half of a mutual tail call: reads the field in a let, then replaces it."
progress = flight.progress
pong(Flight.update(flight, progress = grow(progress, left)), left - 1)

verify ping
ping(Flight(progress = {}, name = "a"), 0) => Flight(progress = {0 => 0}, name = "a")

fn pong(flight: Flight, left: Int) -> Flight
? "The other half: reads the field inside the update that replaces it."
match left <= 0
true -> flight
false -> ping(Flight.update(flight, progress = grow(flight.progress, left + 1000)), left - 1)

verify pong
pong(Flight(progress = {}, name = "a"), 0) => Flight(progress = {}, name = "a")

fn arg(index: Int, fallback: Int) -> Int
? "The index-th argument as an Int, or the fallback."
! [Args.get]
match Vector.get(Vector.fromList(Args.get()), index)
Option.Some(text) -> Result.withDefault(Int.fromString(text), fallback)
Option.None -> fallback

fn main() -> Unit
! [Args.get, Console.print]
size = arg(0, 1000)
a = plain(Flight(progress = {}, name = "plain"), size)
b = selfKeep(Flight(progress = {1 => 1}, name = "keep"), 3)
c = ping(Flight(progress = {}, name = "mutual"), size)
Console.print("{Map.len(a.progress)} {a.name} {Map.len(b.progress)} {b.name} {Map.len(c.progress)} {c.name}")
56 changes: 56 additions & 0 deletions tests/rust_work_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,62 @@ fn a_record_gives_up_its_fields_at_its_last_use() {
result.unwrap_or_else(|error| panic!("{error}"));
}

/// An update that replaces a field read earlier builds, and the read moves.
///
/// `progress = flight.progress` followed by
/// `Flight.update(flight, progress = grow(progress, key))` moved the field
/// out in the `let` of a mutual tail-call arm and then moved the whole
/// `flight` into the update, which rustc rejects (E0382). The arm now sees
/// the same field-move facts as any other body, so the update keeps the
/// other fields with `..flight`. The field read inside the update
/// (`grow(flight.progress, ...)`) and the one bound by a `let` both move into
/// `grow`, which then takes its Map by value and inserts in place. A loop
/// that reads a field and later hands on the whole record clones the field.
#[test]
fn an_update_after_a_field_read_of_the_replaced_field_builds_and_moves() {
let name = "rust_update_after_field_move";
let ws = temp_dir(name);
let project = ws.join("project");
fs::create_dir_all(&project).expect("create project dir");
let args = ["300"];
let result = (|| {
compile_rust(name, &project, name, &[])?;
let entry = fs::read_to_string(project.join("src/aver_generated/entry/mod.rs"))
.map_err(|error| format!("read the generated entry module: {error}"))?;
for moved in [
"pub fn grow(mut progress @ _: aver_rt::AverMap<",
"let progress @ _ = flight.progress;\n",
"__MutualTco1::Pong(Flight { progress: grow(progress, left.clone()), ..flight }",
"__MutualTco1::Ping(Flight { progress: grow(flight.progress, ",
"Flight { progress: grow(progress, key), ..flight }",
"Flight { progress: grow(flight.progress, key), ..flight }",
"let progress @ _ = flight.progress.clone();",
] {
if !entry.contains(moved) {
return Err(format!(
"{name}: missing `{moved}` in the generated entry module:\n{entry}"
));
}
}
if entry.contains("let mut __updated = flight; __updated.progress") {
return Err(format!(
"{name}: an update moves a record whose field was already moved out:\n{entry}"
));
}
let vm = run_vm_with(name, &args)?;
let bin = cargo_build(&project, name)?;
let rust = run_binary_with(&bin, &args)?;
if vm != rust {
return Err(format!(
"{name}: stdout mismatch\n--- VM ---\n{vm}\n--- Rust ---\n{rust}"
));
}
Ok(())
})();
let _ = fs::remove_dir_all(&ws);
result.unwrap_or_else(|error| panic!("{error}"));
}

/// Runs one backend against a loopback peer, on a port nobody else holds.
fn with_peer(run: impl FnOnce(&str) -> Result<String, String>) -> Result<String, String> {
let port = free_port();
Expand Down
Loading