Skip to content

Conversation

@paldepind
Copy link
Contributor

@paldepind paldepind commented Feb 5, 2026

This PR adds type inference support for signatures where an associated type is accessed on a type paramere, such as in:

fn get_content<T: Trait>(item: &T) -> T::Content { ... }

The idea behind the implementation is to turn T::Content into a type parameter. This is explained in the comment for TypeParamAssociatedTypeTypeParameter which 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: SomeTrait is 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 main and started a second DCA run as I suspect some of the recent fixes might actually resolve some of these inconsistencies.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Feb 5, 2026
}

/** 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

This predicate starts with 'as' but does not return a value.
@paldepind paldepind force-pushed the rust/tp-assoc branch 2 times, most recently from 77d9d0a to 210633d Compare February 9, 2026 13:57
@paldepind paldepind marked this pull request as ready for review February 10, 2026 09:05
@paldepind paldepind requested a review from a team as a code owner February 10, 2026 09:05
Copilot AI review requested due to automatic review settings February 10, 2026 09:05
Copy link
Contributor

Copilot AI left a 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) { }
Copy link

Copilot AI Feb 10, 2026

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.

Suggested change
* 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>,
* { }

Copilot uses AI. Check for mistakes.
// 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) { }
Copy link

Copilot AI Feb 10, 2026

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.

Suggested change
// fn foo<T: SomeTrait<Assoc = T_Assoc>, T_Assoc>(arg: T_Assoc) { }
// fn foo<T_Assoc, T: SomeTrait<Assoc = T_Assoc>>(arg: T_Assoc) { }

Copilot uses AI. Check for mistakes.
/**
* Holds if `assoc` is accessed on `tp` in `path`.
*
* That is this is the case when `path` is of the form `<tp as
Copy link

Copilot AI Feb 10, 2026

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.

Suggested change
* 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

Copilot uses AI. Check for mistakes.
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
Copy link
Contributor Author

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.

@paldepind paldepind added the no-change-note-required This PR does not need a change note label Feb 10, 2026
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.

1 participant