Rust: Small refactor in TypeMention.qll#21422
Open
hvitved wants to merge 1 commit intogithub:mainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up refactor to make the trait-specificity check in getPathConcreteAssocTypeAt clearer when resolving associated type paths in Rust type inference (per #21420).
Changes:
- Refactors
pathConcreteTypeAssocTypeto carry a singleimplOrTmTraitnode that represents either the trait mention (for<Type as Trait>::AssocType) or the enclosingimpl(forSelf::AssocType). - Reworks the guard logic in
getPathConcreteAssocTypeAtto branch based on whetherimplOrTmTraitis anImpl. - Minor comment formatting cleanup in
AssociatedType.qll.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll | Refactors how the “trait mention vs impl context” is represented and used for associated-type resolution. |
| rust/ql/lib/codeql/rust/internal/typeinference/AssociatedType.qll | Reflows an existing doc comment for readability. |
Comment on lines
+703
to
+705
| * `implOrTmTrait` is either the mention that resolves to `trait` when `path` | ||
| * is of the form `<Type as Trait>::AssocType`, or the enclosing `impl` block | ||
| * when `path` is of the form `Self::AssocType`. |
There was a problem hiding this comment.
The doc comment describes implOrTmTrait as a “mention”, but the parameter is typed as AstNode and is used as either an Impl node or the trait PathTypeRepr from <Type as Trait>::.... Consider rewording to explicitly mention the concrete AST node kinds (e.g., Impl vs trait PathTypeRepr) to avoid confusion for readers.
Suggested change
| * `implOrTmTrait` is either the mention that resolves to `trait` when `path` | |
| * is of the form `<Type as Trait>::AssocType`, or the enclosing `impl` block | |
| * when `path` is of the form `Self::AssocType`. | |
| * `implOrTmTrait` is the AST node corresponding to the trait in the context of | |
| * the access: it is either the trait `PathTypeRepr` from a path of the form | |
| * `<Type as Trait>::AssocType`, or the enclosing `Impl` node when `path` is of | |
| * the form `Self::AssocType`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#21420 follow-up; should hopefully make the check in
getPathConcreteAssocTypeAta bit more clear.