-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Implement support for associated types accessed on type parameters #21273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| /** Holds if `path` is of the form `<type as trait>::name` */ | ||
| predicate asTraitPath(Path path, TypeRepr typeRepr, Path traitPath, string name) { |
Check warning
Code scanning / CodeQL
Predicates starting with "get" or "as" should return a value Warning
77d9d0a to
210633d
Compare
210633d to
a033057
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR extends the Rust type inference modeling to support associated types accessed on type parameters (e.g. T::Output), by lowering these associated-type accesses into synthetic type parameters that existing inference machinery can already handle.
Changes:
- Introduces a new associated-types helper module and a new type-parameter kind representing
T::Assoc-style accesses on type parameters. - Updates type mention / type inference logic to resolve and propagate these lowered associated-type parameters.
- Updates and extends the type-inference library tests and golden expectations to reflect improved inference results.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/typeinference/AssociatedTypes.qll | New helper module for associated type concepts and predicates used by inference. |
| rust/ql/lib/codeql/rust/internal/typeinference/Type.qll | Adds a new synthetic type-parameter representation for associated types accessed on type parameters; refactors to use the new helper module. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll | Extends path/type mention resolution to map T::Assoc-style accesses to the new synthetic type parameters. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Integrates the new synthetic type parameters into type-parameter identity/position handling. |
| rust/ql/test/library-tests/type-inference/associated_types.rs | Adds/adjusts tests exercising associated types accessed via impl type parameters; updates inline expectations. |
| rust/ql/test/library-tests/type-inference/main.rs | Updates inline expectations where associated-type-on-type-param inference now succeeds. |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates golden results to match the expanded inference behavior. |
| * mention `T::Assoc` resolves to this type parameter. If denoting the type | ||
| * parameter by `T_Assoc` then the above function is treated as if it was | ||
| * ```rust | ||
| * fn foo<T: SomeTrait<Assoc = T_Assoc>, T_Assoc>(arg: T_Assoc) { } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this doc example, T_Assoc is referenced in the bound for T before T_Assoc is declared in the generic parameter list, which is not valid Rust and can be misleading. Consider rewriting the example by either declaring T_Assoc before T or moving the T: SomeTrait<Assoc = T_Assoc> constraint into a where clause.
| * fn foo<T: SomeTrait<Assoc = T_Assoc>, T_Assoc>(arg: T_Assoc) { } | |
| * fn foo<T, T_Assoc>(arg: T_Assoc) | |
| * where | |
| * T: SomeTrait<Assoc = T_Assoc>, | |
| * { } |
| // be instantiated into the bound, as explained in the comment for | ||
| // `TypeParamAssociatedTypeTypeParameter`. | ||
| // ```rust | ||
| // fn foo<T: SomeTrait<Assoc = T_Assoc>, T_Assoc>(arg: T_Assoc) { } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Rust example in this comment uses T_Assoc in the bound for T before T_Assoc is declared in the generics list (fn foo<T: ..., T_Assoc>(...)). This ordering isn’t valid Rust, so the example should be adjusted (e.g., declare T_Assoc first, or move the bound into a where clause) to avoid confusion.
| // fn foo<T: SomeTrait<Assoc = T_Assoc>, T_Assoc>(arg: T_Assoc) { } | |
| // fn foo<T_Assoc, T: SomeTrait<Assoc = T_Assoc>>(arg: T_Assoc) { } |
| /** | ||
| * Holds if `assoc` is accessed on `tp` in `path`. | ||
| * | ||
| * That is this is the case when `path` is of the form `<tp as |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar in this docstring: “That is this is the case …” reads as missing punctuation. Consider changing to “That is, this is the case …” for clarity.
| * That is this is the case when `path` is of the form `<tp as | |
| * That is, this is the case when `path` is of the form `<tp as |
| let _o1 = tp_with_as(S); // $ target=tp_with_as type=_o1:S3 | ||
| let _o2 = tp_without_as(S); // $ target=tp_without_as type=_o2:S3 | ||
| let ( | ||
| _o3, // $ MISSING: type=_o3:S3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we still can't infer this type where the associated type, Output, is from a supertrait of the bound, AnotherGet. That will require some additional effort.
This PR adds type inference support for signatures where an associated type is accessed on a type paramere, such as in:
The idea behind the implementation is to turn
T::Contentinto a type parameter. This is explained in the comment forTypeParamAssociatedTypeTypeParameterwhich is probably a good place to start the review.The result is that associated types on type parameters are "lowered" into things that we already handle in type inference. For instance, a bound like
T::Output: SomeTraitis just a bound on a type parameter, which we already handle.The DCA results show that we infer more types and resolve more calls.
There's an increase in path resolution inconsistencies which is the "Multiple resolved targets" check, that is actually about type inference. I've looked at some of the new inconsistencies, and they don't seem to be caused by any flaws in this PR. I've rebased the PR on
mainand started a second DCA run as I suspect some of the recent fixes might actually resolve some of these inconsistencies.