You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Today, in-place op= overloads in the stdlib (List ++=, String ++=, HashMap -=, etc.) return lhs after mutating it through Rc. The compiler's in-place branch (ndc_vm/src/compiler.rs ~206) then Pops that return value. The Dynamic merged-overload branch (~217-239) leans on this return-shape: it uses a single emit_set_var after the call, which works because:
if dispatch picks an op= candidate → it returns lhs; SetVar re-stores the same Rc (harmless rewrite).
if dispatch falls through to an op candidate → it returns a fresh value; SetVar stores it (necessary).
The compiler comment at line 209-211 already claims the return is unit ("We discard the unit return value") — the comment is stale, not the code.
Proposal
Move the reality to match the intent. op= should be a statement-like contract: mutates, returns nothing observable. This aligns with Python (+= returns None), Rust (AddAssign::add_assign(&mut self, rhs) returns ()), and most other languages with compound assignment.
What changes
All stdlib op= registrations return () instead of lhs.
ndc_stdlib/src/list.rs (++=)
ndc_stdlib/src/string.rs (++=)
ndc_stdlib/src/hash_map.rs (-=, += if/when it exists, etc.)
Any others added since.
Compiler Dynamic merged-overload branch must distinguish op= from op dispatch at runtime. Today it uses a single emit_set_var; that's invalid once op= returns (), because we'd store unit into the variable on op= dispatch. Options:
Add a discriminator on Function (e.g. is_assign_op: bool, or detect by name ending in =) and a new opcode OpCode::CallAssign(usize) that the dispatcher uses to choose Pop vs SetVar based on which kind got picked.
Or drop the merged fallback semantics entirely: += on Any would require a real op= overload (no quiet promotion to op + set_var). User-visible language change; needs a call.
dispatch_vec_call opt-out of result accumulation when scalar return is (). Currently a vec op= would build Tuple([(), (), ...]) and the compiler Pops it. Wasteful. Check the scalar fn's return type once; if unit, skip the results.push(...) and push Value::unit() at the end instead of an Object::Tuple.
Compiler in-place branch (compiler.rs:206) stays as Pop for both scalar and vec. Once op= returns unit unconditionally, the targeted compiler fix from PR feat(vectorization): broaden tuple operators and recover precise types 📐 #141 (which uses emit_set_var for vec-resolved op=) becomes unnecessary defence; both paths just discard.
PR #141 is a vectorization broadening; conflating an op= return-type semantic change with it would balloon the diff and tangle two unrelated regressions if either piece needs to be reverted. The targeted compiler fix from PR #141 (#141 commit on ndc_vm/src/compiler.rs ~206) makes the vec path defensive against future non-Rc-mutating op= overloads without requiring this redesign first.
Acceptance
All stdlib op= registrations return unit.
(a ++= b) evaluates to () as an expression.
Aliasing semantics for List/String/HashMap op= preserved (existing tests pass).
Vec op= produces no wasted tuple allocation.
Dynamic merged-overload += on Any-typed lvalues still picks the right kind of dispatch.
Context
Today, in-place
op=overloads in the stdlib (List ++=,String ++=,HashMap -=, etc.) returnlhsafter mutating it throughRc. The compiler's in-place branch (ndc_vm/src/compiler.rs~206) thenPops that return value. The Dynamic merged-overload branch (~217-239) leans on this return-shape: it uses a singleemit_set_varafter the call, which works because:op=candidate → it returnslhs;SetVarre-stores the same Rc (harmless rewrite).opcandidate → it returns a fresh value;SetVarstores it (necessary).The compiler comment at line 209-211 already claims the return is unit ("We discard the unit return value") — the comment is stale, not the code.
Proposal
Move the reality to match the intent.
op=should be a statement-like contract: mutates, returns nothing observable. This aligns with Python (+=returnsNone), Rust (AddAssign::add_assign(&mut self, rhs)returns()), and most other languages with compound assignment.What changes
All stdlib
op=registrations return()instead of lhs.ndc_stdlib/src/list.rs(++=)ndc_stdlib/src/string.rs(++=)ndc_stdlib/src/hash_map.rs(-=,+=if/when it exists, etc.)Compiler Dynamic merged-overload branch must distinguish
op=fromopdispatch at runtime. Today it uses a singleemit_set_var; that's invalid onceop=returns(), because we'd store unit into the variable onop=dispatch. Options:Function(e.g.is_assign_op: bool, or detect by name ending in=) and a new opcodeOpCode::CallAssign(usize)that the dispatcher uses to choosePopvsSetVarbased on which kind got picked.+=onAnywould require a realop=overload (no quiet promotion toop + set_var). User-visible language change; needs a call.dispatch_vec_callopt-out of result accumulation when scalar return is(). Currently a vecop=would buildTuple([(), (), ...])and the compiler Pops it. Wasteful. Check the scalar fn's return type once; if unit, skip theresults.push(...)and pushValue::unit()at the end instead of anObject::Tuple.Compiler in-place branch (
compiler.rs:206) stays asPopfor both scalar and vec. Onceop=returns unit unconditionally, the targeted compiler fix from PR feat(vectorization): broaden tuple operators and recover precise types 📐 #141 (which usesemit_set_varfor vec-resolvedop=) becomes unnecessary defence; both paths just discard.Tests to keep / add
tests/functional/programs/013_vector_math/011_vector_op_assign_aliasing.ndc(already in PR feat(vectorization): broaden tuple operators and recover precise types 📐 #141) — verifies inner-Rc mutation still propagates after the change.(a ++= b)is()(currently it'slhs, post-change it's unit).+=case keeps working for both numeric and container types.Why not in PR #141
PR #141 is a vectorization broadening; conflating an
op=return-type semantic change with it would balloon the diff and tangle two unrelated regressions if either piece needs to be reverted. The targeted compiler fix from PR #141 (#141 commit onndc_vm/src/compiler.rs~206) makes the vec path defensive against future non-Rc-mutatingop=overloads without requiring this redesign first.Acceptance
op=registrations return unit.(a ++= b)evaluates to()as an expression.op=preserved (existing tests pass).op=produces no wasted tuple allocation.+=on Any-typed lvalues still picks the right kind of dispatch.