Rust: Distinguish `&mut T` from `&T` in type inference #20973

hvitved · 2025-12-05T13:33:48Z

Does what the PR title says, and additionally makes sure to prioritize shared borrows over mutable borrows when constructing candidate receiver types.

DCA is excellent; as expected, we see a large decrease in resolution inconsistencies, because we no longer have resolution overlap for methods with implementations for both shared and mutable borrows.

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

-      string derefChain, boolean borrow, TypePath path
-    ) {
-      result = substituteLookupTraits(this.getACandidateReceiverTypeAt(derefChain, borrow, path))
+    Type getANonPseudoCandidateReceiverTypeAt(string derefChain, BorrowKind borrow, TypePath path) {


Copilot

Pull request overview

This PR improves Rust type inference by distinguishing between mutable references (&mut T) and shared references (&T), which were previously treated as a single RefType. The change significantly reduces path resolution inconsistencies by eliminating ambiguity when methods are implemented for both borrow types.

Key Changes:

Split RefType into RefSharedType and RefMutType in the type system
Updated method resolution to prioritize shared borrows over mutable borrows when constructing candidate receiver types
Enhanced type inference to track borrow mutability through patterns, reference expressions, and method calls

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rust/ql/lib/codeql/rust/internal/Type.qll	Refactored `RefType` into an abstract base class with `RefSharedType` and `RefMutType` subclasses
rust/ql/lib/codeql/rust/frameworks/stdlib/Builtins.qll	Updated builtin type definitions to distinguish shared and mutable reference types
rust/ql/lib/codeql/rust/internal/TypeInference.qll	Enhanced method resolution with `BorrowKind` enum to track shared/mutable borrows, prioritizing shared borrows in candidate generation
rust/ql/lib/codeql/rust/internal/TypeMention.qll	Updated type mention resolution to correctly handle mutable vs shared references
rust/ql/lib/codeql/rust/internal/PathResolution.qll	Updated builtin type resolution to distinguish reference types based on mutability
rust/ql/lib/codeql/rust/elements/internal/OperationImpl.qll	Added TODO comment for handling `DerefMut` trait
rust/ql/test/*/.expected	Updated test expectations reflecting reduced path resolution inconsistencies
rust/ql/test/**/main.rs, dereference.rs, pattern_matching.rs	Updated test annotations to reflect correct `TRefMut` types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T16:46:24Z

rust/ql/lib/codeql/rust/elements/internal/OperationImpl.qll

+      // todo: handle `core::ops::deref::DerefMut`
      op = "*" and path = "core::ops::deref::Deref" and method = "deref" and borrows = 1


The TODO comment indicates that core::ops::deref::DerefMut is not yet handled. Since this PR introduces distinction between mutable and shared borrows, the dereference operator for mutable references should also be handled to properly support the DerefMut trait, which is used when dereferencing &mut T types. Without this, type inference for operations on mutable references may be incomplete.

Suggested change

// todo: handle `core::ops::deref::DerefMut`

op = "*" and path = "core::ops::deref::Deref" and method = "deref" and borrows = 1

op = "*" and path = "core::ops::deref::Deref" and method = "deref" and borrows = 1

or

op = "*" and path = "core::ops::deref::DerefMut" and method = "deref_mut" and borrows = 1

paldepind

Looks really great! I only have a bunch of nitpicky comments.

paldepind · 2025-12-18T09:08:05Z

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

    // forex using recursion
    pragma[nomagic]
-    private predicate hasNoCompatibleNonBlanketTargetBorrowToIndex(
+    private predicate hasNoCompatibleTargetMutBorrowToIndex(


The code repetition here is a bit unfortunate, but I guess it's not that easy to get rid of.

Yeah, I don't think we can easily get rid of the duplication.

paldepind · 2025-12-18T09:23:17Z

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

-        pos.isSelf() and borrows >= 1
+        pos.isSelf() and
+        borrows >= 1 and
+        if this instanceof AssignmentOperation


Could this be

Suggested change

if this instanceof AssignmentOperation

if this instanceof CompoundAssignmentExpr

?

I understand why we want this for stuff like a += 1 which is equivalent to (&mut a).add_assign(1). But do we need it for normal assignments that (in some cases) are handled by SSA?

CompoundAssignmentExpr will do the same here because of the .isOverloaded restriction, but happy to change.

paldepind · 2025-12-18T09:33:26Z

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


  /** Holds if `n` is implicitly borrowed. */
  cached
  predicate implicitBorrow(AstNode n) {


I wonder if it might make sense to expose mutability here as well.

Suggested change

predicate implicitBorrow(AstNode n) {

predicate implicitBorrow(AstNode n, boolean mutable) {

That might be useful elsewhere. For instance in mutablyBorrows in SsaImpl.qll this

e = any(MethodCallExpr mc).getReceiver() and e.(VariableAccess).getVariable() = v and v.isMutable()

is a rough approximation that could (in the future) benefit from using this predicate instead.

paldepind · 2025-12-18T09:53:27Z

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

+abstract class RefType extends StructType { }
+
+pragma[nomagic]
+TypeParamTypeParameter getRefTypeParameter() {


Instead of having getRefTypeParameter, getRefMutTypeParameter, and getRefSharedTypeParameter we could have a single getRefTypeParameter(boolean mutable):

pragma[nomagic] RefType getRefType(boolean mutable) { mutable = true and result = any(RefMutType t) or mutable = false and result = any(RefSharedType t) } pragma[nomagic] TypeParamTypeParameter getRefTypeParameter(boolean mutable) { result = getRefType(mutable).getPositionalTypeParameter(0) }

paldepind · 2025-12-18T09:54:51Z

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

+    or
+    ref = any(RefPat rp | if rp.isMut() then isMut = true else isMut = false)
+  |
+    if isMut = true then result instanceof RefMutType else result instanceof RefSharedType


Using the getRefType suggested above.

Suggested change

if isMut = true then result instanceof RefMutType else result instanceof RefSharedType

result = getRefType(isMut)

paldepind · 2025-12-18T10:22:52Z

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

+      exists(BorrowKind borrow, RefType rt |
+        this.implicitBorrowAt(pos, borrow) and
+        rt = borrow.getRefType()


Suggested change

exists(BorrowKind borrow, RefType rt |

this.implicitBorrowAt(pos, borrow) and

rt = borrow.getRefType()

exists(boolean mutable, RefType rt |

this.implicitBorrowAt(pos, mutable) and

rt = getRefType(mutable)

paldepind · 2025-12-18T10:24:20Z

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

+    override predicate argumentHasImplicitBorrow(AstNode arg, BorrowKind borrow) {
      exists(ArgumentPosition pos |
-        this.implicitBorrowAt(pos) and
+        this.implicitBorrowAt(pos, borrow) and


Suggested change

override predicate argumentHasImplicitBorrow(AstNode arg, BorrowKind borrow) {

exists(ArgumentPosition pos |

this.implicitBorrowAt(pos) and

this.implicitBorrowAt(pos, borrow) and

override predicate argumentHasImplicitBorrow(AstNode arg, boolean mutable) {

exists(ArgumentPosition pos |

this.implicitBorrowAt(pos, mutable) and

paldepind · 2025-12-18T10:25:38Z

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

+private newtype TBorrowKind =
+  TNoBorrowKind() or
+  TSharedBorrowKind() or
+  TMutBorrowKind()
+
+private class BorrowKind extends TBorrowKind {
+  predicate isNoBorrow() { this = TNoBorrowKind() }
+
+  RefType getRefType() {
+    this = TSharedBorrowKind() and
+    result instanceof RefSharedType
+    or
+    this = TMutBorrowKind() and
+    result instanceof RefMutType
+  }
+
+  string toString() {
+    this = TNoBorrowKind() and
+    result = ""
+    or
+    this = TSharedBorrowKind() and
+    result = "&"
+    or
+    this = TMutBorrowKind() and
+    result = "&mut"
+  }
+}


Perhaps this

Suggested change

private newtype TBorrowKind =

TNoBorrowKind() or

TSharedBorrowKind() or

TMutBorrowKind()

private class BorrowKind extends TBorrowKind {

predicate isNoBorrow() { this = TNoBorrowKind() }

RefType getRefType() {

this = TSharedBorrowKind() and

result instanceof RefSharedType

or

this = TMutBorrowKind() and

result instanceof RefMutType

}

string toString() {

this = TNoBorrowKind() and

result = ""

or

this = TSharedBorrowKind() and

result = "&"

or

this = TMutBorrowKind() and

result = "&mut"

}

}

private newtype TBorrowKind =

TNoBorrowKind() or

TSomeBorrowKind(Boolean mutable)

private class BorrowKind extends TBorrowKind {

predicate isNoBorrow() { this = TNoBorrowKind() }

RefType getRefType() {

result = getRefType(any(boolean mutable | this = TSomeBorrowKind(mutable)))

}

string toString() {

this.isNoBorrow() and

result = ""

or

result = this.getRefType().toString()

}

}

or using Option.

paldepind · 2025-12-18T10:28:24Z

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

+    predicate argumentHasImplicitBorrow(AstNode arg, BorrowKind borrow) {
+      exists(this.resolveCallTarget(_, "", borrow)) and
+      borrow != TNoBorrowKind() and


Suggested change

predicate argumentHasImplicitBorrow(AstNode arg, BorrowKind borrow) {

exists(this.resolveCallTarget(_, "", borrow)) and

borrow != TNoBorrowKind() and

predicate argumentHasImplicitBorrow(AstNode arg, boolean mutable) {

exists(this.resolveCallTarget(_, "", TSomeBorrowKind(mutable))) and

paldepind · 2025-12-18T10:35:36Z

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

    // adjust for implicit borrow
    apos.isSelf() and
-    derefChainBorrow = ";borrow" and
+    derefChainBorrow = [";&", ";&mut"] and


Suggested change

derefChainBorrow = [";&", ";&mut"] and

derefChainBorrow = encodeDerefChainBorrow("", TSomeBorrowKind(_)) and

To avoid hard coding the encoding too much and instead use encodeDerefChainBorrow as the source of truth for the encoding.

paldepind · 2025-12-18T14:20:46Z

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

    apos.isSelf() and
-    derefChainBorrow = ".ref;" and
-    path = TypePath::cons(getRefTypeParameter(), path0)
+    derefChainBorrow = MethodCallMatchingInput::encodeDerefChainBorrow(".ref", TNoBorrowKind()) and


paldepind

Thanks for addressing my comments! Really great stuff 🚀

github-actions bot added the Rust Pull requests that update Rust code label Dec 5, 2025

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch 4 times, most recently from f8e7c13 to a74c4e5 Compare December 8, 2025 09:27

hvitved mentioned this pull request Dec 8, 2025

Rust: Include more calls in DB quality metrics #20954

Merged

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch 4 times, most recently from b13cf2b to c62c00a Compare December 11, 2025 10:37

github-advanced-security bot found potential problems Dec 11, 2025

View reviewed changes

rust/ql/lib/codeql/rust/internal/TypeInference.qll Fixed Show fixed Hide fixed

rust/ql/lib/codeql/rust/internal/TypeInference.qll Fixed Show fixed Hide fixed

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch 2 times, most recently from 078f152 to d1eb4ff Compare December 12, 2025 15:22

github-advanced-security bot found potential problems Dec 12, 2025

View reviewed changes

rust/ql/lib/codeql/rust/internal/TypeInference.qll Fixed Show fixed Hide fixed

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch 3 times, most recently from 19551e3 to 5682de7 Compare December 15, 2025 09:12

hvitved mentioned this pull request Dec 15, 2025

Rust: Also use specialized types when inferring types for calls #21027

Merged

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch from 5682de7 to 99b0d0c Compare December 15, 2025 19:31

github-advanced-security bot found potential problems Dec 15, 2025

View reviewed changes

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch 6 times, most recently from 85c8f0d to 63e381c Compare December 17, 2025 15:45

hvitved added the no-change-note-required This PR does not need a change note label Dec 17, 2025

hvitved marked this pull request as ready for review December 17, 2025 16:42

hvitved requested a review from a team as a code owner December 17, 2025 16:42

Copilot AI review requested due to automatic review settings December 17, 2025 16:42

hvitved requested a review from paldepind December 17, 2025 16:43

Copilot started reviewing on behalf of hvitved December 17, 2025 16:43 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

paldepind reviewed Dec 18, 2025

View reviewed changes

hvitved added 2 commits December 18, 2025 12:47

Rust: Distinguish &mut T from &T in type inference

3c6a757

Address review comments

aae6cd9

hvitved force-pushed the rust/type-inference-distinguish-mut-ref branch from 63e381c to aae6cd9 Compare December 18, 2025 11:50

hvitved requested a review from paldepind December 18, 2025 11:50

paldepind reviewed Dec 18, 2025

View reviewed changes

paldepind approved these changes Dec 18, 2025

View reviewed changes

hvitved merged commit 27874ca into github:main Dec 18, 2025
19 checks passed

hvitved deleted the rust/type-inference-distinguish-mut-ref branch December 18, 2025 14:59

		// todo: handle `core::ops::deref::DerefMut`
		op = "*" and path = "core::ops::deref::Deref" and method = "deref" and borrows = 1

	if this instanceof AssignmentOperation
	if this instanceof CompoundAssignmentExpr

	predicate implicitBorrow(AstNode n) {
	predicate implicitBorrow(AstNode n, boolean mutable) {

	if isMut = true then result instanceof RefMutType else result instanceof RefSharedType
	result = getRefType(isMut)

	derefChainBorrow = [";&", ";&mut"] and
	derefChainBorrow = encodeDerefChainBorrow("", TSomeBorrowKind(_)) and

Rust: Distinguish &mut T from &T in type inference #20973

Rust: Distinguish &mut T from &T in type inference #20973

Conversation

hvitved commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

paldepind 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!

paldepind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Rust: Distinguish `&mut T` from `&T` in type inference #20973

Rust: Distinguish `&mut T` from `&T` in type inference #20973

hvitved commented Dec 5, 2025 •

edited

Loading