Skip to content

Rust: Improve handling of where clauses in type inference and path resolution #20177

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

Merged
merged 4 commits into from
Aug 6, 2025

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Aug 6, 2025

A rebased version of #20140.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Aug 6, 2025
@hvitved hvitved marked this pull request as ready for review August 6, 2025 11:11
@hvitved hvitved requested a review from a team as a code owner August 6, 2025 11:11
@Copilot Copilot AI review requested due to automatic review settings August 6, 2025 11:11
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Aug 6, 2025
@hvitved hvitved requested a review from geoffw0 August 6, 2025 11:11
Copy link
Contributor

@Copilot 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 improves the handling of where clauses in Rust type inference and path resolution. It enhances CodeQL's ability to understand type bounds defined in where clauses, particularly for type parameters and traits, improving the accuracy of type inference and path resolution.

  • Adds support for where clauses on type parameters and traits
  • Improves type bound resolution in both direct and where clause contexts
  • Updates test cases to cover more complex where clause scenarios

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rust/ql/test/library-tests/type-inference/main.rs Adds comprehensive test cases for where clause scenarios including trait bounds
rust/ql/test/library-tests/path-resolution/main.rs Adds test for where clause with Self and type parameters
rust/ql/lib/codeql/rust/elements/internal/TypeParamImpl.qll Implements getATypeBound() to collect bounds from direct and where clauses
rust/ql/lib/codeql/rust/elements/internal/TraitImpl.qll Implements getATypeBound() for traits with where clause support
rust/ql/lib/codeql/rust/internal/TypeInference.qll Updates to use new getATypeBound() methods
rust/ql/lib/codeql/rust/internal/PathResolution.qll Simplifies bound path resolution using new methods

exists(WherePred wp |
wp = this.(TypeParamItemNode).getAWherePred() and
tbl = wp.getTypeBoundList() and
wp = any(WhereClause wc).getPredicate(i)
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The logic in the second disjunct doesn't guarantee that the WherePred wp from getAWherePred() is the same as the one retrieved from getPredicate(i). This could lead to incorrect indexing. Consider explicitly linking the where predicate to its index in the where clause.

Suggested change
wp = any(WhereClause wc).getPredicate(i)
exists(WhereClause wc, WherePred wp |
wp = wc.getPredicate(i) and
wp = this.(TypeParamItemNode).getAWherePred() and
tbl = wp.getTypeBoundList()

Copilot uses AI. Check for mistakes.

or
exists(WherePred wp |
wp = this.getWhereClause().getAPredicate() and
wp.getTypeRepr().(PathTypeRepr).getPath().getText() = "Self" and
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The string comparison getText() = \"Self\" is fragile and may not handle all cases where Self is referenced. Consider using a more robust type-based comparison or path resolution to identify Self references.

Suggested change
wp.getTypeRepr().(PathTypeRepr).getPath().getText() = "Self" and
isSelfPath(wp.getTypeRepr().(PathTypeRepr).getPath()) and

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.

The analysis time regression seems to have mostly gone away with the rebase (which was expected). 👍

@hvitved hvitved merged commit ed3a33f into github:main Aug 6, 2025
21 of 22 checks passed
@hvitved hvitved deleted the rust/type-inference-where branch August 6, 2025 13:52
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