Rust: Refactor type inference to use new TypeItem class
#21067
Merged
+91
−146
Conversation
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
paldepind
force-pushed
the
rust/type-inference-use-type-item
branch
from
bfc43d5 to
fce7247
Compare
paldepind
force-pushed
the
rust/type-inference-use-type-item
branch
from
fce7247 to
dde845e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the type inference implementation in Rust QL to use a new unified TypeItem class, consolidating the handling of structs, enums, and unions under a common abstraction.
- Introduces a new
DataTypeclass that replaces separateTStruct,TEnum, andTUniontype constructors with a singleTDataType(TypeItem)constructor - Replaces specific getter methods (
getStruct(),getEnum(),getUnion()) with a unifiedgetTypeItem()method across the codebase - Refactors
TupleLikeConstructorandDeclarationclasses to use the new abstraction, reducing code duplication
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/Type.qll | Introduces DataType class and makes StructType, EnumType, and UnionType subclasses; consolidates type constructors from TStruct, TEnum, TUnion to TDataType |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Updates all type references to use TDataType constructor; refactors TupleLikeConstructor and Declaration classes to use getTypeItem() method and eliminate duplicate logic |
| rust/ql/lib/codeql/rust/internal/TypeMention.qll | Simplifies resolveRootType() by consolidating three separate type resolution cases into a single TDataType case |
| rust/ql/lib/codeql/rust/security/Barriers.qll | Updates barrier classes to use getTypeItem() instead of getStruct() or getEnum() |
| rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll | Updates algorithm name extraction to use getTypeItem() instead of getStruct() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hvitved
requested changes
Dec 19, 2025
Contributor
hvitved
left a comment
hvitved
left a comment
There was a problem hiding this comment.
Nice refactor; some minor comments. I have also started a DCA run.
| EnumType() { this = TEnum(enum) } | ||
| /** A struct type. */ | ||
| class StructType extends DataType { | ||
| StructType() { super.getTypeItem() instanceof Struct } |
Contributor
There was a problem hiding this comment.
Suggested change
| StructType() { super.getTypeItem() instanceof Struct } | |
| private Struct struct; | |
| StructType() { struct = super.getTypeItem() } |
| /** Gets the enum that this enum type represents. */ | ||
| Enum getEnum() { result = enum } | ||
| /** Gets the struct that this struct type represents. */ | ||
| override Struct getTypeItem() { result = super.getTypeItem() } |
Contributor
There was a problem hiding this comment.
Suggested change
| override Struct getTypeItem() { result = super.getTypeItem() } | |
| override Struct getTypeItem() { result = struct } |
| } | ||
| /** An enum type. */ | ||
| class EnumType extends DataType { | ||
| EnumType() { super.getTypeItem() instanceof Enum } |
Contributor
There was a problem hiding this comment.
Suggested change
| EnumType() { super.getTypeItem() instanceof Enum } | |
| private Enum enum; | |
| EnumType() { enum = super.getTypeItem() } |
| result = enum.getGenericParamList().getTypeParam(i).getDefaultType() | ||
| } | ||
| /** Gets the enum that this enum type represents. */ | ||
| override Enum getTypeItem() { result = super.getTypeItem() } |
Contributor
There was a problem hiding this comment.
Suggested change
| override Enum getTypeItem() { result = super.getTypeItem() } | |
| override Enum getTypeItem() { result = enum } |
| override string toString() { result = enum.getName().getText() } | ||
| /** A union type. */ | ||
| class UnionType extends DataType { | ||
| UnionType() { super.getTypeItem() instanceof Union } |
Contributor
There was a problem hiding this comment.
Suggested change
| UnionType() { super.getTypeItem() instanceof Union } | |
| private Union union; | |
| UnionType() { union = super.getTypeItem() } |
|
|
||
| override Location getLocation() { result = enum.getLocation() } | ||
| /** Gets the union that this union type represents. */ | ||
| override Union getTypeItem() { result = super.getTypeItem() } |
Contributor
There was a problem hiding this comment.
Suggested change
| override Union getTypeItem() { result = super.getTypeItem() } | |
| override Union getTypeItem() { result = union } |
| result = this.getTypeParameter(_) and | ||
| path = TypePath::singleton(result) | ||
| or | ||
| // type of the struct itself |
| } | ||
| override TypeItem getTypeItem() { result = this } | ||
|
|
||
| override TupleField getTupleField(int i) { result = this.(Struct).getTupleField(i) } |
Contributor
There was a problem hiding this comment.
Suggested change
| override TupleField getTupleField(int i) { result = this.(Struct).getTupleField(i) } | |
| override TupleField getTupleField(int i) { result = Struct.super.getTupleField(i) } |
| path = TypePath::singleton(result) | ||
| ) | ||
| } | ||
| override TupleField getTupleField(int i) { result = this.(Variant).getTupleField(i) } |
Contributor
There was a problem hiding this comment.
Suggested change
| override TupleField getTupleField(int i) { result = this.(Variant).getTupleField(i) } | |
| override TupleField getTupleField(int i) { result = Variant.super.getTupleField(i) } |
hvitved
approved these changes
Dec 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Just a quick go at getting some mileage out of the new
TypeItemclass in the type inference implementation.