diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index a587bf8..d06c02d 100644 --- a/.github/workflows/pytest.yml +++ b/.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 diff --git a/parser/shapeinfer.py b/parser/shapeinfer.py index 652de4b..d017a01 100644 --- a/parser/shapeinfer.py +++ b/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 diff --git a/tests/test_shapeinfer.py b/tests/test_shapeinfer.py index 9dcb837..d50d91b 100644 --- a/tests/test_shapeinfer.py +++ b/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.