From 1aa02f3367511f2aedb7ae66e0d3aa2319b4f322 Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Thu, 3 Sep 2026 03:33:48 +0200 Subject: [PATCH] State the array arguments a function reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shape says which arrays a function ANSWERS and leaves the ones it READS to each binding to recognise, and the only thing there to recognise them by is the length parameter's name. Those names disagree across the surface — `count`, `count1`, `size`, `ngeoms`, `keys_len`, `path_len`, `pixels_size`, `wkb_size` — so a binding that knows some of them silently drops the rest, and `geo_cluster_kmeans`, `jsonb_delete_array` and their kin reach no binding at all. `shape.inputArrays` states them, the sibling of `arrayReturn` and `outputArrays`: the parameter, the parameter its length comes from, and the element type with one pointer level off. 158 arguments across the surface. The discriminator is the one this module already reads in the other direction. An output array's length is passed BY POINTER, since the callee fills it in; an input array's is passed BY VALUE, since the caller already knows it. So an input array is a parameter that is an array of pointers or of by-value scalars followed by a by-value integer — `jsonb_each`'s written-back `Jsonb **values` is not one, its `int *count` saying so. A pointer to a MEOS VALUE type beside an integer is a value and a number, never an array: `text_left(text *txt, int n)` takes one text and a character count, `jsonb_hash_extended` one jsonb and a seed, `interval_in` one string and a typmod. Only a pointer to a C scalar is an array of them, and `char *` is a string in every binding rather than an array of characters. Six tests hold the rule to those cases, the two that already stated an input array is not an output one now stating what it is instead. --- .github/workflows/pytest.yml | 2 +- parser/shapeinfer.py | 77 ++++++++++++++++++++++++++++++++++-- run.py | 8 ++-- tests/test_shapeinfer.py | 70 +++++++++++++++++++++++++++++--- 4 files changed, 145 insertions(+), 12 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index aa58e4a..272fc38 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 300 + run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 304 # 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 7b7c117..aa30a32 100644 --- a/parser/shapeinfer.py +++ b/parser/shapeinfer.py @@ -6,6 +6,7 @@ TYPE *f(..., int *count) -> returns an array of ``count`` TYPE **f(..., TYPE **extra, int *count) -> primary array return PLUS one or more parallel out-arrays + f(..., TYPE **values, int count, ...) -> reads an array of ``count`` The output length is always passed *by pointer* (``int *count``); an *input* array instead carries its length *by value* (``int count``). That pointer/value @@ -65,6 +66,71 @@ def _strip_one_ptr(ctype: str) -> str: return s +#: The C scalars an array of by-value elements is made of. A pointer to one of +#: these beside a length is an array; a pointer to a MEOS value type beside an +#: integer is a value and a number, as ``text_left(text *txt, int n)`` is. +#: ``char *`` is a string in every binding and never an array of characters. +_ELEMENT_SCALARS = frozenset({ + "bool", "int8", "int8_t", "uint8", "uint8_t", "short", "int16", "int16_t", + "uint16", "uint16_t", "int", "int32", "int32_t", "uint32", "uint32_t", + "float", "Oid", "DateADT", "long", "int64", "int64_t", "uint64", + "uint64_t", "double", "float8", "Datum", "Timestamp", "TimestampTz", + "TimeADT", "TimeOffset", "size_t", +}) + +#: The by-value integer spellings a length is written in. +_LENGTH_TYPES = frozenset({ + "int", "int32", "int32_t", "uint32", "uint32_t", "int64", "int64_t", + "uint64", "uint64_t", "size_t", "int16", +}) + + +def _bare(ctype: str) -> str: + return " ".join((ctype or "").replace("const ", "").split()) + + +def _input_arrays(func: dict) -> list: + """The array ARGUMENTS a function reads, with the parameter each takes its + 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. + + 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. + """ + params = func.get("params", []) + out = [] + for i, prm in enumerate(params[:-1]): + 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): + 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"))), + }, + }) + return out + + def _is_index_pair_return(func: dict, count: str) -> bool: """Whether the ``int *`` return is a FLATTENED array of index PAIRS. @@ -98,11 +164,15 @@ def infer_shapes(idl: dict) -> tuple[dict, dict]: """Populate ``func['shape']`` with ``arrayReturn``/``outputArrays`` derived from the signatures. Returns ``(idl, stats)``. Idempotent and additive: only the array-output families are touched, everything else is untouched.""" - n_arr = n_oa = 0 + n_arr = n_oa = n_ia = 0 for func in idl["functions"]: + inputs = _input_arrays(func) + if inputs: + func.setdefault("shape", {})["inputArrays"] = inputs + n_ia += len(inputs) count = _out_count_param(func) if not count: - continue # not array-returning; nothing to infer + continue # not array-returning; nothing more to infer shape = func.setdefault("shape", {}) # The primary pointer return takes its length from the output count. rtype = func.get("returnType", {}) @@ -134,4 +204,5 @@ def infer_shapes(idl: dict) -> tuple[dict, dict]: if out: shape["outputArrays"] = out n_oa += len(out) - return idl, {"arrayReturn": n_arr, "outputArrays": n_oa} + return idl, {"arrayReturn": n_arr, "outputArrays": n_oa, + "inputArrays": n_ia} diff --git a/run.py b/run.py index d0e82c2..17bc85e 100644 --- a/run.py +++ b/run.py @@ -129,11 +129,13 @@ def main(): file=sys.stderr) # 1d. Generate the codegen `shape` from the signatures + Doxygen, replacing - # the hand-maintained meta stub. outputArrays/arrayReturn come from the - # parameter forms; nullable comes from the C `@param ... may be NULL` SoT. + # the hand-maintained meta stub. inputArrays/outputArrays/arrayReturn + # come from the parameter forms; nullable comes from the C + # `@param ... may be NULL` SoT. idl, sh = infer_shapes(idl) print(f" inferred shape: {sh['arrayReturn']} array returns, " - f"{sh['outputArrays']} output arrays", file=sys.stderr) + f"{sh['outputArrays']} output arrays, " + f"{sh['inputArrays']} input arrays", file=sys.stderr) # The `may be NULL` / `@param[out]` Doxygen tags live in the MEOS C *source* # (meos/src/**/*.c), not the parsed header tree. On the build-libmeos path # HEADERS_DIR is the INSTALLED headers (generated meos_export.h, no src/), so diff --git a/tests/test_shapeinfer.py b/tests/test_shapeinfer.py index b3363c8..00f85a4 100644 --- a/tests/test_shapeinfer.py +++ b/tests/test_shapeinfer.py @@ -5,7 +5,7 @@ * a written-back out-array pairs with a by-pointer ``int *count`` (the callee fills the length) -> ``outputArrays`` + ``arrayReturn.lengthFrom`` -* a read-only in-array pairs with a by-value ``int count`` -> left untouched +* a read-only in-array pairs with a by-value ``int count`` -> ``inputArrays`` Plain unittest, no pytest dependency; fully synthetic IDL, no build artifacts. """ @@ -95,17 +95,22 @@ def test_index_pair_rule_needs_an_array_argument(self): idl, _ = infer_shapes(idl) self.assertNotIn("groupSize", idl["functions"][0]["shape"]["arrayReturn"]) - def test_input_array_with_value_count_untouched(self): + def test_input_array_with_value_count_is_read_not_written(self): # tsequence_make-style: ** input array carries its length BY VALUE idl = {"functions": [_fn( "tsequence_make", "TSequence *", [("instants", "const TInstant **"), ("count", "int"), ("lower_inc", "bool")])]} idl, stats = infer_shapes(idl) - self.assertNotIn("shape", idl["functions"][0]) + shape = idl["functions"][0]["shape"] self.assertEqual(stats["outputArrays"], 0) + self.assertNotIn("outputArrays", shape) + self.assertEqual(shape["inputArrays"], [{ + "param": "instants", + "lengthFrom": {"kind": "param", "name": "count"}, + "element": {"c": "TInstant *", "canonical": "TInstant *"}}]) - def test_nonconst_input_array_with_value_count_untouched(self): + def test_nonconst_input_array_with_value_count_is_read_not_written(self): # tsequenceset_make_gaps-style: non-const ** but BY-VALUE count => input idl = {"functions": [_fn( "tsequenceset_make_gaps", "TSequenceSet *", @@ -113,7 +118,62 @@ def test_nonconst_input_array_with_value_count_untouched(self): ("maxt", "const Interval *")])]} idl, stats = infer_shapes(idl) self.assertEqual(stats["outputArrays"], 0) - self.assertNotIn("shape", idl["functions"][0]) + self.assertEqual( + idl["functions"][0]["shape"]["inputArrays"][0]["param"], "instants") + + def test_a_length_is_read_by_position_not_by_its_name(self): + # The names disagree across the surface — `ngeoms`, `keys_len`, `size` + # — and each is the length of the array before it. + idl = {"functions": [ + _fn("geo_cluster_kmeans", "int *", + [("geoms", "const GSERIALIZED **"), ("ngeoms", "uint32_t"), + ("k", "uint32_t"), ("count", "int *")]), + _fn("jsonb_delete_array", "Jsonb *", + [("jb", "const Jsonb *"), ("keys_elems", "text **"), + ("keys_len", "int")]), + _fn("set_from_wkb", "Set *", + [("wkb", "const uint8_t *"), ("size", "size_t")]), + ]} + idl, stats = infer_shapes(idl) + lengths = {f["name"]: f["shape"]["inputArrays"][0]["lengthFrom"]["name"] + for f in idl["functions"]} + self.assertEqual(lengths, {"geo_cluster_kmeans": "ngeoms", + "jsonb_delete_array": "keys_len", + "set_from_wkb": "size"}) + self.assertEqual(stats["inputArrays"], 3) + # The byte buffer's element is the scalar itself, not a pointer to one. + self.assertEqual( + idl["functions"][2]["shape"]["inputArrays"][0]["element"]["c"], + "uint8_t") + + def test_a_value_beside_a_number_is_not_an_array(self): + # `text_left(text *txt, int n)` takes ONE text and a character count; + # a pointer to a MEOS value type beside an integer says nothing about + # an array, and only a pointer to a C scalar does. + idl = {"functions": [ + _fn("text_left", "text *", [("txt", "text *"), ("n", "int")]), + _fn("jsonb_hash_extended", "uint64_t", + [("jb", "const Jsonb *"), ("seed", "uint64_t")]), + _fn("interval_in", "Interval *", + [("str", "const char *"), ("typmod", "int32")]), + ]} + idl, stats = infer_shapes(idl) + self.assertEqual(stats["inputArrays"], 0) + for f in idl["functions"]: + self.assertNotIn("inputArrays", f.get("shape", {})) + + def test_an_out_array_is_not_read_as_an_input_one(self): + # `jsonb_each(jb, Jsonb **values, int *count)` writes `values` back, and + # its length being BY POINTER is what says so. + idl = {"functions": [_fn( + "jsonb_each", "text **", + [("jb", "const Jsonb *"), ("values", "Jsonb **"), + ("count", "int *")])]} + idl, stats = infer_shapes(idl) + shape = idl["functions"][0]["shape"] + self.assertEqual(stats["inputArrays"], 0) + self.assertNotIn("inputArrays", shape) + self.assertEqual(shape["outputArrays"], [{"param": "values"}]) if __name__ == "__main__":