From 84241a1b7dfddc2d0bed1b026872b09f0273702e Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Fri, 17 Jul 2026 16:18:27 +0200 Subject: [PATCH] Classify wrapper call-args by their Doxygen @param documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bound-literal pass reports a wrapper call argument as unclassified drift when it is a bare identifier that is neither caller-sourced (PG_GETARG or an =-assigned local), an out-param, nor a literal. That heuristic misses a local filled by reference (`int count; temparr_extract(array, &count)`) or by a macro (`INPUT_AGG_TRANS_STATE(fcinfo, state, ...)`): the identifier names a real MEOS parameter the wrapper derives, not a literal to bind. Those parameters are documented systematically in the MEOS Doxygen — `@param[in] count`, `@param[in,out] state` — so consume that. extract_param_names gathers every @param-documented parameter name, and the bound-literal pass skips a bare-identifier argument bound to a documented parameter. Drift is then confined to a genuinely undocumented parameter, the exceptional case worth inspecting. Regenerating the catalog now reports zero unclassified wrapper args (previously eight: count in tsequence_make, tsequenceset_make, tsequenceset_make_gaps and temporal_merge_array; count1 and count2 in mindistance_tgeoarr_tgeoarr; state in the tcount transition functions), with the bound-literal count unchanged. --- parser/boundargs.py | 32 ++++++++++++++++++++++++-------- parser/outparam.py | 29 +++++++++++++++++++++++++++++ run.py | 5 +++-- tests/test_boundargs.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 10 deletions(-) diff --git a/parser/boundargs.py b/parser/boundargs.py index 2944100..54e42de 100644 --- a/parser/boundargs.py +++ b/parser/boundargs.py @@ -123,13 +123,22 @@ def _literal(arg: str) -> str | None: return None -def _wrapper_bound(body: str, func: dict, drift: list) -> dict[str, str]: +def _wrapper_bound(body: str, func: dict, drift: list, + documented: dict[str, set]) -> dict[str, str]: """The literals wrapper ``body`` binds in its call to ``func['name']``, keyed by - ``func``'s parameter name. Empty if the wrapper does not call ``func`` by name.""" + ``func``'s parameter name. Empty if the wrapper does not call ``func`` by name. + + ``documented`` maps a MEOS function to the set of its ``@param``-documented parameter + names (``parser.outparam.extract_param_names``). A bare-identifier argument bound to a + documented parameter is a value the wrapper reads from the caller or derives — an + array length (``@param[in] count``), an aggregate state (``@param[in,out] state``) — + never a hard-coded literal, so it is skipped systematically. Only a bare identifier + for an UNDOCUMENTED parameter is reported as drift (the exceptional manual gap).""" args = _call_args(body, func["name"]) if not args: return {} assigned = {m.group("var") for m in _ASSIGNED.finditer(body)} + doc_params = documented.get(func["name"], frozenset()) params = func.get("params", []) bound: dict[str, str] = {} for i, a in enumerate(args): @@ -143,14 +152,15 @@ def _wrapper_bound(body: str, func: dict, drift: list) -> dict[str, str]: lit = _literal(a) if lit is not None: bound[pname] = lit - elif _IDENT.match(a): - # a bare identifier that is not caller-sourced and not a literal — - # cannot be trusted as a bound value; report for a look. + elif _IDENT.match(a) and pname not in doc_params: + # a bare identifier that is not caller-sourced, not a literal, and not a + # documented @param (caller-read / derived value) — report for a look. drift.append((func["name"], pname, "unclassified-arg: " + a)) return bound -def merge_boundargs(idl: dict, mdb_src: str | Path) -> tuple[dict, int, list]: +def merge_boundargs(idl: dict, mdb_src: str | Path, + documented: dict[str, set] | None = None) -> tuple[dict, int, list]: """Fold wrapper-bound literals into each function's ``shape.boundArgs``. Functions are grouped by the PG wrapper they share (``mdbC``). Every function in a @@ -164,7 +174,13 @@ def merge_boundargs(idl: dict, mdb_src: str | Path) -> tuple[dict, int, list]: Returns ``(idl, count, drift)`` where ``drift`` lists ``(function, param, reason)`` call arguments the pass could not classify as a caller arg / out-param / literal (a bare identifier that is neither) — a - signal to inspect, never trusted as a bound value.""" + signal to inspect, never trusted as a bound value. + + ``documented`` (from ``parser.outparam.extract_param_names``) maps a MEOS function to + its ``@param``-documented parameter names; a bare identifier bound to one of those is a + caller-read / derived value and is skipped, so drift is confined to genuinely + undocumented parameters.""" + documented = documented or {} wrappers = extract_wrappers(mdb_src) n = 0 drift: list[tuple[str, str, str]] = [] @@ -181,7 +197,7 @@ def merge_boundargs(idl: dict, mdb_src: str | Path) -> tuple[dict, int, list]: # the wrapper calls by name (branches — e.g. the RGEO ternary — agree, first wins) wbound: dict[str, str] = {} for func in group: - for k, v in _wrapper_bound(body, func, drift).items(): + for k, v in _wrapper_bound(body, func, drift, documented).items(): wbound.setdefault(k, v) if not wbound: continue diff --git a/parser/outparam.py b/parser/outparam.py index d7ec17e..b1d499e 100644 --- a/parser/outparam.py +++ b/parser/outparam.py @@ -29,6 +29,35 @@ re.S) # One @param[out] entry: capture the (possibly comma-separated) parameter names. _POUT = re.compile(r'@param\[out\]\s+(?P\w+(?:\s*,\s*\w+)*)', re.S) +# Any @param entry, whatever the direction (``[in]``/``[out]``/``[in,out]``) or none: +# capture every DOCUMENTED parameter name. +_PANY = re.compile(r'@param(?:\[[^\]]*\])?\s+(?P\w+(?:\s*,\s*\w+)*)', re.S) + + +def extract_param_names(meos_root: str | Path) -> dict[str, set]: + """Return ``{function: {every @param-documented parameter name}}`` from the MEOS C + Doxygen (scans both ``src`` and ``include``). + + Whichever direction a parameter carries, an ``@param`` entry names a parameter the + function's documentation OWNS. A wrapper argument bound to such a parameter is a + value the wrapper reads from the caller or DERIVES — an array length + (``@param[in] count``), an aggregate state (``@param[in,out] state``), an out buffer + — never a hard-coded literal. ``boundargs`` consumes this set to skip those + systematically, so a bare-identifier argument only drifts when its parameter is + UNDOCUMENTED (the exceptional manual gap worth inspecting).""" + root = Path(meos_root) + out: dict[str, set] = {} + files = glob.glob(str(root / "src/**/*.c"), recursive=True) + files += glob.glob(str(root / "include/**/*.h"), recursive=True) + for f in files: + txt = Path(f).read_text(errors="ignore") + for m in _FUNC.finditer(txt): + name = m.group("name") + for pm in _PANY.finditer(m.group("doc")): + for p in (n.strip() for n in pm.group("names").split(",")): + if p: + out.setdefault(name, set()).add(p) + return out def extract_outparams(meos_root: str | Path) -> dict[str, list[str]]: diff --git a/run.py b/run.py index 016d6f7..15c88a3 100644 --- a/run.py +++ b/run.py @@ -11,7 +11,7 @@ from parser.header_types import reconcile from parser.shapeinfer import infer_shapes from parser.nullable import merge_nullable -from parser.outparam import merge_outparams +from parser.outparam import extract_param_names, merge_outparams from parser.boundargs import merge_boundargs from parser.enrich import enrich_idl from parser.sqlfn import (attach_sqlfn_map, attach_aggfn_map, lint_ea_sqlfn, @@ -207,7 +207,8 @@ def main(): # bound literals from the wrapper body as `shape.boundArgs`, the input-side # sibling of `shape.outParams`, so a binding emits the literal it can no # longer read off the (narrower) SQL signature. Needs `mdbC` (step 4). - idl, nba, ba_drift = merge_boundargs(idl, MDB_SRC) + idl, nba, ba_drift = merge_boundargs(idl, MDB_SRC, + extract_param_names(_doxy_root)) print(f" Bound-literal args from PG wrappers `shape.boundArgs`: {nba}", file=sys.stderr) if ba_drift: diff --git a/tests/test_boundargs.py b/tests/test_boundargs.py index 2c4aa79..9d6ba39 100644 --- a/tests/test_boundargs.py +++ b/tests/test_boundargs.py @@ -67,6 +67,19 @@ Temporal *result = temporal_append_tinstant(temp, inst, interp, 0.0, NULL, false); PG_RETURN_TEMPORAL_P(result); } + +/** + * @sqlfn fooFromArray() + */ +Datum +Foo_from_array(PG_FUNCTION_ARGS) +{ + ArrayType *array = PG_GETARG_ARRAYTYPE_P(0); + int count; + Temporal **arr = temparr_extract(array, &count); + Temporal *result = foo_from_array(arr, count); + PG_RETURN_TEMPORAL_P(result); +} ''' @@ -87,6 +100,11 @@ def _idl(): # but the wrapper never calls it by name -> inherits {strict:true} by param name {"name": "tbool_value_at_timestamptz", "mdbC": "Temporal_value_at_timestamptz", "params": [{"name": "temp"}, {"name": "t"}, {"name": "strict"}, {"name": "value"}]}, + # `count` is a declared local filled by reference (`temparr_extract(array, &count)`), + # so the `=`-assignment heuristic does not see it; it is classified by its + # @param documentation instead of drifting. + {"name": "foo_from_array", "mdbC": "Foo_from_array", + "params": [{"name": "arr"}, {"name": "count"}]}, ]} @@ -147,6 +165,21 @@ def test_orphan_and_count(self): self.assertNotIn("shape", idl["functions"][4]) self.assertEqual(n, 5) + def test_documented_param_is_not_drift(self): + # `count` (declared, filled via &count) is a documented @param -> caller-derived, + # skipped systematically: no boundArg, no drift. + idl, n, drift = merge_boundargs(_idl(), self.tmp.name, + {"foo_from_array": {"arr", "count"}}) + self.assertFalse([d for d in drift if d[0] == "foo_from_array"]) + foo = next(f for f in idl["functions"] if f["name"] == "foo_from_array") + self.assertNotIn("shape", foo) + + def test_undocumented_param_drifts(self): + # with NO @param documentation for the parameter, the same bare identifier is + # the exceptional gap worth inspecting -> reported as drift. + idl, n, drift = merge_boundargs(_idl(), self.tmp.name) + self.assertIn(("foo_from_array", "count", "unclassified-arg: count"), drift) + if __name__ == "__main__": unittest.main()