Uh oh!
There was an error while loading. Please reload this page.
Rust: add documentation for AST nodes - #19630
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds detailed documentation comments for various Rust AST node classes and updates the .gitattributes to include generated test files that were previously marked as missing.
- Enriches QLL class docs with clear descriptions and examples for associated items, inline assembly elements, array types, argument lists, and ABI specs.
- Introduces a constructor predicate in
ControlFlowGraphImpl.qllto filter out macro pattern calls. - Replaces
MISSING_SOURCE.txtentries in.gitattributeswith actual.qltest paths for inline assembly and other elements.
Reviewed Changes
Copilot reviewed 644 out of 644 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/elements/AssocTypeArg.qll | Added a descriptive doc comment and example for associated type arguments. |
| rust/ql/lib/codeql/rust/elements/AssocItemList.qll | Extended doc to mention Impl contexts; refined formatting. |
| rust/ql/lib/codeql/rust/elements/AssocItem.qll | Updated comment to describe associated items in traits/impls with example. |
| rust/ql/lib/codeql/rust/elements/Asm*.qll | Added docs and examples for inline assembly symbols, registers, options, etc. |
| rust/ql/lib/codeql/rust/elements/ArrayTypeRepr.qll | Documented array type representations with example. |
| rust/ql/lib/codeql/rust/elements/ArgList.qll | Clarified argument list usage in calls with example. |
| rust/ql/lib/codeql/rust/elements/Abi.qll | Documented ABI specs for extern functions/blocks. |
| rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll | Added MacroCallTree() predicate to exclude calls in macro patterns. |
| rust/ql/.gitattributes | Replaced missing-source placeholders with actual generated .ql test entries. |
Uh oh!
There was an error while loading. Please reload this page.
| | test.rs:455:13:455:25 | [match(false)] 1 \| 2 | no-match | test.rs:455:13:455:25 | one_or_two!... | | ||
| | test.rs:455:13:455:25 | [match(false)] 1 \| 2 | no-match | test.rs:456:13:456:13 | _ | | ||
| | test.rs:455:13:455:25 | [match(true)] 1 \| 2 | match | test.rs:455:13:455:25 | one_or_two!... | | ||
| | test.rs:455:13:455:25 | [match(true)] 1 \| 2 | match | test.rs:455:30:455:30 | 3 | |
| Foo<'a> | ||
| // ^^ | ||
| let text: Text<'a>; | ||
| // ^^ |
| | gen_arg_list.rs:5:5:5:11 | ArgList | 0 | gen_arg_list.rs:5:5:5:11 | "not yet implemented" | | ||
| | gen_arg_list.rs:7:8:7:16 | ArgList | 0 | gen_arg_list.rs:7:9:7:9 | 1 | | ||
| | gen_arg_list.rs:7:8:7:16 | ArgList | 1 | gen_arg_list.rs:7:12:7:12 | 2 | | ||
| | gen_arg_list.rs:7:8:7:16 | ArgList | 2 | gen_arg_list.rs:7:15:7:15 | 3 | |
There was a problem hiding this comment.
Why are we getting better test results from doc changes? Are these the tests of the example code you were talking about earlier today?
There was a problem hiding this comment.
All the examples in triple backticks are place in source files for QL tests. That way we can test that the examples in the docs are actually valid. The "not yet implemented" stuff comes from the expansion of the todo!() macro.
| ```rust | ||
| todo!() | ||
| extern "C" fn foo() {} | ||
| // ^^^ |
There was a problem hiding this comment.
New documentation 👍 (I won't claim to have picked through it with a fine-toothed comb, but it all looks good at a glance, and is certainly an improvement on all those todo!()s).
| ```rust | ||
| todo!() | ||
| fn foo<T, U>(t: T, u: U) {} | ||
| // ^ ^ |
There was a problem hiding this comment.
The ^ are a helpful addition in many of these examples.
There was a problem hiding this comment.
They were co-piliot's idea, and I quite like them too. Ideally we make those ^^ things inline expectation markers, so we get even stronger testing of the documentation examples. @redsun82 perhaps something for a rainy afternoon ;-)
There was a problem hiding this comment.
Perhaps copilot was inspired by the excellent comments in https://github.com/github/codeql/blob/main/shared/typeinference/codeql/typeinference/internal/TypeInference.qll ;-)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| @@ -1069,9 +1142,17 @@ class _: | |||
| @annotate(ForTypeRepr) | |||
| class _: | |||
| """ | |||
| A ForTypeRepr. For example: | |||
| A higher-ranked trait bound(HRTB) type. | |||
There was a problem hiding this comment.
| Ahigher-rankedtraitbound(HRTB)type. | |
| Ahigher-rankedtraitbound(HRTB). |
There was a problem hiding this comment.
I dropped the acronym, actually not much point defining a fancy acronym if it is not used anywhere in the text
| println!("{} {}!", "Hello", "world"); | ||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| ``` | ||
| ``` |
redsun82
left a comment
There was a problem hiding this comment.
LGTM, that's a very good improvement in both documentation and testing 👍
Some minor cosmetic nits, but this is really neat!
geoffw0
commented
Jun 4, 2025
My comments have all been addressed and I'm [very] happy with the PR. @redsun82 please approve if you're happy your comments have been addressed as well. |
This pull request adds missing documentation for Rust AST nodes. Best reviewed commit-by-commit, skipping the codegen ones. The most important changed files is
rust/schema/annotations.pywhere the missing documentation was added.