Skip to content

Rust: Avoid path resolution in Element.toString - #22520

Merged
hvitved merged 1 commit into
mainfrom
hvitved-rust-tostring-implementations
Sep 9, 2026
Merged

Rust: Avoid path resolution in Element.toString#22520
hvitved merged 1 commit into
mainfrom
hvitved-rust-tostring-implementations

Conversation

@hvitved

@hvitved hvitved commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

This PR removes the dependency on evaluating path resolution when evaluating Element.toString (and hence any query whatsoever). The dependency happened because the root def of Element.toString() had a fallback call to getAPrimaryQlClass, which in turn requires evaluating path resolution (e.g. here).

This PR avoids the root def call by instead pushing those calls down to classes that are currently lacking a proper toString implementation, and to help verify my changes I also added a new consistency check for missing toString implementations, which revealed existing bugs that could all be handled by adding concat(...) wrappers to account for missing AST children.

DCA shows that we have a few more AST inconsistencies (because of the newly added check), but also that data flow inconsistencies go down (because of an existing data flow consistency check for missing toString), but most importantly DIL sizes decrease as expected.

@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Sep 7, 2026
@hvitved
hvitved force-pushed the hvitved-rust-tostring-implementations branch 5 times, most recently from 0d5bb66 to f9d728d Compare September 8, 2026 08:18
Define string representations on concrete element classes, make existing renderers total when unresolved child data is absent, and remove the path-resolution-dependent fallback from Element.

Add a consistency check for elements that lack a string representation and update generated-file metadata for customized wrappers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hvitved
hvitved force-pushed the hvitved-rust-tostring-implementations branch from f9d728d to c05fc73 Compare September 8, 2026 13:14
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Sep 9, 2026
@hvitved
hvitved marked this pull request as ready for review September 9, 2026 08:30
@hvitved
hvitved requested a review from a team as a code owner September 9, 2026 08:31
Copilot AI balanced review requested due to automatic review settings September 9, 2026 08:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Attribute-macro item placeholders now dispatch to incomplete concrete strings such as fn instead of their dedicated representation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ItemImpl.qll — This no longer preserves the special string for attribute-macro placeholders. For a placeholder…
What changed in this PR

Removes the global path-resolution dependency from Rust AST Element.toString() by moving fallback formatting to concrete element classes.

Changes:

  • Replaces the root fallback with concrete toStringImpl implementations.
  • Handles incomplete AST children with concat.
  • Adds missing-toString consistency diagnostics and updates test baselines.
File Description
rust/​ql/​test/​query-tests/​diagnostics/​AstConsistencyCounts.expected Adds the new consistency count.
rust/​ql/​test/​extractor-tests/​macro-in-library/​PrintAst.expected Updates macro-call output.
rust/​ql/​test/​extractor-tests/​macro-expansion/​test.expected Updates macro-expansion strings.
rust/​ql/​test/​extractor-tests/​macro-expansion/​PrintAst.expected Updates printed macro placeholders.
rust/​ql/​lib/​codeql/​rust/​internal/​AstConsistency.qll Detects missing string representations.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​YieldExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​YeetExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​WherePredImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​WhereClauseImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​VariantListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​UseTreeListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​UseImpl.qll Handles missing use trees.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​UseBoundGenericArgsImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​UnsafeMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TypeBoundListImpl.qll Totalizes abbreviated output.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TypeBoundImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TypeArgImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TupleTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TuplePatImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TupleFieldListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TupleFieldImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TupleExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TryExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TryBlockModifierImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TraitImpl.qll Handles missing trait names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TokenTreeMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​TokenTreeImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StructPatFieldListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StructImpl.qll Handles missing struct names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StructFieldListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StructExprFieldListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StmtListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​StaticImpl.qll Handles missing static names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​SourceFileImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​SliceTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​SlicePatImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​SelfParamImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ReturnTypeSyntaxImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​RetTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​RenameImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​RefTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​RangePatImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​PtrTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​PathSegmentImpl.qll Totalizes abbreviated output.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​PathMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ParenTypeReprImpl.qll Handles missing inner types.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ParenthesizedArgListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ParamListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​OffsetOfExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​NamedCrateImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ModuleImpl.qll Handles missing module names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MatchGuardImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MatchArmListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroRulesImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroPatImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroItemsImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroDefImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​MacroCallImpl.qll Handles missing macro paths.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​LifetimeParamImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​LifetimeArgImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​KeyValueMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ItemListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ItemImpl.qll Changes attribute-macro formatting dispatch.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ImplImpl.qll Handles missing self types.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FunctionImpl.qll Handles missing function names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ForTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FormatArgsExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FormatArgsArgNameImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FormatArgsArgImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FnPtrTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​FieldExprImpl.qll Handles missing field names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ExternItemListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ExternCrateImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ExternBlockImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ExprStmtImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​EnumImpl.qll Handles missing enum names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ElementImpl.qll Removes the root string fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​DynTraitTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ConstParamImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ConstImpl.qll Handles missing constant names.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ConstBlockPatImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ConstArgImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ClosureExprImpl.qll Handles missing closure bodies.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​CfgMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​CfgCompositeImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​CfgAttrMetaImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​CfgAtomImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AttrImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AssocTypeArgImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AssocItemListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmSymImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmRegSpecImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmRegOperandImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmOptionsListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmOptionImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmOperandNamedImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmOperandExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmLabelImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmExprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmDirSpecImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmConstImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AsmClobberAbiImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ArrayTypeReprImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ArrayExprInternalImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​ArgListImpl.qll Adds a class-name fallback.
rust/​ql/​lib/​codeql/​rust/​elements/​internal/​AbiImpl.qll Adds a class-name fallback.
rust/​ql/​.gitattributes Reclassifies customized wrappers.
rust/​ql/​.generated.list Removes customized wrappers from generation tracking.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rust/ql/lib/codeql/rust/elements/internal/ItemImpl.qll

@paldepind paldepind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR avoids the root def call by instead pushing those calls down to classes that are currently lacking a proper toString implementation, and to help verify my changes I also added a new consistency check for missing toString implementations, which revealed existing bugs that could all be handled by adding concat(...) wrappers to account for missing AST children.

Some of these I don't really understand. For instance, when will a closure expression not have a body? Is it for broken code that rust-analyzer parses anyway?

For some of the concat usages the strings will have trailing spaces when children are missing. For instance, an enum without a name will stringify as "enum ". However, if the cases are broken code, then I guess we don't care?

@hvitved

hvitved commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Some of these I don't really understand. For instance, when will a closure expression not have a body? Is it for broken code that rust-analyzer parses anyway?

I think it is mostly for things inside macro expansions, but not sure exactly.

For some of the concat usages the strings will have trailing spaces when children are missing. For instance, an enum without a name will stringify as "enum ". However, if the cases are broken code, then I guess we don't care?

Yes, that's what I think.

@hvitved
hvitved merged commit 2294b39 into main Sep 9, 2026
21 checks passed
@hvitved
hvitved deleted the hvitved-rust-tostring-implementations branch September 9, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants