verify: warn on bare +=/mset/mdel result-discard (ILO-T033)#312
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
85e80db to
f000170
Compare
…O-T033)
`+=xs v`, `mset m k v`, and `mdel m k` look like in-place mutation but
ilo's functional semantics return a new value. As a bare statement
(`@i 0..3{+=out i}`, `mset m "a" 1;m`) the result is silently discarded
and the source binding is unchanged.
Same pattern as ILO-T032 for bare fmt/fmt2. Mirrors the verifier check
in verify_body: warn when Stmt::Expr is BinOp::Append or a Builtin::Mset
/ Builtin::Mdel call, at any non-tail statement or anywhere inside a
loop body. The hint extracts the source binding name from the first
operand so the suggested rebind shape matches the user's local
variable rather than a generic placeholder.
Registry entry covers the three rebind shapes with worked before/after
examples. Diagnostic-only change: runtime semantics across tree, VM,
and Cranelift unchanged. Verifier still emits a warning (not an error)
so the program still runs.
10 unit tests pin every contract: bare-in-loop warns (+=, mset, mdel),
rebind shapes stay silent, tail-position-in-function stays silent,
guard-body-in-loop warns, nested-in-call stays silent, multiple bare
calls each warn independently.
Closes db-analyst rerun4 finding logged across three consecutive
persona sessions (assessment doc lines 2351, 2628, 3865).
Demonstrates the three canonical fix shapes (`out=+=out i`, `m=mset m k v`, `m=mdel m k`) plus a tail-position case where bare `+=xs v` is legitimate as the function's return value. The engine harness in tests/examples_engines.rs runs every function across each engine and asserts the output matches, so the file doubles as a cross-engine regression test for the rebind semantics.
Adds a callout next to the existing fmt-does-not-print note in SPEC.md, ai.txt, and skills/ilo/SKILL.md. Same shape as the ILO-T032 entry: short paragraph stating the functional semantics, the three discarded positions the verifier flags, and the rebind form the agent should reach for. Cross-references ILO-T033 so `ilo --explain ILO-T033` surfaces the full worked example from the registry.
f000170 to
14fabc2
Compare
#313 (inverse-trig: OP_ASIN=171, OP_ACOS=172, OP_ATAN=173) and #314 (flat-cross-engine: OP_FLAT=171) both landed without rebasing one onto the other, so the merged main has two opcodes sharing value 171. Clippy's exhaustive-pattern checks catch it as "unreachable pattern" because the inverse-trig match arm matches all relevant values before OP_FLAT's arm gets a chance. Renumbering OP_FLAT to 174 (the next free slot after OP_ATAN=173) restores a unique mapping. No semantic change beyond the byte value, and bytecode is in-process so there's no compatibility concern.
danieljohnmorris
added a commit
that referenced
this pull request
May 16, 2026
twelve fixes since 0.11.3, surfaced by rerun4 personas plus standing asks: srt-Cranelift TLS desync (#306), CLI auto-run restoration (#307), OP_LISTAPPEND O(n^2) JIT memory regression (#308), precedence-pair hint false-positive on parens (#309), prefix ?? accepts call expression (#310), += pure-shape docs (#311), bare-mutation silent no-op verifier warning ILO-T033 (#312), asin/acos/atan inverse trig builtins (#313), flat cross-engine (#314), cond{~v} discard hint multi-stmt false-positive (#315), rsrt fn xs key-fn overloads (#316), xs.(expr) paren-after-dot diagnostic hint (#317).
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
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.
Summary
+=xs v,mset m k v, andmdel m klook like in-place mutation but return a new value under ilo's functional semantics. As a bare statement, the result is silently discarded on every engine and the source binding is unchanged. The verifier now flags this with ILO-T033 and a hint pointing at the rebind shape that fixes it.This is a diagnostic-only change. Runtime behaviour across tree, VM, and Cranelift is unchanged. Same shape as the ILO-T032 fix for bare
fmt/fmt2shipped in #299.Persona context
db-analyst surfaced the silent no-op three sessions in a row (assessment doc lines 2351, 2628, 3865). The semantics are correct ilo, but the surface area, no diagnostic at all, was the worst-of-both-worlds case I'd noted as a follow-up at the bottom of the T032 PR. This closes that loop.
Repro before/after
Before: runs silently to completion, returns
[]. The+=out iinside the loop body builds a new list each iteration but the result is thrown away, sooutstays empty. Agent spends 5-30 minutes diagnosing why their accumulator is zero-length.After: verifier emits
ILO-T033 bare '+=' result is discardedwith hint+=xs v returns a new list, rebind with out=+=out v to mutate, or use the result. Program still runs (warning, not error), but the trap is visible at verify time.Same shape for
mset:returns
{}before, warns + returns{}after; fix ism=mset m "a" 1.What's in the diff (per commit)
verify: warn on bare +=/mset/mdel at non-tail or in-loop position (ILO-T033)- the verifier check, the registry entry with worked examples, and 10 unit tests covering bare-in-loop warns for all three shapes, rebind no-warn for all three, tail-position-in-function no-warn, guard-body-in-loop warns, nested-in-call no-warn, multiple bare calls each warn independently. Hint extracts the source binding name from the first operand so the suggested rebind matches the user's local variable.example: bare-mut-warns.ilo pins the rebind shapes for ILO-T033- newexamples/bare-mut-warns.ilodemonstrating all three canonical fix shapes plus a tail-position case where bare+=xs vis legitimate. Exercised across tree + VM viatests/examples_engines.rs.docs: note that +=/mset/mdel return new values, not in-place mutation- SPEC.md, ai.txt, and skills/ilo/SKILL.md gain a short callout next to the existing fmt-does-not-print note, cross-referencing ILO-T033.Test plan
cargo test --release --features cranelift --lib verify::tests- 415/415 passcargo test --release --features cranelift --test examples_engines- examples_engines passes (new example included)cargo test --release --features cranelift --lib vm::compile_cranelift::tests::aot- all 7 AOT tests pass against default target dircargo test --release --features cranelift(full suite) - 3074 tests pass in isolated target dir; the 7 AOT failures there are the pre-existing target-dir-relative behaviour also seen in verify: warn on bare fmt/fmt2 at non-tail position (ILO-T032) #299 and pass against the default targetcargo fmt --check- cleancargo clippy --release --features cranelift --all-targets -- -D warnings- clean+=,mset,mdel) plus their rebind forms on all three engines, confirmed warning surfaces at CLI and program still runs (exit 0)Follow-ups
fmt/fmt2(T032's deferred follow-up). Same shape: T032 only checks non-tail position, so@i 0..3{fmt "x={}" i}doesn't warn yet. Widening the T032 check to mirror this PR's in-loop logic is the natural next step.ilo --explain ILO-T033worked examples on the warning line itself, that's an orthogonal CLI change.