Conversation
f2bc15c to
b01b959
Compare
b01b959 to
1f05b41
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The broad generated-code migration and performance-sensitive QL dispatch behavior warrant final human validation.
Review effort: Balanced
Findings: None
What changed in this PR
Adds an inherited codegen option that generates concrete Rust toStringImpl fallbacks while preserving specialized implementations.
Changes:
- Adds and tests the inherited schema pragma.
- Enables fallback generation under
Element, excludingLabelableExpr. - Regenerates Rust QL classes and restores eligible wrappers as generated files.
| File | Description |
|---|---|
misc/codegen/lib/{ql.py,schemadefs.py} |
Defines the new codegen option and pragma. |
misc/codegen/generators/qlgen.py |
Propagates the pragma into QL generation. |
misc/codegen/templates/ql_class.mustache |
Emits the concrete toStringImpl. |
misc/codegen/test/{test_qlgen.py,test_schemaloader.py} |
Tests generation and inherited overrides. |
rust/schema/{prelude.py,annotations.py} |
Enables the fallback and excludes the final custom implementation hierarchy. |
rust/ql/lib/codeql/rust/elements/internal/generated/*.qll |
Adds generated concrete fallbacks. |
rust/ql/lib/codeql/rust/elements/internal/*Impl.qll |
Removes now-generated manual fallbacks. |
rust/ql/.gitattributes |
Reclassifies regenerated wrappers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
hvitved
left a comment
There was a problem hiding this comment.
Nice. I think this means we can get rid of the override string toStringImpl() { none() } in ElementImpl.qll? Did you verify that Element.toString() still doesn't require evaluating path resolution?
|
With the latest commit the generator checks for a manual Removing the I haven't investigated any further, since things seem to otherwise work :) |
This PR adds an additional option to codegen for generating a
toStringImplthat delegates togetAPrimaryQlClass.This reverts the files that #22520 made manual into generated once.
The option is rather Rust specific, but oh well.