Skip to content

Rust: Speedup inferMethodCallTypeSelf #21305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hvitved merged 2 commits into from
Feb 11, 2026
Merged

Rust: Speedup inferMethodCallTypeSelf #21305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
564a3bd
Rust: Simplify `inferMethodCallTypeSelf`
hvitved Feb 10, 2026
49f24ca
Rust: Avoid using `regexpCapture` with multiple capture groups
hvitved Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2477,10 +2477,10 @@ private module MethodCallMatchingInput implements MatchingWithEnvironmentInputSi
additional predicate decodeDerefChainBorrow(
string derefChainBorrow, DerefChain derefChain, BorrowKind borrow
) {
exists(string regexp |
regexp = "^(.*);(.*)$" and
derefChain = derefChainBorrow.regexpCapture(regexp, 1) and
borrow.toString() = derefChainBorrow.regexpCapture(regexp, 2)
exists(int i |
i = derefChainBorrow.indexOf(";") and
derefChain = derefChainBorrow.prefix(i) and
borrow.toString() = derefChainBorrow.suffix(i + 1)
)
}

Expand Down Expand Up @@ -2604,37 +2604,39 @@ private Type inferMethodCallTypeNonSelf(AstNode n, boolean isReturn, TypePath pa
}

/**
* Gets the type of `n` at `path` after applying `derefChain` and `borrow`,
* where `n` is the `self` argument of a method call.
* Gets the type of `n` at `path` after applying `derefChain`, where `n` is the
* `self` argument of a method call.
*
* The predicate recursively pops the head of `derefChain` until it becomes
* empty, at which point the inferred type can be applied back to `n`.
*/
pragma[nomagic]
private Type inferMethodCallTypeSelf(
AstNode n, DerefChain derefChain, BorrowKind borrow, TypePath path
) {
exists(MethodCallMatchingInput::AccessPosition apos, string derefChainBorrow |
result = inferMethodCallType0(_, apos, n, derefChainBorrow, path) and
private Type inferMethodCallTypeSelf(AstNode n, DerefChain derefChain, TypePath path) {
exists(
MethodCallMatchingInput::AccessPosition apos, string derefChainBorrow, BorrowKind borrow,
TypePath path0
|
result = inferMethodCallType0(_, apos, n, derefChainBorrow, path0) and
apos.isSelf() and
MethodCallMatchingInput::decodeDerefChainBorrow(derefChainBorrow, derefChain, borrow)
)
or
// adjust for implicit borrow
exists(TypePath path0, BorrowKind borrow0 |
result = inferMethodCallTypeSelf(n, derefChain, borrow0, path0) and
path0.isCons(borrow0.getRefType().getPositionalTypeParameter(0), path) and
borrow.isNoBorrow()
|
borrow.isNoBorrow() and
path = path0
or
// adjust for implicit borrow
exists(TypePath prefix |
prefix = TypePath::singleton(borrow.getRefType().getPositionalTypeParameter(0)) and
path0 = prefix.appendInverse(path)
)
)
or
// adjust for implicit deref
exists(
DerefChain derefChain0, Type t0, TypePath path0, DerefImplItemNode impl, Type selfParamType,
TypePath selfPath
|
t0 = inferMethodCallTypeSelf(n, derefChain0, borrow, path0) and
t0 = inferMethodCallTypeSelf(n, derefChain0, path0) and
derefChain0.isCons(impl, derefChain) and
borrow.isNoBorrow() and
selfParamType = impl.resolveSelfTypeAt(selfPath)
|
result = selfParamType and
Expand All @@ -2653,7 +2655,7 @@ private Type inferMethodCallTypeSelf(
private Type inferMethodCallTypePreCheck(AstNode n, boolean isReturn, TypePath path) {
result = inferMethodCallTypeNonSelf(n, isReturn, path)
or
result = inferMethodCallTypeSelf(n, DerefChain::nil(), TNoBorrowKind(), path) and
result = inferMethodCallTypeSelf(n, DerefChain::nil(), path) and
isReturn = false
}

Expand Down
21 changes: 14 additions & 7 deletions shared/util/codeql/util/UnboundList.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ module Make<LocationSig Location, InputSig<Location> Input> {
/** Holds if this list is empty. */
predicate isEmpty() { this = "" }

bindingset[this]
private int stringLength() { result = super.length() }

/** Gets the length of this list. */
bindingset[this]
pragma[inline_late]
Expand Down Expand Up @@ -115,19 +118,23 @@ module Make<LocationSig Location, InputSig<Location> Input> {
/** Holds if this list starts with `e`, followed by `suffix`. */
bindingset[this]
predicate isCons(Element e, UnboundList suffix) {
exists(string regexp | regexp = "^([0-9]+)\\.(.*)$" |
e = decode(this.regexpCapture(regexp, 1)) and
suffix = this.regexpCapture(regexp, 2)
exists(string elem |
// it is more efficient to not create a capture group for the suffix, since
// `regexpCapture` will then always join in both groups, only to afterwards filter
// based on the requested group (the group number is not part of the binding set
// of `regexpCapture`)
Copy link
Contributor

@paldepind paldepind Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this saying that each call to regexpCapture would produce both capture groups and in the first call the second capture group would be thrown away and vice versa for the second call? Or something else?

Copy link
Contributor Author

@hvitved hvitved Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Here is a simple example

predicate p1(string a, string b, string c) {
  a = "b;c" and
  exists(string reg |
    reg = "^(.*);(.*)$" and
    b = a.regexpCapture(reg, 1) and
    c = a.regexpCapture(reg, 2)
  )
}

which generates the following RA:

[2026-02-10 15:54:01] (0s) Tuple counts for quickquery::p1#QuickEval#c8bd3cdf#query/3@b482101n after 2ms:
                      1 ~0%     {2} r1 = CONSTANT(unique string, unique string)["b;c","^(.*);(.*)$"]
                      2 ~0%     {4}    | JOIN WITH PRIMITIVE regexpCapture#bbff ON Lhs.0,Lhs.1
                      2 ~0%     {5}    | SCAN OUTPUT In.0, In.1, In.2, In.3 'c', _
                                {4}    | REWRITE WITH Tmp.4 := 2, TEST InOut.2 = Tmp.4 KEEPING 4
                      1 ~0%     {3}    | SCAN OUTPUT In.3 'c', _, _
                      1 ~0%     {3}    | REWRITE WITH Out.1 := "b;c", Out.2 := "^(.*);(.*)$"
                      2 ~0%     {5}    | JOIN WITH PRIMITIVE regexpCapture#bbff ON Lhs.1,Lhs.2
                      2 ~0%     {6}    | SCAN OUTPUT In.0 'c', In.1, In.2, In.3, In.4 'b', _
                                {5}    | REWRITE WITH Tmp.5 := 1, TEST InOut.3 = Tmp.5 KEEPING 5
                      1 ~0%     {3}    | SCAN OUTPUT _, In.4 'b', In.0 'c'
                      1 ~0%     {3}    | REWRITE WITH Out.0 'a' := "b;c"
                                return r1

The workaround

predicate p4(string a, string b, string c) {
  a = "b;c" and
  exists(string reg |
    reg = "^(.*);.*$" and
    b = a.regexpCapture(reg, 1) and
    c = a.suffix(b.length() + 1)
  )
}

with just a single capture group performs better:

[2026-02-10 15:57:35] (0s) Tuple counts for quickquery::p4#QuickEval#b3644a80#query/3@e8e0caoi after 0ms:
                      1 ~0%     {2} r1 = CONSTANT(unique string, unique string)["b;c","^(.*);.*$"]
                      1 ~0%     {4}    | JOIN WITH PRIMITIVE regexpCapture#bbff ON Lhs.0,Lhs.1
                      1 ~0%     {5}    | SCAN OUTPUT In.0, In.1, In.2, In.3 'b', _
                                {4}    | REWRITE WITH Tmp.4 := 1, TEST InOut.2 = Tmp.4 KEEPING 4
                      1 ~0%     {4}    | SCAN OUTPUT In.3 'b', _, _, _
                                {2}    | REWRITE WITH Tmp.1 := "b;c", Tmp.2 := length(InOut.0), Tmp.3 := 1, Tmp.2 := (Tmp.2 + Tmp.3), Out.1 'c' := suffix(Tmp.1,Tmp.2) KEEPING 2
                      1 ~0%     {3}    | SCAN OUTPUT _, In.0 'b', In.1 'c'
                      1 ~0%     {3}    | REWRITE WITH Out.0 'a' := "b;c"

elem = this.regexpCapture("^([0-9]+)\\..*$", 1) and
Comment on lines +122 to +126
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new explanatory comment should start with a capital letter and could be tightened for readability (it currently spans multiple lines and uses a long parenthetical). Consider rephrasing with shorter sentences and consistent terminology (e.g., backticking bindingset).

This issue also appears on line 135 of the same file.

Copilot uses AI. Check for mistakes.
e = decode(elem) and
suffix = this.suffix(elem.length() + 1)
)
}

/** Holds if this list starts with `prefix`, followed by `e`. */
bindingset[this]
predicate isSnoc(UnboundList prefix, Element e) {
exists(string regexp | regexp = "^(|.+\\.)([0-9]+)\\.$" |
prefix = this.regexpCapture(regexp, 1) and
e = decode(this.regexpCapture(regexp, 2))
)
// same remark as above about not using multiple capture groups
prefix = this.regexpCapture("^(|.+\\.)[0-9]+\\.$", 1) and
e = decode(this.substring(prefix.stringLength(), this.stringLength() - 1))
}

/** Gets the head of this list, if any. */
Expand Down
Loading