Shared/Unified: Hoist members in generated Ast.qll - #22507
Conversation
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The generated hierarchy and consumer update are consistent, and no unresolved correctness issues were found.
Review tier: Balanced
Findings: None
What changed in this PR
Hoists shared generated AST predicates onto common superclasses, enabling polymorphic access without subtype casts.
Changes:
- Adds abstract predicate generation and override detection.
- Hoists
Callable.getBody(). - Simplifies control-flow body lookup.
| File | Description |
|---|---|
shared/tree-sitter-extractor/src/generator/ql.rs |
Supports abstract predicates. |
shared/tree-sitter-extractor/src/generator/ql_gen.rs |
Computes and hoists common predicates. |
unified/ql/lib/codeql/unified/internal/Ast.qll |
Adds generated Callable.getBody() hierarchy. |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll |
Uses polymorphic body access. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Looks great!
Did you try regenerating the AST for Ruby and QL4QL? Were they unaffected by this?
Not blocking for this PR, but I think in the future we'll want the ability to manually declare fields on super-types to account for a few situations:
- There are subtypes where the field has a more precise type thus failing an exact signature match
- Adding a new subtype that lacks a previously-common field can cause a breaking AST change.
- Sometimes it's just nice to have getters for fields that are present in the common case, but some subtypes are lacking it. Defaulting to
none()in the base class.
I admit I did not. But copilot claimed that it did, and that they were unaffected.
Completely agree! |
This hoists identical predicates shared by all subclasses to a root-def on their superclass. The second commit shows the effect.
Copilot wrote most of the Rust code, but I've tweaked it a bit and I've read through it all and convinced myself that it does the right thing.