Rust: Unify handling of struct and tuple constructors#21488
Rust: Unify handling of struct and tuple constructors#21488hvitved merged 5 commits intogithub:mainfrom
Conversation
paldepind
force-pushed
the
rust/tuple-constructor-self
branch
from
3f392e8 to
b25b78b
Compare
paldepind
changed the title
paldepind
force-pushed
the
rust/tuple-constructor-self
branch
from
b25b78b to
97670b3
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Rust type inference to unify handling of struct-like and tuple-like struct/variant constructors and their corresponding patterns, reducing duplicated matching/configuration logic.
Changes:
- Unifies constructor type inference for tuple-struct calls, struct expressions, and nullary path expressions under a single matching configuration.
- Unifies pattern type inference for struct patterns and tuple-struct patterns under a single matching configuration.
- Adds
getNthStructField(int i)helpers onStruct,Variant,StructExpr, andStructPatwrappers to support index-based field access.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Consolidates constructor and constructor-pattern inference paths into shared matching modules. |
| rust/ql/lib/codeql/rust/elements/internal/VariantImpl.qll | Adds getNthStructField helper to support unified constructor handling. |
| rust/ql/lib/codeql/rust/elements/internal/StructImpl.qll | Adds getNthStructField helper to support unified constructor handling. |
| rust/ql/lib/codeql/rust/elements/internal/StructPatImpl.qll | Adds getNthStructField helper for resolved struct/variant fields in patterns. |
| rust/ql/lib/codeql/rust/elements/internal/StructExprImpl.qll | Adds getNthStructField helper for instantiated struct/variant fields in expressions. |
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll
Outdated
Show resolved
Hide resolved
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
hvitved
left a comment
hvitved
left a comment
There was a problem hiding this comment.
Nice refactor, do you know why DCA shows some changes to e.g. Missing call targets?
| result = this.getReturnType(path) | ||
| or | ||
| pos.isTypeQualifier() and | ||
| result = this.getReturnType(path) |
There was a problem hiding this comment.
I checked, and you are right that this is not needed since in constructs like Option::<i32>::None we treat the i32 as an explicit type argument.
| /** | ||
| * A matching configuration for resolving types of tuple-like variants and tuple | ||
| * structs such as `Result::Ok(42)`. | ||
| * A matching configuration for resolving types of constructors of enums and |
There was a problem hiding this comment.
I think constructions would be more accurate, and for this reason I think the module should be named ConstructionMatchingInput (just like we have FunctionCallMatchingInput and not FunctionMatchingInput).
There was a problem hiding this comment.
Yes, that's better!
I took at look and it turns out that a few projects had struct expressions with more fields than the any function had parameters. This caused |
Removes some code duplication by unifying handling of tuple-like and struct-like structs and variants in type inference.