- Notifications
You must be signed in to change notification settings - Fork1.9k
Rust: Refactor type inference to use newTypeItem class#21067
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
bfc43d5 tofce7247Comparefce7247 todde845eCompareThere was a problem hiding this 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 refactors the type inference implementation in Rust QL to use a new unifiedTypeItem 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 | IntroducesDataType class and makesStructType,EnumType, andUnionType subclasses; consolidates type constructors fromTStruct,TEnum,TUnion toTDataType |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Updates all type references to useTDataType constructor; refactorsTupleLikeConstructor andDeclaration classes to usegetTypeItem() method and eliminate duplicate logic |
| rust/ql/lib/codeql/rust/internal/TypeMention.qll | SimplifiesresolveRootType() by consolidating three separate type resolution cases into a singleTDataType case |
| rust/ql/lib/codeql/rust/security/Barriers.qll | Updates barrier classes to usegetTypeItem() instead ofgetStruct() orgetEnum() |
| rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll | Updates algorithm name extraction to usegetTypeItem() instead ofgetStruct() |
💡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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice refactor; some minor comments. I have also started a DCA run.
| EnumType(){this=TEnum(enum)} | ||
| /** A struct type. */ | ||
| classStructTypeextendsDataType{ | ||
| StructType(){super.getTypeItem()instanceofStruct} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| StructType(){super.getTypeItem()instanceofStruct} | |
| privateStructstruct; | |
| StructType(){struct=super.getTypeItem()} |
| /** Gets the enum that this enum type represents. */ | ||
| EnumgetEnum(){result=enum} | ||
| /** Gets the struct that this struct type represents. */ | ||
| overrideStructgetTypeItem(){result=super.getTypeItem()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| overrideStructgetTypeItem(){result=super.getTypeItem()} | |
| overrideStructgetTypeItem(){result=struct} |
| } | ||
| /** An enum type. */ | ||
| classEnumTypeextendsDataType{ | ||
| EnumType(){super.getTypeItem()instanceofEnum} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| EnumType(){super.getTypeItem()instanceofEnum} | |
| privateEnumenum; | |
| EnumType(){enum=super.getTypeItem()} |
| result=enum.getGenericParamList().getTypeParam(i).getDefaultType() | ||
| } | ||
| /** Gets the enum that this enum type represents. */ | ||
| overrideEnumgetTypeItem(){result=super.getTypeItem()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| overrideEnumgetTypeItem(){result=super.getTypeItem()} | |
| overrideEnumgetTypeItem(){result=enum} |
| overridestringtoString(){result=enum.getName().getText()} | ||
| /** A union type. */ | ||
| classUnionTypeextendsDataType{ | ||
| UnionType(){super.getTypeItem()instanceofUnion} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| UnionType(){super.getTypeItem()instanceofUnion} | |
| privateUnionunion; | |
| UnionType(){union=super.getTypeItem()} |
| overrideLocationgetLocation(){result=enum.getLocation()} | ||
| /** Gets the union that this union type represents. */ | ||
| overrideUniongetTypeItem(){result=super.getTypeItem()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| overrideUniongetTypeItem(){result=super.getTypeItem()} | |
| overrideUniongetTypeItem(){result=union} |
| result=this.getTypeParameter(_)and | ||
| path= TypePath::singleton(result) | ||
| or | ||
| // type of the struct itself |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Needs to be updated
| } | ||
| overrideTypeItemgetTypeItem(){result=this} | ||
| overrideTupleFieldgetTupleField(inti){result=this.(Struct).getTupleField(i)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| overrideTupleFieldgetTupleField(inti){result=this.(Struct).getTupleField(i)} | |
| overrideTupleFieldgetTupleField(inti){result=Struct.super.getTupleField(i)} |
| path= TypePath::singleton(result) | ||
| ) | ||
| } | ||
| overrideTupleFieldgetTupleField(inti){result=this.(Variant).getTupleField(i)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| overrideTupleFieldgetTupleField(inti){result=this.(Variant).getTupleField(i)} | |
| overrideTupleFieldgetTupleField(inti){result=Variant.super.getTupleField(i)} |
Uh oh!
There was an error while loading.Please reload this page.
Just a quick go at getting some mileage out of the new
TypeItemclass in the type inference implementation.