-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Model builtin types in path resolution #20830
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 our terms of service and privacy 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?
Rust: Model builtin types in path resolution #20830
Conversation
d90e355 to
07e2215
Compare
07e2215 to
98a1b94
Compare
98a1b94 to
f73ba1f
Compare
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.
Pull Request Overview
This PR refactors the Rust type system to model builtin types (slices, arrays, references, pointers, and tuples) as normal struct types defined in types.rs, rather than as special structural types. This enables path resolution for expressions like <[_]>::foo and improves type inference for vec![...] macro invocations.
Key changes:
- Added struct definitions for builtin types (Slice, Array, Ref, Ptr, Tuple0-Tuple16) in the extractor's
types.rsfile - Refactored type classes to extend
StructTypeinstead of being special-cased structural types - Updated type parameter handling to use helper functions (
getTupleTypeParameter,getArrayTypeParameter, etc.)
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/tools/builtins/types.rs | Added struct definitions for builtin generic types (Slice, Array, Ref, Ptr) and tuples (Tuple0-Tuple16) |
| rust/ql/lib/codeql/rust/frameworks/stdlib/Builtins.qll | Added QL classes for new builtin types with helper methods like TupleType.getArity() |
| rust/ql/lib/codeql/rust/internal/Type.qll | Refactored TupleType, ArrayType, RefType, SliceType, and PtrType to extend StructType; removed separate type parameter classes; added helper functions for accessing type parameters |
| rust/ql/lib/codeql/rust/internal/TypeMention.qll | Updated type resolution to use new type parameter accessors and check types using instanceof |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Updated type inference logic to use new type representations and helper functions |
| rust/ql/lib/codeql/rust/internal/PathResolution.qll | Added resolveBuiltin() to handle path resolution for builtin types; filtered builtin types from regular name-based resolution |
| rust/ql/lib/codeql/rust/security/Barriers.qll | Added namespace qualification for builtin type references |
| rust/ql/lib/codeql/rust/elements/internal/StructImpl.qll | Added helper methods getAStructField() and getATupleField() |
| rust/ql/lib/codeql/rust/dataflow/internal/Content.qll | Excluded synthetic tuple type fields from TupleFieldContent |
| rust/ql/test/library-tests/type-inference/main.rs | Added test cases for builtin type method resolution |
| rust/ql/test/library-tests/type-inference/type-inference.ql | Updated test query structure for certainType vs type tagging |
| rust/ql/test/library-tests/elements/builtintypes/BuiltinTypes.ql | Removed unused import |
| shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Added inline TODO comment (no functional change) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| i = builtins.getASuccessor(name) and | ||
| not name = ["Slice", "Array", "Ref", "Ptr"] and | ||
| not name.matches("Tuple%") | ||
| ) |
Copilot
AI
Nov 14, 2025
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.
[nitpick] The filter excludes builtin types from the regular path resolution: not name = ["Array", "Slice", "Ref", "Ptr"] and not name.matches("Tuple%"). However, these exact names are still defined in types.rs and will be extracted.
This creates a subtle asymmetry - the structs exist in the codebase but are excluded from certain resolution paths. Consider adding a comment explaining why these specific builtin types are filtered out here, and how they are resolved through the alternative resolveBuiltin() mechanism instead.
| // TODO: Remove once we can handle the `impl<I: Iterator> IntoIterator for I` implementation | ||
| tp = getIteratorItemTypeParameter() and | ||
| inferType(fe.getIterable()) != TArrayType() | ||
| inferType(fe.getIterable()) != getArrayTypeParameter() |
Copilot
AI
Nov 14, 2025
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.
Incorrect comparison: getArrayTypeParameter() returns a TypeParamTypeParameter, but inferType() returns a Type. This comparison will always be false.
The original code compared against TArrayType() which was a Type. The fix should be to compare against an ArrayType instance instead:
inferType(fe.getIterable()) instanceof ArrayType| inferType(fe.getIterable()) != getArrayTypeParameter() | |
| not (inferType(fe.getIterable()) instanceof ArrayType) |
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.
NO!
f73ba1f to
bf0dc3c
Compare
Previously, we were not able to resolve paths like
<[_]>::foo, because there is noItemNodeto represent the[T]type. This means we cannot infer types forvec![...]macro invocations, which desugar into<[_]>::into_vec(alloc::boxed::box_new([..])).This PR synthesizes such nodes by adding entries to the
types.rsfile shipped with the extractor, meaning that we model builtin generic types like normal structs. The following types are modeled:[T](slices),[T;N](arrays),&T(references),*T(pointers), and(T1,...,Tn)(tuples, up to arity 16, surely Enough for Everyone...).In addition to enabling path resolution, this arguably simplifies the code as well.
Note that we are still not able to infer
vec![...]element types; while we are now able to resolve theinto_veccall, and hence that it has typeVec, we are not able to match the array type[...]against the slice type expected by into_vec.DCA looks good.