Rust: Type inference for `for` loops and array expressions #19754

geoffw0 · 2025-06-13T12:11:49Z

Adds type inference for for loops and array expressions. Currently for for loops this is limited to loops iterating through arrays.

@hvitved please advise.

I think for the more general cases we will need to decode the Iterator type and perhaps understand IntoIterator.
I'm not sure whether I should be attempting to model implicit dereferences, or perhaps a more general solution for that problem might be in the works?

geoffw0 · 2025-06-17T07:42:49Z

@hvitved do you think this is heading in the right direction?

hvitved

Great start!

hvitved · 2025-06-17T08:49:19Z

rust/ql/lib/codeql/rust/internal/TypeInference.qll

@@ -1124,6 +1124,37 @@ private Type inferIndexExprType(IndexExpr ie, TypePath path) {
  )
 }

+pragma[nomagic]
+private Type inferArrayExprType(ArrayExpr ae, TypePath path) {


I think this should be implemented inside the typeEquality predicate instead.

You also need a

/** Gets the root type of the array expression `ae`. */ pragma[nomagic] private Type inferArrayExprType(ArrayExpr ae) { exists(ae) and result = TArrayType() }

predicate, similar to inferRefExprType.

I see. Done. Got a couple of extra expected results.

hvitved · 2025-06-17T08:50:45Z

rust/ql/lib/codeql/rust/internal/TypeInference.qll

+    (
+      iterablePath.isCons(any(ArrayTypeParameter tp), path) and
+      result = iterableType
+      // TODO: iterables (containers, ranges etc)


We may as well pull out the logic from inferIndexExprType that handles arrays, slices, and Vecs, and reuse it here.

Also done, but it doesn't catch any additional test cases. I think at some point we're going to need to cover the general case for iterables.

The reason why we are not seeing results for Vec is that we cannot yet infer the type of e.g. vec![1, 2, 3], which desugars to

<[_]>::into_vec( #[rustc_box] alloc::boxed::Box::new([1, 2, 3]), )

and we cannot currently resolve the into_vec call.

Ah, then I'll add a simpler test case that we will hopefully cover.

If you add an explicit : Vec<i32> type annotation it ought to work.

I've added some variations with Vec and we get ... none of them. There's probably something very narrow I could write that we would get a Vec type for, but there's clearly not enough here to work on most normal code involving Vecs.

... I read your comment again and added a case with an explicit type annotation. That does work, though it feels narrow as people usually don't seem to use explicit types much when they don't need to.

hvitved · 2025-06-18T17:35:27Z

rust/ql/test/library-tests/type-inference/main.rs

+
+        // for loops with containers
+
+        let vals3 = vec![1, 2, 3]; // $ MISSING: type=vals3:Vec<i32>


This is not how to make expected type annotations for constructed types, instead you should use the format type=<element>:<path>.<type> (see

codeql/rust/ql/test/library-tests/type-inference/type-inference.ql

Line 60 in 2e5d52e

else value = element + ":" + path.toString() + "." + t.toString()

). So in this case it should instead by type=vals3:T.i32.

Ah, I think I was relying on the test runner to start suggesting correct syntax, which it doesn't really do for optional results. I've updated the expectations now as best I can - there may still be add mistakes.

geoffw0 · 2025-06-19T13:20:25Z

Ready for review: there are gaps (and apparently a conflict with main), but we get some useful results and I'd like to push towards merging this ASAP.

paldepind

Looks really great 👍. I've left a few minor comments.

paldepind · 2025-06-19T19:22:21Z

rust/ql/lib/codeql/rust/internal/TypeInference.qll

+      exists(TypePath path0 |
+        iterablePath.isCons(any(RefTypeParameter tp), path0) and
+        path0.isCons(any(SliceTypeParameter tp), path)
+      )


This makes the last case look more like the prior cases:

Suggested change

exists(TypePath path0 |

iterablePath.isCons(any(RefTypeParameter tp), path0) and

path0.isCons(any(SliceTypeParameter tp), path)

)

iterablePath

.stripPrefix(TypePath::cons(TRefTypeParameter(),

TypePath::singleton(any(SliceTypeParameter tp)))) = path

paldepind · 2025-06-19T19:24:56Z

rust/ql/test/library-tests/type-inference/main.rs

@@ -1881,6 +1881,109 @@ mod method_determined_by_argument_type {
    }
 }

+mod loops {
+    struct MyCallable {
+    }


The type inference tests are currently formatted with rustfmt and I think it would be nice to keep doing that.

paldepind · 2025-06-19T19:27:28Z

rust/ql/test/library-tests/type-inference/main.rs

+        map1.insert(1, Box::new("one")); // $ method=insert
+        map1.insert(2, Box::new("two")); // $ method=insert
+        for key in map1.keys() { } // $ method=keys MISSING: type=key:i32
+        for value in map1.values() { } // $ method=values MISSING: type=value:Box type=value:Box.T:&T.str


This type annotation has two :s. I think the later should be a . or tweaked in some other way.

geoffw0 added 4 commits June 13, 2025 12:37

Rust: Add test cases for type inference in loops.

840ef5c

Rust: Implement type inference for 'for' loops on arrays.

f76b562

Rust: Implement type inference for ArrayListExprs.

51343a5

Rust: Implement type inference for ArrayRepeatExprs.

b89d6d3

geoffw0 added the Rust Pull requests that update Rust code label Jun 13, 2025

geoffw0 added 2 commits June 13, 2025 13:45

Merge branch 'main' into typeinfer

62e3cc5

Rust: Accept consistency failures (for now).

6194676

hvitved requested changes Jun 17, 2025

View reviewed changes

geoffw0 added 4 commits June 17, 2025 10:45

Rust: Move inferArrayExprType logic into typeEquality predicate.

69da4e7

Rust: If we're inferring both ways, it should really be to any element.

66d6770

Rust: Add logic for Vecs and slices.

4292b03

Rust: Add some more test cases for type inference on Vecs.

dec0deb

hvitved reviewed Jun 18, 2025

View reviewed changes

geoffw0 added 6 commits June 19, 2025 11:15

Merge branch 'main' into typeinfer

639f85a

Rust: Add inferArrayExprType.

1622d08

Rust: Add a Vec test case that we actually get (explicit type).

f670fcb

Rust: Update test expectations format (type=...).

7170e97

Rust: Add another test case for ranges.

d55e8b7

Rust: Accept path resolution consistency changes.

26e7b2d

geoffw0 marked this pull request as ready for review June 19, 2025 13:19

geoffw0 requested a review from a team as a code owner June 19, 2025 13:19

Merge branch 'main' into typeinfer

7a25596

paldepind requested changes Jun 19, 2025

View reviewed changes


		// for loops with containers

		let vals3 = vec![1, 2, 3]; // $ MISSING: type=vals3:Vec<i32>

Rust: Type inference for for loops and array expressions #19754

Are you sure you want to change the base?

Rust: Type inference for for loops and array expressions #19754

Conversation

geoffw0 commented Jun 13, 2025

Uh oh!

geoffw0 commented Jun 17, 2025

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 commented Jun 19, 2025

Uh oh!

paldepind left a comment

Choose a reason for hiding this comment

Uh oh!

paldepind Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Rust: Type inference for `for` loops and array expressions #19754

Rust: Type inference for `for` loops and array expressions #19754

paldepind Jun 19, 2025 •

edited

Loading