Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions parser/boundargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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
Expand All @@ -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]] = []
Expand All @@ -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
Expand Down
29 changes: 29 additions & 0 deletions parser/outparam.py
Original file line number Diff line number Diff line change
Expand Up @@ -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<names>\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<names>\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]]:
Expand Down
5 changes: 3 additions & 2 deletions run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
33 changes: 33 additions & 0 deletions tests/test_boundargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
'''


Expand All @@ -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"}]},
]}


Expand Down Expand Up @@ -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()
Loading