Rust: Rework call disambiguation logic #21217

hvitved · 2026-01-26T14:27:30Z

The following simple program would make our type inference implementation explode:

#[derive(Default)]
struct S;

fn f() -> S {
    let x = Default::default();
    From::from(x)
}

Firstly, because of a getSuccessor vs getAssocItem bug, From::from would allow for any sub trait of From, for example the Int trait, which meant that x could be inferred to have type bool. Secondly, since many from implementations are polymorphic in their argument, we should really be checking that the return type matches the type of the calling context.

This PR resolves the above, and other issues (see tests) by rewriting when and how we do type-based call disambiguation, aligning the cases for methods and non-methods. Key changes:

As for calls to methods, we now also always check the Self type for calls to associated non-methods f. The type at the call site may be obtained by looking at the qualifier, e.g. Vec::<i32>::new() (which also means we need extra care for default type arguments, such as the default Global for A in Vec). Additionally, when f is (an implementation of) a trait function (that is, not in an inherent implementation), we may additionally obtain the type at any of the positions at which Self is mentioned in the trait function, for example in Default::default() the type may be obtained from the context of the call.
Since the Self type is now always checked, both cases can now use the functionResolutionDependsOnArgument predicate from Overloading.qll, because the sibling logic assumes we only want to disambiguate targets with the same Self type.
The logic in ArgsAreInstantiationsOf has been changed to do a ranked forall over type parameters instead of over type positions, because what we really want is to check that there is some position at which a given type parameter occurs, where the types match. This allows us to handle cases like this one.
The typeCanBeUsedForDisambiguation logic has been removed, which gives us more call targets, but also some false targets like this one. I tried to take type parameter constraints into account as well, but that turned out quite difficult, since in cases like let mut i = 0i64; let mut pin = Pin::new(&i); we need type information from both the Pin qualifier and the &i argument to first match against the Self type, and subsequently to validate the Ptr : Deref constraint of the new function.

~~DCA~~ DCA looks great; while we fix the performance issue, we increase Percentage of calls with call target by ~~1.67~~ 1.73 % point. There is an increase in Nodes With Type At Length Limit, in particular on radiance, which is a consequence of removing typeCanBeUsedForDisambiguation. This PR also fixes 8 projects that currently timeout during analysis.

rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll

hvitved · 2026-01-27T07:31:29Z

shared/typeinference/codeql/typeinference/internal/TypeInference.qll

+        hasTypeConstraint(tt, constraint) and
        t = getTypeAt(tt, path)


This conjunction has non-linear recursion, hence the magic'ed version of hasTypeConstraint.

Copilot

Pull request overview

This PR fixes a type inference explosion issue in Rust queries by correcting two bugs in the type inference implementation:

Path resolution bug: Changed getASuccessor(_) to getAnAssocItem() to correctly resolve trait functions only from the trait itself, not from sub-traits
Context-based return type checking: Enhanced the logic to only use return type for disambiguation when all argument positions are trivially satisfied

Changes:

Fixed trait function resolution to prevent matching sub-trait implementations
Added logic to check return type matches calling context for polymorphic functions
Refactored Input modules into a unified structure
Added regression test for the Default::default() and From::from() pattern

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
shared/typeinference/codeql/typeinference/internal/TypeInference.qll	Added helper predicate with `pragma[nomagic]` to optimize type constraint checking
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll	Fixed `getASuccessor` → `getAnAssocItem` bug, merged Input modules, fixed parameter ordering in context typing
rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll	Split predicate and added return type disambiguation logic
rust/ql/test/library-tests/type-inference/main.rs	Added regression test for `from_default` module
rust/ql/test/library-tests/type-inference/type-inference.expected	Updated expected test output
rust/ql/test/query-tests/security/CWE-312/CONSISTENCY/PathResolutionConsistency.expected	Removed spurious multiple resolution error

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

geoffw0

Looks helpful.

Is there a reason this is still draft?

hvitved · 2026-01-29T10:26:01Z

Is there a reason this is still draft?

I would like to rework this a bit more; while it does fix the example posted in the description, there are still cases resulting in timeouts that I would like to cover as well.

shared/typeinference/codeql/typeinference/internal/TypeInference.qll

rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll

rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll

+ * for non-methods.
+ */
+pragma[nomagic]
+Type getAssocFunctionTypeInclNonMethodSelfAt(


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

geoffw0

LGTM. This is a complex CodeQL change that's difficult to review closely - if there are any particular areas you'd like to be scrutinised in more detail, please point to them. The excellent new tests, the existing tests, and the DCA run add a lot of confidence for me.

geoffw0 · 2026-02-05T15:42:46Z

rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll

   * Holds if all arguments of `call` have types that are instantiations of the
   * types of the corresponding parameters of `f` inside `i`.
+   *
+   * TODO: Check type parameter constraints as well.


Are the two TODO comments intended as follow-up work?

paldepind

Great improvements!

paldepind · 2026-02-09T08:30:28Z

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

+    #[derive(Default)]
+    struct S;
+    impl FirstTrait for S {
+        // S::method2


This method and the other method2 below have the same comment, so we can't distinguish them in the expectations.

Suggested change

// S::method2

// S_as_FirstTrait::method2

paldepind · 2026-02-09T09:03:08Z

shared/util/codeql/util/UnboundList.qll

+     * Gets a prefix of this list, including the list itself.
+     */
+    bindingset[this]
+    UnboundList getAPrefixOrSelf() { result = [this, this.getAPrefix()] }


What about this renaming:

getPrefix -> getProperPrefix

getAPrefix -> getAProperPrefix

getAPrefixOrSelf -> getAPrefix

These names a more precise and the most used predicate, getAPrefixOrSelf, gets a shorter name.

paldepind · 2026-02-09T12:03:01Z

rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll

+        result = this.getDefaultPositionalTypeArgument(pragma[only_bind_into](i), path) and
+        tp = this.resolveRootType().getPositionalTypeParameter(pragma[only_bind_into](i))
+      )
+    }


There's a bit of duplication here and in getTypeForTypeParameterAt below.

Perhaps we could get rid of it with these predicates:

private Type getPositionalTypeArgument(int i, TypePath path, boolean isDefault) { result = getPathTypeArgument(this, i).getTypeAt(path) and isDefault = false or result = this.getDefaultPositionalTypeArgument(i, path) and isDefault = true } pragma[nomagic] Type getTypeArgument(TypeParameter tp, TypePath path, boolean isDefault) { exists(int i | result = this.getPositionalTypeArgument(pragma[only_bind_into](i), path, isDefault) and tp = this.resolveRootType().getPositionalTypeParameter(pragma[only_bind_into](i)) ) }

Where getTypeArgument can be used in getTypeForTypeParameterAt and in place of getDefaultTypeForTypeParameterInNonAnnotationAt as in this diff:

diff --git a/rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll b/rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll index 3e9c823c570..27052410294 100644 --- a/rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll +++ b/rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll @@ -1071,8 +1071,7 @@ private Type getCallExprTypeQualifier(CallExpr ce, TypePath path, boolean isDefa isDefaultTypeArg = false or exists(TypeParameter tp, TypePath suffix | - result = - tm.(NonAliasPathTypeMention).getDefaultTypeForTypeParameterInNonAnnotationAt(tp, suffix) and + result = tm.(NonAliasPathTypeMention).getTypeArgument(tp, suffix, true) and path = TypePath::cons(tp, suffix) and isDefaultTypeArg = true ) diff --git a/rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll b/rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll index a1177e55cc9..b41e86ed9a9 100644 --- a/rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll +++ b/rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll @@ -247,12 +247,20 @@ private module MkTypeMention<getAdditionalPathTypeAtSig/2 getAdditionalPathTypeA ) } - private Type getPositionalTypeArgument(int i, TypePath path) { - result = getPathTypeArgument(this, i).getTypeAt(path) + private Type getPositionalTypeArgument(int i, TypePath path, boolean isDefault) { + result = getPathTypeArgument(this, i).getTypeAt(path) and + isDefault = false or - // Defaults only apply to type mentions in type annotations - this.isInTypeAnnotation() and - result = this.getDefaultPositionalTypeArgument(i, path) + result = this.getDefaultPositionalTypeArgument(i, path) and + isDefault = true + } + + pragma[nomagic] + Type getTypeArgument(TypeParameter tp, TypePath path, boolean isDefault) { + exists(int i | + result = this.getPositionalTypeArgument(pragma[only_bind_into](i), path, isDefault) and + tp = this.resolveRootType().getPositionalTypeParameter(pragma[only_bind_into](i)) + ) } /** @@ -260,9 +268,8 @@ private module MkTypeMention<getAdditionalPathTypeAtSig/2 getAdditionalPathTypeA * type parameter does not correspond directly to a type mention. */ private Type getTypeForTypeParameterAt(TypeParameter tp, TypePath path) { - exists(int i | - result = this.getPositionalTypeArgument(pragma[only_bind_into](i), path) and - tp = this.resolveRootType().getPositionalTypeParameter(pragma[only_bind_into](i)) + exists(boolean isDefault | result = this.getTypeArgument(tp, path, isDefault) | + isDefault = false or this.isInTypeAnnotation() ) or // Handle the special syntactic sugar for function traits. The syntactic

I went for a slightly different refactor.

paldepind · 2026-02-09T12:27:57Z

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

+      result = this.getPathResolutionResolved() and
+      result = i.getASuccessor(_) and


Suggested change

result = this.getPathResolutionResolved() and

result = i.getASuccessor(_) and

result = this.getPathResolutionResolved(i) and

This is equivalent to changing i.getASuccessor(_) to i.getAnAssocItem() which I think this is what we want.

On rust db this roughly halves the size of this predicate with no changes to inferType.

More generally, I rarely think that we actually want to use getASuccessor on ImplItem in type inference. With recent changes that I made to path resolution getASuccessor can give anything that is available on Self inside the impl block, which is not necessarily functions that are actually within the impl block (see relevantSelfFunctionName).

Good catch.

paldepind · 2026-02-09T12:32:58Z

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

+    f = impl.getASuccessor(_) and
+    not impl.(Impl).hasTrait() and
+    tp = TTypeParamTypeParameter(impl.resolveSelfTy().getTypeParam(0)) and
+    not f.hasSelfParam() and


Suggested change

not f.hasSelfParam() and

f has type NonMethodFunction so this is superflous.

paldepind · 2026-02-09T12:35:23Z

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

+    // For inherent implementations of generic types, we also need to check the type being
+    // implemented. We arbitrarily choose the first type parameter of the type being implemented
+    // to represent this case.
+    f = impl.getASuccessor(_) and


Suggested change

f = impl.getASuccessor(_) and

f = impl.getAnAssocItem() and

Similar to this comment.

github-actions bot added the Rust Pull requests that update Rust code label Jan 26, 2026

github-advanced-security bot found potential problems Jan 26, 2026

View reviewed changes

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

hvitved force-pushed the rust/type-inference-perf branch 3 times, most recently from 38929dd to 50ce393 Compare January 26, 2026 19:16

hvitved changed the title ~~Rust: Type inference performance improvements~~ Rust: Fix context-based type inference explosion Jan 26, 2026

hvitved added the no-change-note-required This PR does not need a change note label Jan 27, 2026

hvitved commented Jan 27, 2026

View reviewed changes

hvitved marked this pull request as ready for review January 27, 2026 07:31

hvitved requested review from a team as code owners January 27, 2026 07:31

hvitved requested review from Copilot and paldepind January 27, 2026 07:31

Copilot started reviewing on behalf of hvitved January 27, 2026 07:32 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

hvitved marked this pull request as draft January 27, 2026 13:14

geoffw0 reviewed Jan 29, 2026

View reviewed changes

hvitved force-pushed the rust/type-inference-perf branch from 50ce393 to 3244471 Compare January 29, 2026 15:58

github-advanced-security bot found potential problems Jan 29, 2026

View reviewed changes

shared/typeinference/codeql/typeinference/internal/TypeInference.qll Fixed Show fixed Hide fixed

shared/typeinference/codeql/typeinference/internal/TypeInference.qll Fixed Show fixed Hide fixed

shared/typeinference/codeql/typeinference/internal/TypeInference.qll Fixed Show fixed Hide fixed

hvitved force-pushed the rust/type-inference-perf branch 2 times, most recently from c1b3551 to 9079fde Compare January 30, 2026 10:13

github-advanced-security bot found potential problems Jan 30, 2026

View reviewed changes

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

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

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

hvitved force-pushed the rust/type-inference-perf branch 7 times, most recently from e2ebf44 to 026ddfe Compare February 2, 2026 18:13

hvitved force-pushed the rust/type-inference-perf branch from 026ddfe to ee7c24b Compare February 3, 2026 12:32

github-advanced-security bot found potential problems Feb 3, 2026

View reviewed changes

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

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

hvitved force-pushed the rust/type-inference-perf branch from ee7c24b to dde7c24 Compare February 3, 2026 12:54

github-advanced-security bot found potential problems Feb 3, 2026

View reviewed changes

hvitved force-pushed the rust/type-inference-perf branch from dde7c24 to 9aad861 Compare February 3, 2026 14:15

github-advanced-security bot found potential problems Feb 3, 2026

View reviewed changes

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

hvitved force-pushed the rust/type-inference-perf branch 5 times, most recently from c663247 to b61ef72 Compare February 5, 2026 09:12

hvitved changed the title ~~Rust: Fix context-based type inference explosion~~ Rust: Rework call disambiguation logic Feb 5, 2026

hvitved force-pushed the rust/type-inference-perf branch from b61ef72 to 7b9ae9f Compare February 5, 2026 09:49

hvitved marked this pull request as ready for review February 5, 2026 10:36

hvitved added 3 commits February 5, 2026 12:49

Rust: Move some overloading tests into a separate file

025f733

Rust: More type inference tests

c62d95a

Rust: Merge Input1 and Input2 modules

2764d69

hvitved force-pushed the rust/type-inference-perf branch 2 times, most recently from 2158fdb to 7289906 Compare February 5, 2026 14:18

geoffw0 previously approved these changes Feb 5, 2026

View reviewed changes

hvitved added 2 commits February 5, 2026 17:29

Rust: Rework call disambiguation logic

2dc7576

Rust: Add type inference regression test

32aaac2

hvitved dismissed geoffw0’s stale review via 32aaac2 February 5, 2026 16:30

hvitved force-pushed the rust/type-inference-perf branch from 7289906 to 32aaac2 Compare February 5, 2026 16:30

geoffw0 previously approved these changes Feb 6, 2026

View reviewed changes

paldepind requested changes Feb 9, 2026

View reviewed changes

Address review comments

16539b4

hvitved dismissed geoffw0’s stale review via 16539b4 February 9, 2026 18:35

		hasTypeConstraint(tt, constraint) and
		t = getTypeAt(tt, path)

		result = this.getPathResolutionResolved() and
		result = i.getASuccessor(_) and

Rust: Rework call disambiguation logic #21217

Are you sure you want to change the base?

Rust: Rework call disambiguation logic #21217

Conversation

hvitved commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

hvitved commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Uh oh!

geoffw0 left a comment • 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hvitved commented Jan 26, 2026 •

edited

Loading

geoffw0 left a comment •

edited

Loading