diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index ca91be7..5aed1c7 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 341 + run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 343 # 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/parser.py b/parser/parser.py index e1eaa73..04c3924 100644 --- a/parser/parser.py +++ b/parser/parser.py @@ -170,7 +170,7 @@ def parse_meos(entry: Path, include_dir: Path, struct["name"] = typedef_name else: continue - structs.append(struct) + structs.append((struct, node.is_definition())) elif node.kind == clang.cindex.CursorKind.ENUM_DECL and node.spelling: enums.append(extract_enum(node)) @@ -186,8 +186,24 @@ def _dedup(items: list) -> list: result.append(item) return result + def _dedup_structs(items: list) -> list: + """One record per structure, as #_dedup keeps one per name, read from its + definition when the unit holds one: a forward declaration (`struct varlena;`) + met before the definition has no fields, and a structure only ever declared + (opaque, `struct NumericData;`) keeps its declaration. The structure keeps the + place where the parse first meets it.""" + order, chosen = [], {} + for struct, is_def in items: + name = struct["name"] + if name not in chosen: + order.append(name) + chosen[name] = (struct, is_def) + elif is_def and not chosen[name][1]: + chosen[name] = (struct, is_def) + return [chosen[name][0] for name in order] + functions = _dedup(functions) - structs = _dedup(structs) + structs = _dedup_structs(structs) enums = _dedup(enums) macros = _dedup(macros) diff --git a/tests/test_struct_layout.py b/tests/test_struct_layout.py index eef2fa2..fdf901a 100644 --- a/tests/test_struct_layout.py +++ b/tests/test_struct_layout.py @@ -79,6 +79,33 @@ def test_core_structs_have_real_offsets(self): fld["offset_bits"], 0, f"{s['name']}.{fld['name']} has unresolved offset") + def test_a_structure_declared_first_is_read_from_its_definition(self): + # `meos.h` declares MeosArray and SkipList before `meos_internal.h` defines + # them, so their layout is read where they are defined + for name, field in (("MeosArray", "count"), ("SkipList", "capacity"), + ("varlena", "vl_len_")): + self.assertIn(field, self._fields(name), name) + + +class ForwardDeclarationTests(unittest.TestCase): + """#_dedup_structs of parser/parser.py over headers that declare a structure before + defining it, and one they only declare, parsed as #StructLayoutTests's catalog is.""" + + def test_the_definition_wins_and_an_opaque_structure_stays(self): + import os + import tempfile + if not os.environ.get("MDB_SRC_ROOT"): + self.skipTest("MDB_SRC_ROOT not set; the parse reads MobilityDB's families") + from parser.parser import parse_all_headers + with tempfile.TemporaryDirectory() as d: + root = Path(d) + (root / "a.h").write_text("struct S;\nstruct O;\n" + "extern int f(struct S *s, struct O *o);\n") + (root / "b.h").write_text("struct S { int x; double y; };\n") + idl = parse_all_headers(root) + structs = {s["name"]: [f["name"] for f in s["fields"]] for s in idl["structs"]} + self.assertEqual(structs, {"S": ["x", "y"], "O": []}) + if __name__ == "__main__": unittest.main()