Skip to content

conveyor-engine: set_value on a pass-driven conveyor/queue flow is silently accepted but has no effect #871

Description

@bpowers

Found by branch review of conveyor-engine before merge (the in-flight XMILE conveyor+queue implementation; specs docs/design/conveyors.md + docs/design/queues.md). Filed from a review report that exceeded its 15-finding cap.

Summary

Expansion rewrites every conveyor/queue driven flow (conveyor outflow/leak, queue outflow) to a literal 0 equation. That placeholder compiles to AssignConstCurr in the Flows phase, so the VM's collect_constant_info classifies the slot as an overridable constant. simlin_sim_set_value therefore silently accepts an override on a belt-driven flow (and get_value echoes it back), instead of rejecting it with BadOverride -- yet the conveyor/queue pass overwrites that slot every step, so the override never affects the simulation.

Failure scenario

simlin_sim_set_value(sim, "graduating", 999.0) on test/conveyors/minimal_conveyor.xmile returns Ok (the placeholder 0 compiles to AssignConstCurr, making is_constant(off) == true), and get_value reports 999 before running (via set_value_now). But every simulated step's saved graduating is the belt-driven value (~250), because run_coupled_passes/phase_a does curr[plan.primary_out_off] = r.out_vol / dt after the Flows phase applied the override literal. The override is silently ignored, whereas a genuinely computed flow (AssignCurr) would be rejected with BadOverride.

Evidence

  • placeholder rewrite: f.equation = placeholder_zero_equation(&f.equation); (src/simlin-engine/src/conveyor_compile.rs:907) and Equation::Scalar(_) => Equation::Scalar("0".to_string()) (:1481)
  • const codegen: if let Expr::Const(value, _) = rhs.as_ref() { ... self.push(Opcode::AssignConstCurr { off, literal_id }) } (src/simlin-engine/src/codegen.rs:1226-1231)
  • classification (no conveyor exclusion): for op in module.compiled_flows.code.iter() { if let Opcode::AssignConstCurr { off, literal_id } = op { (src/simlin-engine/src/vm.rs:530-531)
  • accept path: if !self.is_constant(off) { return sim_err!(BadOverride, ...); } self.apply_override(off, value); Ok(off) (src/simlin-engine/src/vm.rs:1263-1272)
  • per-step overwrite after Flows: curr[plan.primary_out_off] = r.out_vol / dt; (conveyor_compile.rs:1999; leaks at :2001), with Flows (vm.rs:874) running before the pass (vm.rs:885)
  • FFI surfaces the silent success: match vm.set_value(&canon_name, val) { Ok(off) => { state.overrides.insert(off, val); } (src/libsimlin/src/simulation.rs:607)

Suggested direction

Record the pass-driven flow slots at expansion time and reject set_value on them with BadOverride, so the FFI surfaces an error instead of a no-op.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingengineIssues with the rust-based simulation engine

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions