Codegen: improve implementation of generated parent/child relationship

[redsun82]

This improves the implementation of the generated parent/child relationship by adding a new all_children field to ql.Class which lists all children (both direct and inherited) of a class, carefully avoiding duplicating children in case of diamond inheritance. This:

• simplifies the generated code,
• avoids children ambiguities in case of diamond inheritance.

This only comes with some changes in the order of children in the generated tests (we were previously sorting bases alphabetically there). For the rest this is a non-functional change, particularly highlighted by the fact that no PrintAst tests changed their results. It also only changes a private module, so no change note is even required.

While we currently have no diamond inheritance of child fields, we will have it shortly in Rust.

[Copilot]

Pull Request Overview

This PR enhances codegen for parent/child relationships by introducing an all_children list on ql.Class, ensuring both direct and inherited children are included without duplication. It refactors generators to a Resolver class, updates templates to iterate over all_children, and aligns tests with the new API.

• Add all_children support and replace prev_child logic across lib, templates, and generators
• Refactor qlgen into a stateful Resolver with caching and diamond-safe traversal
• Update tests in test_qlgen.py/test_ql.py and checksums in generated file lists

Reviewed Changes

Copilot reviewed 8 out of 19 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
swift/ql/.generated.list	Updated checksum for ParentChild.qll reflecting template changes
rust/ql/.generated.list	Updated checksum for ParentChild.qll reflecting template changes
misc/codegen/test/test_qlgen.py	Adjust tests to assert all_children and drop prev_child args
misc/codegen/test/test_ql.py	Replace properties-based child checks with all_children usage
misc/codegen/templates/ql_parent.mustache	Refactor getImmediateChildOf to loop over all_children
misc/codegen/lib/ql.py	Add Child class, is_child flag, and all_children on Class
misc/codegen/generators/rusttestgen.py	Use Resolver.should_skip_qltest instead of previous skip logic
misc/codegen/generators/qlgen.py	Introduce Resolver class, caching, and new property/class methods

Comments suppressed due to low confidence (4)

misc/codegen/generators/qlgen.py:254

• [nitpick] Add a docstring to _get_all_properties to explain its traversal order, deduplication strategy, and how it handles inherited properties.

    def _get_all_properties(

misc/codegen/test/test_qlgen.py:474

• Consider adding a dedicated test case for a diamond inheritance scenario to verify that all_children correctly avoids duplicate entries when multiple bases introduce the same child property.

        [

misc/codegen/generators/rusttestgen.py:63

• The qlgen module is referenced here but not imported; please add import qlgen at the top of rusttestgen.py to avoid a NameError.

        resolver = qlgen.Resolver(schema.classes)

misc/codegen/templates/ql_parent.mustache:12

• [nitpick] This deeply nested logic around getImmediateChildOf{{name}} can be hard to follow. Consider refactoring into smaller sections or adding clarifying comments to improve readability.

    {{#final}}

[redsun82]

Hmm, I'm a bit surprised there were some changes in results highlighted by DCA on the rust experiment. @hvitved is there maybe some instability there?

[hvitved]

Hmm, I'm a bit surprised there were some changes in results highlighted by DCA on the rust experiment. @hvitved is there maybe some instability there?

Yes, I have also seen some wobble when databases are not cached.

[redsun82]

Hmm, I'm a bit surprised there were some changes in results highlighted by DCA on the rust experiment. @hvitved is there maybe some instability there?

Yes, I have also seen some wobble when databases are not cached.

I see.

I've tried locally slapping a PrintAst.qlref test on the controlflow test, and also saw no change, so I'm pretty confident this can't be introducing any actual change in the library.

[redsun82]

In order to be sure I'm rerunning a DCA experiment for rust with database caching (I have it off by default because I usually work on the extractor).

[redsun82]

@hvitved even with cached databases there was a small variation, but looking better than without cached DBs. I'm willing to accept that, even if I can't fully explain it. What do you think?

[hvitved]

@hvitved even with cached databases there was a small variation, but looking better than without cached DBs

I see your DCA run still uses -X 'change-ql-submodule-in-semmle-code=true'; AFIK that effectively disables caching (see https://github.com/github/codeql-dca/issues/2530).

[redsun82]

@hvitved even with cached databases there was a small variation, but looking better than without cached DBs

I see your DCA run still uses -X 'change-ql-submodule-in-semmle-code=true'; AFIK that effectively disables caching (see github/codeql-dca#2530).

aha, I see!

Let me try disabling that as well then.

[redsun82]

@hvitved that's it, no changes at all this time around, thanks for the support!

[hvitved]

Looks plausible to me