Rust: Adjustments to type inference #19081
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses adjustments to type inference by updating generic parameter names for improved consistency.
- Renamed the generic parameter in struct MyThing from A to T.
- Renamed the generic parameters in traits MyTrait1, MyTrait2, and MyTrait3 to Tr1, Tr2, and Tr3 respectively.
Files not reviewed (8)
- rust/ql/lib/codeql/rust/elements/internal/FieldExprImpl.qll: Language not supported
- rust/ql/lib/codeql/rust/elements/internal/StructImpl.qll: Language not supported
- rust/ql/lib/codeql/rust/elements/internal/VariantImpl.qll: Language not supported
- rust/ql/lib/codeql/rust/internal/Type.qll: Language not supported
- rust/ql/lib/codeql/rust/internal/TypeInference.qll: Language not supported
- rust/ql/test/library-tests/type-inference/type-inference.expected: Language not supported
- rust/ql/test/library-tests/type-inference/type-inference.ql: Language not supported
- shared/typeinference/codeql/typeinference/internal/TypeInference.qll: Language not supported
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
| @@ -387,12 +387,12 @@ mod method_supertraits { | |||
| #[derive(Debug)] | |||
| struct S2; | |||
|
|
|||
| trait MyTrait1<A> { | |||
| fn m1(self) -> A; | |||
| trait MyTrait1<Tr1> { | |||
There was a problem hiding this comment.
The expected file contained some duplicated lines, making it a bit hard to se what was what. So this renames some generics s.t. fewer names are identical.
| * the type parameter `tp` in `target`. | ||
| * the type parameter `tp` in `target`, if any. | ||
| * | ||
| * Note, that this predicate crucially does not depend on type inference, |
| * where the method call is an access and `new Sub<int>()` is an access | ||
| * position , which is the receiver of a method call, we have: |
There was a problem hiding this comment.
new Sub<int>() in not an access position, but it is at an access position.
| @@ -605,19 +614,21 @@ module Make1<LocationSig Location, InputSig1<Location> Input1> { | |||
| * | |||
| * class Sub<T4> : Mid<C<T4>> { } | |||
| * | |||
| * new Sub<int>().ToString(); | |||
| * new Sub<int>().ToString(); | |||
| * // ^^^^^^^^^^^^^^ `apos` | |||
There was a problem hiding this comment.
Not quite right, see comment below.
| @@ -635,28 +646,36 @@ module Make1<LocationSig Location, InputSig1<Location> Input1> { | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
| * Holds if the type of `a` at `apos` has the base type `base`, and when | |||
| * viewed as an element of that type has at `path` the type `t`. | |||
There was a problem hiding this comment.
has at path the type t -> has the type t at path.
There was a problem hiding this comment.
Yeah, let's change it. I was trying to phrase this to match the order of the parameters, but it is clumpsy.
| */ | ||
| pragma[nomagic] | ||
| private predicate baseTypeMatch( | ||
| Access a, Declaration target, TypePath path, Type t, TypeParameter tp | ||
| ) { | ||
| not exists(getTypeArgument(a, target, tp, _)) and | ||
| target = a.getTarget() and |
There was a problem hiding this comment.
This may or may not introduce a bad join, let's see what DCA says.
This PR attempts to address most of @aschackmull's comments over in #18632, plus a few additional tweaks to the type inference implementation.