Rust: Include synthetic type parameters in Type.getATypeParameter#20274
Rust: Include synthetic type parameters in Type.getATypeParameter#20274hvitved merged 1 commit intogithub:mainfrom
Type.getATypeParameter#20274Conversation
hvitved
force-pushed
the
rust/type-synth-type-param
branch
from
008231b to
9ef839d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the type parameter handling in Rust to properly include synthetic type parameters (such as associated types in traits) in Type.getATypeParameter(). The changes address an issue where the certainTypeConflict check wasn't working correctly for synthetic type parameters.
- Separates positional type parameters from all type parameters by introducing
getPositionalTypeParameter() - Updates
getATypeParameter()to include both positional and synthetic type parameters - Adds specific handling for trait and dynamic trait types to include their associated types
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/Type.qll | Refactors type parameter methods and adds synthetic type parameter support for trait types |
| rust/ql/lib/codeql/rust/internal/TypeMention.qll | Updates call to use the new positional type parameter method |
| * This includes both positional type parameters and synthetic type parameters, | ||
| * such as associated types in traits. | ||
| */ | ||
| TypeParameter getATypeParameter() { result = this.getPositionalTypeParameter(_) } |
There was a problem hiding this comment.
The default implementation of getATypeParameter() only returns positional type parameters. This means that types like StructType, EnumType, etc. won't include their synthetic type parameters unless they override this method. Consider making this method abstract or providing a more comprehensive default implementation.
| TypeParameter getATypeParameter() { result = this.getPositionalTypeParameter(_) } | |
| abstract TypeParameter getATypeParameter(); |
paldepind
left a comment
paldepind
left a comment
There was a problem hiding this comment.
I think this is a great change!
The way I understand it, #19847 got rid of the annoying traitAliasIndex that assigned arbitrary indices to associated types for traits. But, it also meant that getTypeParameter on traits no longer gave the tp's for associated types.
With this PR we get the best of both worlds: A getATypeParameter that gives the full set of type parameters (without requiring one to invent positions where it doesn't make much sense) and a getPositionalTypeParameter that holds for the subset of the TPs that have a meaningful position.
2 participants
#20179 follow-up: The
not certainType.getATypeParameter() = tpcheck incertainTypeConflictdoes not work as intended for synthetic type parameters.DCA is uneventful.