Give every array of a run the one length that follows it

Arrays read in parallel are declared together and counted once, so the length
belongs to all of them. `inputArrays` names only the array the count sits
beside, which leaves nine arrays across seven functions with no stated length:
`jsonb_make_two_arg(text **keys, text **values, int count)` pairs the two
element by element and the entry names `values` alone, `tpointseq_make_coords`
reads four coordinate arrays and the entry names `times`, and the h3 and
quadbin sequence constructors each read a value array beside their timestamps.
A binding meets the unnamed ones as bare pointers it cannot size.

The inference walks a RUN of array parameters to the first parameter that is
not one, and where that parameter is a by-value integer every array of the run
takes its length from it. Where a family counts each array separately the run
is one long and this says what it always said:
`edwithin_tgeoarr_tgeoarr(arr1, count1, arr2, count2, …)` keeps `arr1` on
`count1`, which is the case the suite states beside the run itself.

The catalog carries 167 input arrays over 158, the nine gained belonging to
those seven functions, and no entry loses the length it had.

Author: Esteban Zimanyi

.github/workflows/pytest.yml

@@ -96,7 +96,7 @@ jobs:
       # carries, or a change to them is not exercised until after it merges.
       # Consumers use the action; this repository owns the rules.
       - name: Refuse a skip, and a suite that shrank
-        run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 315
+        run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 317
 
       # The rules earn their place by refusing a log that carries what they
       # name. Both fixtures are written here rather than tracked, and the

parser/shapeinfer.py

@@ -110,40 +110,57 @@ def _input_arrays(func: dict) -> list:
     length from.
 
     An input array is a parameter that is an array of pointers (``TYPE **``) or
-    of by-value scalars (``uint8_t *``, ``int64_t *``), immediately followed by
-    a by-value integer.  That the length is by VALUE is what tells an argument
-    apart from a written-back out-array, whose length is by POINTER — the same
-    distinction this module already reads in the other direction.
+    of by-value scalars (``uint8_t *``, ``int64_t *``), followed by a by-value
+    integer.  That the length is by VALUE is what tells an argument apart from
+    a written-back out-array, whose length is by POINTER — the same distinction
+    this module already reads in the other direction.
 
     Without it a binding matches the LENGTH PARAMETER'S NAME, and the names
     disagree: ``count``, ``size``, ``ngeoms``, ``keys_len``, ``path_len``,
     ``pixels_size``, ``wkb_size``, ``count1``.  Every one of them is a length,
     and a binding that knows only some of them silently drops the rest.
+
+    A RUN of arrays shares the one length that follows it.  Arrays read in
+    parallel are declared together and counted once — ``jsonb_make_two_arg(text
+    **keys, text **values, int count)`` pairs the two element by element, and
+    ``tpointseq_make_coords`` reads four — so the length belongs to every array
+    of the run, not only to the one the count happens to sit beside.  Where a
+    family counts each array separately the run is one long and this says what
+    it always said: ``edwithin_tgeoarr_tgeoarr(arr1, count1, arr2, count2, …)``
+    keeps ``arr1`` on ``count1``.
     """
     params = func.get("params", [])
-    out = []
-    for i, prm in enumerate(params[:-1]):
+
+    def is_array(prm) -> bool:
         ctype = _bare(prm.get("cType"))
-        if _bare(params[i + 1].get("cType")) not in _LENGTH_TYPES:
-            continue
         if ctype.endswith("**"):
-            if ctype in ("char **", "void **"):
-                continue
-        elif not (ctype.endswith("*")
-                  and ctype[:-1].strip() in _ELEMENT_SCALARS):
+            return ctype not in ("char **", "void **")
+        return ctype.endswith("*") and ctype[:-1].strip() in _ELEMENT_SCALARS
+
+    out = []
+    start = 0
+    while start < len(params):
+        if not is_array(params[start]):
+            start += 1
             continue
-        out.append({
-            "param": prm["name"],
-            "lengthFrom": {"kind": "param", "name": params[i + 1]["name"]},
-            # The element reads as the return's does — the type with one
-            # pointer level off and no `const`, which belongs to the argument
-            # rather than to the element type a binding marshals.
-            "element": {
-                "c": _strip_one_ptr(_bare(prm.get("cType"))),
-                "canonical": _strip_one_ptr(
-                    _bare(prm.get("canonical") or prm.get("cType"))),
-            },
-        })
+        end = start
+        while end < len(params) and is_array(params[end]):
+            end += 1
+        if end < len(params) and _bare(params[end].get("cType")) in _LENGTH_TYPES:
+            for prm in params[start:end]:
+                out.append({
+                    "param": prm["name"],
+                    "lengthFrom": {"kind": "param", "name": params[end]["name"]},
+                    # The element reads as the return's does — the type with one
+                    # pointer level off and no `const`, which belongs to the
+                    # argument rather than to the element type a binding marshals.
+                    "element": {
+                        "c": _strip_one_ptr(_bare(prm.get("cType"))),
+                        "canonical": _strip_one_ptr(
+                            _bare(prm.get("canonical") or prm.get("cType"))),
+                    },
+                })
+        start = end
     return out
 
 

tests/test_shapeinfer.py

@@ -162,6 +162,44 @@ def test_a_value_beside_a_number_is_not_an_array(self):
         for f in idl["functions"]:
             self.assertNotIn("inputArrays", f.get("shape", {}))
 
+    def test_a_run_of_arrays_shares_the_length_that_follows_it(self):
+        # `jsonb_make_two_arg(text **keys, text **values, int count)` pairs the
+        # two arrays element by element and counts them once, so `count` is the
+        # length of BOTH; reading only the array it sits beside drops the other.
+        idl = {"functions": [_fn(
+            "jsonb_make_two_arg", "Jsonb *",
+            [("keys", "text **"), ("values", "text **"), ("count", "int")]),
+            _fn("tpointseq_make_coords", "TSequence *",
+                [("xcoords", "const double *"), ("ycoords", "const double *"),
+                 ("zcoords", "const double *"), ("times", "const TimestampTz *"),
+                 ("count", "int"), ("srid", "int32_t")])]}
+        idl, stats = infer_shapes(idl)
+        self.assertEqual(
+            [(a["param"], a["lengthFrom"]["name"])
+             for a in idl["functions"][0]["shape"]["inputArrays"]],
+            [("keys", "count"), ("values", "count")])
+        self.assertEqual(
+            [(a["param"], a["lengthFrom"]["name"])
+             for a in idl["functions"][1]["shape"]["inputArrays"]],
+            [("xcoords", "count"), ("ycoords", "count"),
+             ("zcoords", "count"), ("times", "count")])
+        self.assertEqual(stats["inputArrays"], 6)
+
+    def test_an_array_counted_on_its_own_keeps_its_own_length(self):
+        # The counter-case the run rule must not swallow: a family that counts
+        # each array separately declares each one beside ITS length, so every
+        # run is one long and each array keeps the count it is declared with.
+        idl = {"functions": [_fn(
+            "edwithin_tgeoarr_tgeoarr", "int *",
+            [("arr1", "const Temporal **"), ("count1", "int"),
+             ("arr2", "const Temporal **"), ("count2", "int"),
+             ("dist", "double"), ("count", "int *")])]}
+        idl, _ = infer_shapes(idl)
+        self.assertEqual(
+            [(a["param"], a["lengthFrom"]["name"])
+             for a in idl["functions"][0]["shape"]["inputArrays"]],
+            [("arr1", "count1"), ("arr2", "count2")])
+
     def test_an_out_array_is_not_read_as_an_input_one(self):
         # `temporal_time_split(temp, ..., TimestampTz **bins, int *count)` writes
         # `bins` back, and its length being BY POINTER is what says so.