Skip to content

Rust: Replace recursion through forall with ranked recursion#21679

Open
hvitved wants to merge 3 commits intogithub:mainfrom
hvitved:rust/type-inference-forall-checks
Open

Rust: Replace recursion through forall with ranked recursion#21679
hvitved wants to merge 3 commits intogithub:mainfrom
hvitved:rust/type-inference-forall-checks

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Apr 9, 2026

A recent QA run showed a timeout on mimuw-distributed-systems-group/cmemu. The problematic predicate was hasNoCompatibleTargetNoBorrowToIndex, where we do recursion through forall via the inlined predicate hasNoCompatibleNonBlanketTargetCheck. This PR applies the standard trick of replacing such recursion with ranked recursion, and since the original check already happened inside such (another) ranked recursion, it means we are now doing two levels of ranked recursion (see QL doc on the newly introduced NoCompatibleTarget module).

I also noted that in the original hasNoCompatibleTargetSharedBorrow check we were calling hasNoCompatibleNonBlanketLikeTargetCheck, which should have been hasNoCompatibleTargetCheck (just like in hasNoCompatibleTargetNoBorrow), so this has been fixed as well.

The second commit is an unrelated improvement to how we check blanket constraints.

DCA shows improved performance as well as reduction in inconsistencies (because of the hasNoCompatibleTargetCheck change), except for rust-lang/rust: while type inference is in fact faster, the query rust/access-after-lifetime-ended is much slower, as a side-effect of improved call resolution.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Apr 9, 2026
@hvitved hvitved force-pushed the rust/type-inference-forall-checks branch from 27f8658 to 5c6c3c0 Compare April 9, 2026 11:55
@hvitved hvitved force-pushed the rust/type-inference-forall-checks branch 4 times, most recently from 1888e7a to ad0a58f Compare April 9, 2026 18:35
@hvitved hvitved force-pushed the rust/type-inference-forall-checks branch from ad0a58f to e7930fe Compare April 10, 2026 08:19
@hvitved hvitved force-pushed the rust/type-inference-forall-checks branch from e7930fe to 27f7f74 Compare April 10, 2026 11:22
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 13, 2026
@hvitved hvitved marked this pull request as ready for review April 13, 2026 08:21
@hvitved hvitved requested a review from a team as a code owner April 13, 2026 08:21
Copilot AI review requested due to automatic review settings April 13, 2026 08:21
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 updates the Rust type inference library to avoid timeouts caused by recursion-through-forall in call-target compatibility checks, and includes a small refinement to blanket-constraint handling. It also updates/adjusts affected library tests and expectations to reflect the new resolution behavior.

Changes:

  • Replaces recursion-through-forall with ranked recursion for “no compatible target” checks (via a new NoCompatibleTarget helper module) and fixes an incorrect compatibility check used for shared-borrow cases.
  • Refines blanket-constraint checking to work from the bound Path (and updates related constraint-satisfaction plumbing accordingly).
  • Updates type-inference and dataflow library-test sources/expected outputs for the new behavior.
Show a summary per file
File Description
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll Introduces ranked-recursion based “no compatible target” logic and refactors borrow-candidate selection.
rust/ql/lib/codeql/rust/internal/typeinference/BlanketImplementation.qll Adjusts blanket-constraint selection to use bound paths and updates constraint satisfaction interface usage.
rust/ql/lib/codeql/rust/internal/PathResolution.qll Adds getBoundPath(int) and refactors bound resolution to use it.
rust/ql/test/library-tests/type-inference/closure.rs Updates/extends a closure test case to capture the new/known-missing resolution behavior.
rust/ql/test/library-tests/type-inference/type-inference.expected Updates expected type-inference results for the modified call/borrow resolution behavior.
rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected Updates expected local dataflow steps reflecting changed implicit borrow/deref modeling.

Copilot's findings

  • Files reviewed: 6/6 changed files
  • Comments generated: 1

@hvitved hvitved requested a review from geoffw0 April 13, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants