From 6208d94076030387720bad0b71e79f6bf1fe4666 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Fri, 13 Nov 2026 09:58:10 +0100 Subject: [PATCH] =?UTF-8?q?fix(analyser):=20keep=20promised=20element=20ty?= =?UTF-8?q?pes=20true=20for=20pops,=20map=20literals=20and=20Any=20indexes?= =?UTF-8?q?=20=F0=9F=A9=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rQP9SKVmpvME4F2LXeWmG --- ndc_analyser/src/analyser/generics.rs | 3 +- ndc_analyser/src/analyser/mod.rs | 33 ++++++++--- ndc_analyser/src/analyser/solver.rs | 58 ++++++++++++++++--- ndc_stdlib/src/index.rs | 2 - ndc_stdlib/src/list.rs | 17 +++--- ndc_vm/src/value/mod.rs | 6 +- .../015_string_as_list_index_error.ndc | 2 +- ..._index_with_augmented_assignment_error.ndc | 2 +- ...ce_append_settles_each_list_in_a_tuple.ndc | 3 +- .../056_pop_on_an_empty_list_is_an_error.ndc | 8 +++ ...ite_with_an_any_index_checks_the_value.ndc | 5 ++ ...e_with_an_any_index_picks_by_the_index.ndc | 8 +++ .../033_a_key_without_a_value_holds_unit.ndc | 6 ++ ...rehension_default_is_one_of_the_values.ndc | 7 +++ 14 files changed, 130 insertions(+), 30 deletions(-) create mode 100644 tests/functional/programs/006_lists/056_pop_on_an_empty_list_is_an_error.ndc create mode 100644 tests/functional/programs/006_lists/057_a_write_with_an_any_index_checks_the_value.ndc create mode 100644 tests/functional/programs/006_lists/058_a_write_with_an_any_index_picks_by_the_index.ndc create mode 100644 tests/functional/programs/007_map_and_set/033_a_key_without_a_value_holds_unit.ndc create mode 100644 tests/functional/programs/007_map_and_set/034_a_comprehension_default_is_one_of_the_values.ndc diff --git a/ndc_analyser/src/analyser/generics.rs b/ndc_analyser/src/analyser/generics.rs index c431d91d..88d54c40 100644 --- a/ndc_analyser/src/analyser/generics.rs +++ b/ndc_analyser/src/analyser/generics.rs @@ -375,8 +375,7 @@ impl Analyser { }) .collect::>(); let binding = self - .overloads() - .resolve_call(call.candidates(), &arguments, call.kind()) + .resolve_overloads(call.candidates(), &arguments, call.kind()) .binding; self.record_pick(call.site(), binding, &arguments); } diff --git a/ndc_analyser/src/analyser/mod.rs b/ndc_analyser/src/analyser/mod.rs index 52fe87d5..57ada60b 100644 --- a/ndc_analyser/src/analyser/mod.rs +++ b/ndc_analyser/src/analyser/mod.rs @@ -564,14 +564,26 @@ impl Analyser { Expression::For { iterations, body } => { // A map comprehension's default is evaluated once, before the // loop starts. - if let ForBody::Map { - default: Some(default), - .. - } = body.as_ref() - { - self.analyse_expr(default); + let default = match body.as_ref() { + ForBody::Map { + default: Some(default), + .. + } => { + let typ = self.analyse_expr(default); + Some(self.constrain_default_value(typ, default.span)) + } + _ => None, + }; + match (self.analyse_for_iterations(iterations, body), default) { + // Reads can produce the default value, so it's one of the + // map's values: `%{:"none", x: x for x in xs}` holds + // Strings as well as what `xs` does. + (StaticType::Map { key, value }, Some(default)) => StaticType::Map { + key, + value: Box::new(self.join(&value, &default, *span)), + }, + (typ, _) => typ, } - self.analyse_for_iterations(iterations, body) } Expression::Call { function, @@ -626,6 +638,13 @@ impl Analyser { held.push((Some(value.id), value.span, self.analyse_expr(value))); } } + // A key without a value holds `()`, which in `%{"a", "b": 1}` + // is one of the map's values too. + if !held.is_empty() { + for (key, _) in values.iter().filter(|(_, value)| value.is_none()) { + held.push((None, key.span, StaticType::unit())); + } + } // Reads can produce the default value, so its type is part of // the map's value type. A default that's a function, as an diff --git a/ndc_analyser/src/analyser/solver.rs b/ndc_analyser/src/analyser/solver.rs index 0f1a2276..f279ed88 100644 --- a/ndc_analyser/src/analyser/solver.rs +++ b/ndc_analyser/src/analyser/solver.rs @@ -426,8 +426,7 @@ impl Analyser { .map(|argument| self.vars.resolve(argument)) .collect::>(); let binding = self - .overloads() - .resolve_call(&candidates, &arguments, kind) + .resolve_overloads(&candidates, &arguments, kind) .binding; assert!( binding == picked, @@ -1198,15 +1197,60 @@ impl Analyser { merged } + /// [`Overloads::resolve_call`], with the overloads it leaves the program + /// to pick from as it runs cut to the ones that take the arguments with + /// each type parameter one type. Picking looks at each argument on its + /// own, so `[]=(List, Int, T)` looks like it takes a String for a + /// `List` when the index is `Any`, but it can't. + pub(super) fn resolve_overloads( + &mut self, + candidates: &Candidates, + arguments: &[StaticType], + kind: CallKind, + ) -> ResolvedCall { + let mut resolved = self.overloads().resolve_call(candidates, arguments, kind); + // An operator may apply an overload to each element of a tuple + // instead, which takes what the elements are. + if kind == CallKind::Operator { + return resolved; + } + if let Binding::Dynamic(picked) = &resolved.binding { + let picked = picked.clone(); + // What a function takes and returns isn't checked as the program + // runs, so it's left out: a lambda that returns `Any` may still be + // what's expected. + let snapshot = self.vars.snapshot(); + let arguments = arguments + .iter() + .map(|argument| match self.vars.resolve(argument) { + StaticType::Function { .. } => self.vars.fresh(StaticType::Any), + argument => argument, + }) + .collect::>(); + let kept = picked + .into_iter() + .filter(|candidate| match candidate { + Candidate::Scalar(symbol) => self.could_accept(*symbol, &arguments), + Candidate::Vec(_) => true, + }) + .collect::>(); + self.vars.rollback(snapshot); + resolved.binding = if kept.is_empty() { + Binding::None + } else { + Binding::Dynamic(kept) + }; + } + resolved + } + /// Resolves `call` with the arguments as they are, reporting it when no /// overload accepts them. fn resolve_call_now(&mut self, call: &CallConstraint, arguments: &[StaticType]) -> StaticType { let ResolvedCall { binding, return_type, - } = self - .overloads() - .resolve_call(&call.candidates, arguments, call.kind); + } = self.resolve_overloads(&call.candidates, arguments, call.kind); if matches!(binding, Binding::None) { self.report_failed_call(call, arguments); self.record_pick(call.site, binding, arguments); @@ -1498,9 +1542,7 @@ impl Analyser { let ResolvedCall { binding, return_type, - } = self - .overloads() - .resolve_call(&call.candidates, arguments, call.kind); + } = self.resolve_overloads(&call.candidates, arguments, call.kind); let fits = !matches!(binding, Binding::None) && self .commit_call(&binding, arguments, &call.nodes.arguments, return_type) diff --git a/ndc_stdlib/src/index.rs b/ndc_stdlib/src/index.rs index 314ab282..fbf8f279 100644 --- a/ndc_stdlib/src/index.rs +++ b/ndc_stdlib/src/index.rs @@ -124,8 +124,6 @@ fn register_set(env: &mut FunctionRegistry>) { for signature in [ "(List, Int, T)", "(List, Iterator, Iterable)", - // An index whose type isn't known could be either. - "(List, Any, Any)", ] { env.declare_global_fn(Rc::new(NativeFunction { name: "[]=".to_string(), diff --git a/ndc_stdlib/src/list.rs b/ndc_stdlib/src/list.rs index 21b7c2aa..6d847b30 100644 --- a/ndc_stdlib/src/list.rs +++ b/ndc_stdlib/src/list.rs @@ -98,10 +98,12 @@ mod inner { } } - /// Removes and returns the last element from the list, or unit if empty. + /// Removes and returns the last element from the list. Use `pop?` to get + /// `None` from an empty list instead of an error. #[function(signature = "(List) -> T")] - pub fn pop(list: &mut Vec) -> Value { - list.pop().unwrap_or_else(Value::unit) + pub fn pop(list: &mut Vec) -> anyhow::Result { + list.pop() + .ok_or_else(|| anyhow::anyhow!("the list is empty")) } /// Removes and returns the first element from the list, or `None` if empty. @@ -113,13 +115,14 @@ mod inner { Value::Object(Rc::new(Object::Some(list.remove(0)))) } - /// Removes and returns the first element from the list, or unit if empty. + /// Removes and returns the first element from the list. Use `pop_left?` + /// to get `None` from an empty list instead of an error. #[function(signature = "(List) -> T")] - pub fn pop_left(list: &mut Vec) -> Value { + pub fn pop_left(list: &mut Vec) -> anyhow::Result { if list.is_empty() { - return Value::unit(); + return Err(anyhow::anyhow!("the list is empty")); } - list.remove(0) + Ok(list.remove(0)) } /// Resolves a possibly-negative index against `len`, wrapping negative diff --git a/ndc_vm/src/value/mod.rs b/ndc_vm/src/value/mod.rs index 884d1af8..af892356 100644 --- a/ndc_vm/src/value/mod.rs +++ b/ndc_vm/src/value/mod.rs @@ -355,6 +355,10 @@ impl Value { StaticType::MaxHeap(t) if t.is_unknown() => { matches!(self, Self::Object(o) if matches!(o.as_ref(), Object::MaxHeap(_))) } + // Unit, the empty tuple, is checked as cheaply as a scalar. + StaticType::Tuple(elements) if elements.is_empty() => { + matches!(self, Self::Object(o) if matches!(o.as_ref(), Object::Tuple(values) if values.is_empty())) + } // An option holds at most one value, which is cheap to check. StaticType::Option(t) => match self { Self::None => true, @@ -410,7 +414,7 @@ impl Value { (Some(expected), Some(takes)) => expected.len() == takes.len(), _ => true, }, - (StaticType::Iterator(element), StaticType::Iterator(_)) => element.is_unknown(), + (StaticType::Iterator(_), StaticType::Iterator(_)) => true, (target, found) => found == *target, } } diff --git a/tests/functional/programs/006_lists/015_string_as_list_index_error.ndc b/tests/functional/programs/006_lists/015_string_as_list_index_error.ndc index 6fe80975..ab413124 100644 --- a/tests/functional/programs/006_lists/015_string_as_list_index_error.ndc +++ b/tests/functional/programs/006_lists/015_string_as_list_index_error.ndc @@ -1,3 +1,3 @@ -// expect-error: Invalid list index +// expect-error: No function called '[]=' found that matches the arguments 'List, String let x = [1,2,3]; x["foo"] = 69; // ERROR diff --git a/tests/functional/programs/006_lists/016_string_as_list_index_with_augmented_assignment_error.ndc b/tests/functional/programs/006_lists/016_string_as_list_index_with_augmented_assignment_error.ndc index 152533dd..ba7ebc38 100644 --- a/tests/functional/programs/006_lists/016_string_as_list_index_with_augmented_assignment_error.ndc +++ b/tests/functional/programs/006_lists/016_string_as_list_index_with_augmented_assignment_error.ndc @@ -1,3 +1,3 @@ -// expect-error: Invalid list index +// expect-error: No function called '[]=' found that matches the arguments 'List, String let x = [1,2,3]; x["foo"] += 69; // ERROR diff --git a/tests/functional/programs/006_lists/052_an_in_place_append_settles_each_list_in_a_tuple.ndc b/tests/functional/programs/006_lists/052_an_in_place_append_settles_each_list_in_a_tuple.ndc index 11a7d640..6c852696 100644 --- a/tests/functional/programs/006_lists/052_an_in_place_append_settles_each_list_in_a_tuple.ndc +++ b/tests/functional/programs/006_lists/052_an_in_place_append_settles_each_list_in_a_tuple.ndc @@ -1,6 +1,7 @@ // `++=` on a tuple of lists appends to each one, so each empty list takes // the type of what it appends. -// expect-error: mismatched types: found String but expected Int +// expect-error: No function called '++' found that matches the arguments 'Tuple, List>, List' +// (`a[0] = "x"` decides first that `a` holds Strings.) let t = ([], []); t ++= [1, 2]; let a, b = t; diff --git a/tests/functional/programs/006_lists/056_pop_on_an_empty_list_is_an_error.ndc b/tests/functional/programs/006_lists/056_pop_on_an_empty_list_is_an_error.ndc new file mode 100644 index 00000000..d1219484 --- /dev/null +++ b/tests/functional/programs/006_lists/056_pop_on_an_empty_list_is_an_error.ndc @@ -0,0 +1,8 @@ +// `pop` returns an element, so an empty list has none to give; `pop?` gives +// `None` instead. +// expect-output: None +// expect-error: the list is empty +let xs = ["a".len()]; +xs.pop(); +print(xs.pop?()); +xs.pop(); diff --git a/tests/functional/programs/006_lists/057_a_write_with_an_any_index_checks_the_value.ndc b/tests/functional/programs/006_lists/057_a_write_with_an_any_index_checks_the_value.ndc new file mode 100644 index 00000000..b6c0ce31 --- /dev/null +++ b/tests/functional/programs/006_lists/057_a_write_with_an_any_index_checks_the_value.ndc @@ -0,0 +1,5 @@ +// Whatever the index turns out to be, a String doesn't go in a list of Ints. +// expect-error: mismatched types: found String but expected Int +let xs = ["a".len()]; +let i: Any = 0; +xs[i] = "bad"; diff --git a/tests/functional/programs/006_lists/058_a_write_with_an_any_index_picks_by_the_index.ndc b/tests/functional/programs/006_lists/058_a_write_with_an_any_index_picks_by_the_index.ndc new file mode 100644 index 00000000..5db14f8b --- /dev/null +++ b/tests/functional/programs/006_lists/058_a_write_with_an_any_index_picks_by_the_index.ndc @@ -0,0 +1,8 @@ +// expect-output: [5] [7,8,3] +let xs = ["a".len()]; +let i: Any = 0; +xs[i] = 5; +let ys = [1, 2, 3]; +let range: Any = 0..2; +ys[range] = [7, 8]; +print(xs, ys); diff --git a/tests/functional/programs/007_map_and_set/033_a_key_without_a_value_holds_unit.ndc b/tests/functional/programs/007_map_and_set/033_a_key_without_a_value_holds_unit.ndc new file mode 100644 index 00000000..3c2593bc --- /dev/null +++ b/tests/functional/programs/007_map_and_set/033_a_key_without_a_value_holds_unit.ndc @@ -0,0 +1,6 @@ +// expect-output: unit int +fn g(x: Int) => "int"; +fn g(x: ()) => "unit"; +fn g(x: Any) => "any"; +let m = %{"a", "b": 1}; +print(g(m["a"]), g(m["b"])); diff --git a/tests/functional/programs/007_map_and_set/034_a_comprehension_default_is_one_of_the_values.ndc b/tests/functional/programs/007_map_and_set/034_a_comprehension_default_is_one_of_the_values.ndc new file mode 100644 index 00000000..2accb112 --- /dev/null +++ b/tests/functional/programs/007_map_and_set/034_a_comprehension_default_is_one_of_the_values.ndc @@ -0,0 +1,7 @@ +// The default is a String, so a read may give one, not just the Ints. +// expect-output: str int +fn g(x: Int) => "int"; +fn g(x: String) => "str"; +fn g(x: Any) => "any"; +let m = %{:"missing", x: x for x in 1..3}; +print(g(m[99]), g(m[1]));