fix(analyser): keep promised types true for pops, maps and Any indexes 🩹

ndc_analyser/src/analyser/generics.rs

@@ -375,8 +375,7 @@ impl Analyser {
             })
             .collect::<Vec<_>>();
         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);
     }

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

ndc_analyser/src/analyser/solver.rs

@@ -426,8 +426,7 @@ impl Analyser {
                         .map(|argument| self.vars.resolve(argument))
                         .collect::<Vec<_>>();
                     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<T>, Int, T)` looks like it takes a String for a
+    /// `List<Int>` 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::<Vec<_>>();
+            let kept = picked
+                .into_iter()
+                .filter(|candidate| match candidate {
+                    Candidate::Scalar(symbol) => self.could_accept(*symbol, &arguments),
+                    Candidate::Vec(_) => true,
+                })
+                .collect::<Vec<_>>();
+            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)

ndc_stdlib/src/index.rs

@@ -124,8 +124,6 @@ fn register_set(env: &mut FunctionRegistry<Rc<NativeFunction>>) {
     for signature in [
         "<T>(List<T>, Int, T)",
         "<T>(List<T>, Iterator<Int>, Iterable<T>)",
-        // An index whose type isn't known could be either.
-        "<T>(List<T>, Any, Any)",
     ] {
         env.declare_global_fn(Rc::new(NativeFunction {
             name: "[]=".to_string(),

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 = "<T>(List<T>) -> T")]
-    pub fn pop(list: &mut Vec<Value>) -> Value {
-        list.pop().unwrap_or_else(Value::unit)
+    pub fn pop(list: &mut Vec<Value>) -> anyhow::Result<Value> {
+        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 = "<T>(List<T>) -> T")]
-    pub fn pop_left(list: &mut Vec<Value>) -> Value {
+    pub fn pop_left(list: &mut Vec<Value>) -> anyhow::Result<Value> {
         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

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,
         }
     }

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<Int>, String
 let x = [1,2,3];
 x["foo"] = 69; // ERROR

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<Int>, String
 let x = [1,2,3];
 x["foo"] += 69; // ERROR

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<String>, List<Any>>, List<Int>'
+// (`a[0] = "x"` decides first that `a` holds Strings.)
 let t = ([], []);
 t ++= [1, 2];
 let a, b = t;

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();

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";

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);

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"]));

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]));