fix(vectorization): address PR review and tighten compile-time checks 🔍

OpAssignment with vec dispatch
  • Use candidate_return so `a += (3, 4)` on a `Tuple<Int, Int>` lvalue
    widens against the tuple return type instead of the underlying
    scalar's `Int`. Without this, annotated tuple lvalues errored and
    unannotated ones silently widened to Any.
  • Vec op= now emits SET_VAR (not Pop): dispatch_vec_call allocates a
    fresh result tuple, so the lvalue must be re-stored. The inner Rcs
    inside that tuple are still the same ones the element calls mutated,
    so aliasing semantics match scalar op= for List/HashMap stdlib ops.
    New test 011_vector_op_assign_aliasing.ndc pins the behaviour.
  • Only emit function_not_found when BOTH `op` and `op=` are
    Binding::None — previously a missing bare `-` would error even when
    `-=` handled the call (e.g. `Map -= Map` via difference_assign).

Compile-time function_not_found for incompat call sites
  • resolve_function_binding now filters the all_by_name fallback by
    parameter compatibility. Concrete-Function slots whose params don't
    accept the call's arg types get dropped; Any-typed and variadic
    slots stay (we can't tell statically). When the filtered list is
    empty, Binding::None is returned and the analyser emits
    function_not_found at compile time instead of letting the call
    fail at runtime with a less-specific error. Regression test
    bug0022_incompat_dynamic_misinferred.ndc covers the case where the
    old behaviour silently inferred a misleading concrete return type.

Empty-map value-type inference
  • `%{}` now infers `Map<Any, Any>` (was `Map<Any, ()>` which was too
    narrow for almost any operation).
  • `%{:0}` now folds the default's type into value_type, so the map
    infers `Map<Any, Int>` instead of `Map<Any, ()>` — needed so that
    `map[k] += 1` on a defaulted map type-checks.

Runtime / VM
  • `Callable::Vec` carries axis_len so dispatch_vec_call doesn't
    re-walk the args to find it.
  • dispatch_vec_call uses split_off instead of stack-slice + truncate.

Tidy
  • analyser.rs has a single `impl Analyser` block again; free helpers
    moved below.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Author: timfennis

ndc_analyser/src/analyser.rs

@@ -214,22 +214,26 @@ impl Analyser {
                     .scope_tree
                     .resolve_function_binding(operation, &arg_types, true);
 
-                if let Binding::None = resolved_operation {
+                // Either operator can handle the call: `op=` modifies in
+                // place, `op` falls back via `a = a op b`. Only error when
+                // both are missing — e.g. `Map -= Map` is fine via `-=`
+                // even though bare `-` has no overload for maps.
+                if matches!(resolved_assign_operation, Binding::None)
+                    && matches!(resolved_operation, Binding::None)
+                {
                     self.emit(AnalysisError::function_not_found(
                         operation, &arg_types, *span,
                     ));
                 }
 
-                // Determine the result type of the operation
+                // Determine the result type of the operation. Routed through
+                // candidate_return so that a Resolved vec candidate widens the
+                // lvalue with `Tuple<elem_return; max_len>` instead of the
+                // underlying scalar's return — otherwise `a += (3, 4)` on a
+                // `Tuple<Int, Int>` lvalue would try to widen with `Int`.
                 let result_type = match resolved_operation {
                     Binding::Resolved(res) => {
-                        if let StaticType::Function { return_type, .. } =
-                            self.scope_tree.get_type(res.var)
-                        {
-                            Some(return_type.as_ref().clone())
-                        } else {
-                            None
-                        }
+                        Some(candidate_return(&self.scope_tree, res, &arg_types))
                     }
                     _ => None,
                 };
@@ -460,12 +464,16 @@ impl Analyser {
                 }
 
                 if let Some(default) = default {
-                    self.analyse_or_any(default);
+                    // The default value's type is what `map[missing]` returns,
+                    // so it contributes to the value type just like a regular
+                    // entry would.
+                    let default_type = self.analyse_or_any(default);
+                    Self::fold_lub(&mut value_type, default_type);
                 }
 
                 Ok(StaticType::Map {
                     key: Box::new(key_type.unwrap_or(StaticType::Any)),
-                    value: Box::new(value_type.unwrap_or_else(StaticType::unit)),
+                    value: Box::new(value_type.unwrap_or(StaticType::Any)),
                 })
             }
             Expression::Return { value } => {
@@ -540,6 +548,9 @@ impl Analyser {
                 // `LUB(Tuple<...>, scalar) = Any` in the type lattice, so the
                 // mixed case naturally falls to Any; pure-scalar Dynamic
                 // recovers the precision PR #140 had to pessimise.
+                // `resolve_function_binding` filters the all_by_name fallback
+                // by signature compatibility, so by the time we get here
+                // every candidate could plausibly accept the call's args.
                 let return_type = candidates
                     .iter()
                     .map(|c| candidate_return(&self.scope_tree, c, argument_types))
@@ -556,52 +567,7 @@ impl Analyser {
 
         out_type
     }
-}
-
-/// Returns the type a single candidate would produce for this call.
-///
-/// Examples (assume `+(Int, Int) -> Int` is the underlying scalar):
-/// - scalar candidate, call args `[Int, Int]` → `Int`
-/// - vec candidate, call args `[Tuple<Int, Int>, Tuple<Int, Int>]` →
-///   `Tuple<Int, Int>` (two element calls, each returns `Int`)
-/// - vec candidate, call args `[Tuple<Int, Int>, Int]` →
-///   `Tuple<Int, Int>` (the scalar `Int` broadcasts)
-/// - vec candidate, call args `[Any, Any]` → `Any` (no tuple length
-///   visible at compile time, so we can't say how long the result is)
-fn candidate_return(
-    scope_tree: &ScopeTree,
-    candidate: &Candidate,
-    argument_types: &[StaticType],
-) -> StaticType {
-    let StaticType::Function { return_type, .. } = scope_tree.get_type(candidate.var) else {
-        return StaticType::Any;
-    };
-    let scalar_return = return_type.as_ref().clone();
-    if !candidate.vectorized {
-        return scalar_return;
-    }
-    match static_vec_axis_len(argument_types) {
-        Some(len) => StaticType::Tuple(vec![scalar_return; len]),
-        None => StaticType::Any,
-    }
-}
-
-/// Length of the first tuple-shaped argument, which is how many element
-/// calls a vec dispatch will make.
-///
-/// Examples:
-/// - `[Tuple<Int, Int>, Int]` → `Some(2)`
-/// - `[Int, Tuple<Int, Int, Int>]` → `Some(3)`
-/// - `[Int, Int]` → `None` (vec doesn't apply)
-/// - `[Any, Any]` → `None` (we can't tell at compile time)
-fn static_vec_axis_len(sig: &[StaticType]) -> Option<usize> {
-    sig.iter().find_map(|t| match t {
-        StaticType::Tuple(elems) if !elems.is_empty() => Some(elems.len()),
-        _ => None,
-    })
-}
 
-impl Analyser {
     fn resolve_for_iterations(
         &mut self,
         iterations: &mut [ForIteration],
@@ -925,6 +891,49 @@ impl Analyser {
     }
 }
 
+/// Returns the type a single candidate would produce for this call.
+///
+/// Examples (assume `+(Int, Int) -> Int` is the underlying scalar):
+/// - scalar candidate, call args `[Int, Int]` → `Int`
+/// - vec candidate, call args `[Tuple<Int, Int>, Tuple<Int, Int>]` →
+///   `Tuple<Int, Int>` (two element calls, each returns `Int`)
+/// - vec candidate, call args `[Tuple<Int, Int>, Int]` →
+///   `Tuple<Int, Int>` (the scalar `Int` broadcasts)
+/// - vec candidate, call args `[Any, Any]` → `Any` (no tuple length
+///   visible at compile time, so we can't say how long the result is)
+fn candidate_return(
+    scope_tree: &ScopeTree,
+    candidate: &Candidate,
+    argument_types: &[StaticType],
+) -> StaticType {
+    let StaticType::Function { return_type, .. } = scope_tree.get_type(candidate.var) else {
+        return StaticType::Any;
+    };
+    let scalar_return = return_type.as_ref().clone();
+    if !candidate.vectorized {
+        return scalar_return;
+    }
+    match static_vec_axis_len(argument_types) {
+        Some(len) => StaticType::Tuple(vec![scalar_return; len]),
+        None => StaticType::Any,
+    }
+}
+
+/// Length of the first tuple-shaped argument, which is how many element
+/// calls a vec dispatch will make.
+///
+/// Examples:
+/// - `[Tuple<Int, Int>, Int]` → `Some(2)`
+/// - `[Int, Tuple<Int, Int, Int>]` → `Some(3)`
+/// - `[Int, Int]` → `None` (vec doesn't apply)
+/// - `[Any, Any]` → `None` (we can't tell at compile time)
+fn static_vec_axis_len(sig: &[StaticType]) -> Option<usize> {
+    sig.iter().find_map(|t| match t {
+        StaticType::Tuple(elems) if !elems.is_empty() => Some(elems.len()),
+        _ => None,
+    })
+}
+
 #[derive(thiserror::Error, Debug)]
 #[error("{text}")]
 pub struct AnalysisError {

ndc_analyser/src/scope.rs

@@ -563,8 +563,36 @@ impl ScopeTree {
             return Binding::Dynamic(combined);
         }
 
-        if !all_by_name.is_empty() {
-            return Binding::Dynamic(all_by_name.into_iter().map(Candidate::scalar).collect());
+        // Filter the all_by_name fallback by signature compatibility:
+        // a concrete `Function<...>` slot whose parameters are
+        // incompatible with the call's args is guaranteed to fail
+        // dispatch at runtime, so leaving it in would let the analyser
+        // synthesise a misleading concrete return for a doomed call.
+        // `Any`-typed and variadic (`parameters: None`) slots stay — we
+        // can't statically tell what those would accept.
+        let filtered: Vec<ResolvedVar> = all_by_name
+            .into_iter()
+            .filter(|var| match self.get_type(*var) {
+                StaticType::Function {
+                    parameters: Some(params),
+                    ..
+                } => {
+                    params.len() == sig.len()
+                        && params
+                            .iter()
+                            .zip(sig)
+                            .all(|(p, a)| !p.is_incompatible_with(a))
+                }
+                StaticType::Function {
+                    parameters: None, ..
+                }
+                | StaticType::Any => true,
+                _ => false,
+            })
+            .collect();
+
+        if !filtered.is_empty() {
+            return Binding::Dynamic(filtered.into_iter().map(Candidate::scalar).collect());
         }
         Binding::None
     }

ndc_vm/src/compiler.rs

@@ -203,17 +203,33 @@ impl Compiler {
                         ..
                     } => {
                         let var = resolved.expect("lvalue must be resolved");
-                        if matches!(resolved_assign_operation, Binding::Resolved(_)) {
-                            // In-place operation (e.g. |=, &=) resolved exactly: modifies
-                            // the value's Rc in place via sync_map_mutations in the bridge,
-                            // so all aliases sharing the Rc see the change. We discard the
-                            // unit return value; the variable slot already holds the
+                        if matches!(resolved_assign_operation, Binding::Resolved(c) if !c.vectorized)
+                        {
+                            // Scalar in-place op= resolved exactly: modifies the value's
+                            // Rc in place via sync_map_mutations in the bridge, so all
+                            // aliases sharing the Rc see the change. We discard the
+                            // return value; the variable slot already holds the
                             // (now-updated) shared reference.
                             self.compile_binding(resolved_assign_operation, span)?;
                             self.emit_get_var(var, lv_span);
                             self.compile_expr(*r_value)?;
                             self.chunk.write(OpCode::Call(2), span);
                             self.chunk.write(OpCode::Pop, span);
+                        } else if matches!(resolved_assign_operation, Binding::Resolved(c) if c.vectorized)
+                        {
+                            // Vec-resolved op=: dispatch_vec_call allocates a fresh
+                            // result tuple whose elements are the per-element scalar
+                            // returns. Store it back so we don't silently lose the
+                            // update if a future scalar op= doesn't mutate through Rc.
+                            // For the current stdlib (List/String ++=, HashMap -=)
+                            // this is functionally equivalent to Pop because the
+                            // element calls already mutated their inputs through Rc;
+                            // the stored tuple just holds those same (now-mutated) Rcs.
+                            self.compile_binding(resolved_assign_operation, span)?;
+                            self.emit_get_var(var, lv_span);
+                            self.compile_expr(*r_value)?;
+                            self.chunk.write(OpCode::Call(2), span);
+                            self.emit_set_var(var, lv_span);
                         } else if let Binding::Dynamic(assign_candidates) =
                             resolved_assign_operation
                         {

ndc_vm/src/vm.rs

@@ -261,8 +261,8 @@ impl Vm {
                     {
                         let result = match callable {
                             Callable::Scalar(func) => self.dispatch_call(func, args),
-                            Callable::Vec(scalar_fn) => {
-                                self.dispatch_vec_call(&scalar_fn, args, span)
+                            Callable::Vec(scalar_fn, axis_len) => {
+                                self.dispatch_vec_call(&scalar_fn, args, axis_len, span)
                             }
                         };
                         if let Err(mut e) = result {
@@ -805,7 +805,8 @@ impl Vm {
                 return None;
             };
             if candidate.vectorized {
-                vec_candidate_applies(f, args).then(|| Callable::Vec(f.clone()))
+                let axis_len = vec_axis_len(args)?;
+                vec_candidate_applies(f, args, axis_len).then(|| Callable::Vec(f.clone(), axis_len))
             } else {
                 f.matches_value_args(args)
                     .then(|| Callable::Scalar(f.clone()))
@@ -822,19 +823,17 @@ impl Vm {
         &mut self,
         scalar_fn: &Function,
         args: usize,
+        axis_len: usize,
         span: Span,
     ) -> Result<(), VmError> {
-        let callee_idx = self.stack.len() - args - 1;
         let arg_start = self.stack.len() - args;
-
-        let axis_len = vec_axis_len(&self.stack[arg_start..])
-            .expect("dispatch_vec_call requires at least one non-empty tuple arg");
         let callee_name = self.callee_name(args);
 
         // Materialise arg values up front so the stack borrow doesn't conflict
         // with call_callback's mutable access. Tuples are cheap to clone (Rc
         // for the values; the Vec is the only fresh allocation).
-        let arg_values: Vec<Value> = self.stack[arg_start..].to_vec();
+        let arg_values: Vec<Value> = self.stack.split_off(arg_start);
+        self.stack.pop(); // clean up the callee
 
         let mut results = Vec::with_capacity(axis_len);
         for i in 0..axis_len {
@@ -857,7 +856,6 @@ impl Vm {
             results.push(result);
         }
 
-        self.stack.truncate(callee_idx);
         self.stack
             .push(Value::Object(Rc::new(Object::Tuple(results))));
         Ok(())
@@ -969,19 +967,15 @@ impl Vm {
 /// should run once per element of the broadcast axis.
 pub(crate) enum Callable {
     Scalar(Function),
-    Vec(Function),
+    Vec(Function, usize),
 }
 
 /// Determines whether a vec candidate over `scalar_fn` applies to the given
 /// runtime argument values. Returns `true` only when at least one arg is a
 /// non-empty `Object::Tuple`, all tuple-shaped args share a length, and the
 /// scalar's parameter types accept every per-position element pair (under
 /// broadcast of non-tuple args).
-fn vec_candidate_applies(scalar_fn: &Function, args: &[Value]) -> bool {
-    let Some(axis_len) = vec_axis_len(args) else {
-        return false;
-    };
-
+fn vec_candidate_applies(scalar_fn: &Function, args: &[Value], axis_len: usize) -> bool {
     let mut pair: Vec<Value> = Vec::with_capacity(args.len());
     for i in 0..axis_len {
         pair.clear();

tests/functional/programs/013_vector_math/002_vector_error.ndc

@@ -1,2 +1,2 @@
-// expect-error: no function called '+' found matches the arguments
+// expect-error: No function called '+' found that matches the arguments
 (1,2) + (5,3,2)

tests/functional/programs/013_vector_math/010_vector_op_assignment.ndc

@@ -0,0 +1,23 @@
+// Compound assignment must use the vec return type, not the underlying
+// scalar's return. `a += (3, 4)` on a `Tuple<Int, Int>` lvalue must
+// widen-check against `Tuple<Int, Int>`, not `Int`.
+let a: Tuple<Int, Int> = (1, 2);
+a += (3, 4);
+assert_eq(a, (4, 6));
+
+let b: Tuple<Int, Int> = (10, 20);
+b -= (1, 2);
+assert_eq(b, (9, 18));
+
+// Scalar broadcast on the right-hand side.
+let c: Tuple<Int, Int> = (3, 4);
+c *= 2;
+assert_eq(c, (6, 8));
+
+// Without an annotation the inferred lvalue type must also widen
+// correctly. `d` starts as `Tuple<Int, Int>` and a follow-up read
+// must still see the precise tuple element types.
+let d = (1, 2);
+d += (10, 20);
+let e: Tuple<Int, Int> = d + (100, 200);
+assert_eq(e, (111, 222));

tests/functional/programs/013_vector_math/011_vector_op_assign_aliasing.ndc

@@ -0,0 +1,16 @@
+// Vec-resolved op= must preserve the in-place aliasing contract that
+// scalar op= guarantees: inner Rc mutations stay visible through every
+// alias of the originals, and the variable itself still reads as the
+// updated value.
+let l1 = [1, 2];
+let l2 = [3, 4];
+let pair = (l1, l2);
+let outer_alias = pair;
+pair ++= ([5], [6]);
+// Inner-list mutation visible through the original bindings.
+assert_eq(l1, [1, 2, 5]);
+assert_eq(l2, [3, 4, 6]);
+// Outer alias still sees the same inner lists, now mutated.
+assert_eq(outer_alias, ([1, 2, 5], [3, 4, 6]));
+// pair itself reads as the updated value.
+assert_eq(pair, ([1, 2, 5], [3, 4, 6]));

tests/functional/programs/900_bugs/bug0022_incompat_dynamic_misinferred.ndc

@@ -0,0 +1,11 @@
+// expect-error: No function called 'returns_int' found that matches the arguments 'Int'
+// `returns_int(42)` has no compatible overload — the only `returns_int`
+// declared takes a `String`. Before the fix, `resolve_function_binding`
+// fell back to all-by-name and emitted `Binding::Dynamic([returns_int])`
+// despite the type mismatch; the analyser then LUB'd its declared return
+// (`Int`) and reported "found Int but expected …" at downstream
+// assignments, masking the real bug. Now the fallback filters concrete-
+// Function slots whose params don't match the call, leaving the binding
+// empty so the call surfaces at compile time as "no function found".
+fn returns_int(s: String) -> Int { 1 };
+let x: String = returns_int(42);