Rust: Refactor type inference to use new TypeItem class#21067
Rust: Refactor type inference to use new TypeItem class#21067paldepind merged 2 commits intogithub:mainfrom
TypeItem class#21067Conversation
bfc43d5 to
fce7247
Compare
fce7247 to
dde845e
Compare
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
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 } |
There was a problem hiding this comment.
| 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() } |
There was a problem hiding this comment.
| override Struct getTypeItem() { result = super.getTypeItem() } | |
| override Struct getTypeItem() { result = struct } |
| } | ||
| /** An enum type. */ | ||
| class EnumType extends DataType { | ||
| EnumType() { super.getTypeItem() instanceof Enum } |
There was a problem hiding this comment.
| 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() } |
There was a problem hiding this comment.
| 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 } |
There was a problem hiding this comment.
| 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() } |
There was a problem hiding this comment.
| 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) } |
There was a problem hiding this comment.
| 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) } |
There was a problem hiding this comment.
| override TupleField getTupleField(int i) { result = this.(Variant).getTupleField(i) } | |
| override TupleField getTupleField(int i) { result = Variant.super.getTupleField(i) } |
Just a quick go at getting some mileage out of the new
TypeItemclass in the type inference implementation.