Skip to content

Rust: Handle chained let expressions #20203

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.

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
merged 3 commits into from
Aug 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions rust/ql/lib/change-notes/2025-08-14-if-while-let-chains.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* [`let` chains in `if` and `while`](https://doc.rust-lang.org/edition-guide/rust-2024/let-chains.html) are now supported, as well as [`if let` guards in `match` expressions](https://rust-lang.github.io/rfcs/2294-if-let-guard.html).
3 changes: 1 addition & 2 deletions rust/ql/lib/codeql/rust/controlflow/internal/CfgNodes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ private import codeql.rust.controlflow.CfgNodes
private import codeql.rust.internal.CachedStages

private predicate isPostOrder(AstNode n) {
n instanceof Expr and
not n instanceof LetExpr
n instanceof Expr
or
n instanceof OrPat
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ class TypeReprTree extends LeafTree instanceof TypeRepr { }
/**
* Provides `ControlFlowTree`s for expressions.
*
* Since expressions construct values, they are modeled in post-order, except for
* `LetExpr`s.
* Since expressions construct values, they are modeled in post-order.
*/
module ExprTrees {
class ArrayExprTree extends StandardPostOrderTree, ArrayExpr {
Expand Down Expand Up @@ -341,21 +340,15 @@ module ExprTrees {
child = [super.getCondition(), super.getABranch()]
}

private ConditionalCompletion conditionCompletion(Completion c) {
if super.getCondition() instanceof LetExpr
then result = c.(MatchCompletion)
else result = c.(BooleanCompletion)
}

override predicate succ(AstNode pred, AstNode succ, Completion c) {
// Edges from the condition to the branches
last(super.getCondition(), pred, c) and
(
first(super.getThen(), succ) and this.conditionCompletion(c).succeeded()
first(super.getThen(), succ) and c.(ConditionalCompletion).succeeded()
or
first(super.getElse(), succ) and this.conditionCompletion(c).failed()
first(super.getElse(), succ) and c.(ConditionalCompletion).failed()
or
not super.hasElse() and succ = this and this.conditionCompletion(c).failed()
not super.hasElse() and succ = this and c.(ConditionalCompletion).failed()
)
or
// An edge from the then branch to the last node
Expand Down Expand Up @@ -401,10 +394,7 @@ module ExprTrees {
}
}

// `LetExpr` is a pre-order tree such that the pattern itself ends up
// dominating successors in the graph in the same way that patterns do in
// `match` expressions.
class LetExprTree extends StandardPreOrderTree, LetExpr {
class LetExprTree extends StandardPostOrderTree, LetExpr {
override AstNode getChildNode(int i) {
i = 0 and
result = this.getScrutinee()
Expand Down Expand Up @@ -456,21 +446,15 @@ module ExprTrees {

override predicate first(AstNode node) { first(super.getCondition(), node) }

private ConditionalCompletion conditionCompletion(Completion c) {
if super.getCondition() instanceof LetExpr
then result = c.(MatchCompletion)
else result = c.(BooleanCompletion)
}

override predicate succ(AstNode pred, AstNode succ, Completion c) {
super.succ(pred, succ, c)
or
last(super.getCondition(), pred, c) and
this.conditionCompletion(c).succeeded() and
c.(ConditionalCompletion).succeeded() and
first(this.getLoopBody(), succ)
or
last(super.getCondition(), pred, c) and
this.conditionCompletion(c).failed() and
c.(ConditionalCompletion).failed() and
succ = this
}
}
Expand Down
11 changes: 4 additions & 7 deletions rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,7 @@ module ConditionalCompletionSplitting {
child = parent.(LogicalNotExpr).getExpr() and
childCompletion.getDual() = parentCompletion
or
(
childCompletion = parentCompletion
or
// needed for `let` expressions
childCompletion.(MatchCompletion).getValue() =
parentCompletion.(BooleanCompletion).getValue()
) and
childCompletion = parentCompletion and
(
child = parent.(BinaryLogicalOperation).getAnOperand()
or
Expand All @@ -92,6 +86,9 @@ module ConditionalCompletionSplitting {
or
child = parent.(PatternTrees::PostOrderPatTree).getPat(_)
)
or
child = parent.(LetExpr).getPat() and
childCompletion.(MatchCompletion).getValue() = parentCompletion.(BooleanCompletion).getValue()
}
}

Expand Down
13 changes: 10 additions & 3 deletions rust/ql/lib/codeql/rust/dataflow/Ssa.qll
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,16 @@ module Ssa {
ae.getRhs() = value
)
or
exists(LetStmtCfgNode ls |
ls.getPat().(IdentPatCfgNode).getName() = write and
ls.getInitializer() = value
exists(IdentPatCfgNode pat | pat.getName() = write |
exists(LetStmtCfgNode ls |
pat = ls.getPat() and
ls.getInitializer() = value
)
or
exists(LetExprCfgNode le |
pat = le.getPat() and
le.getScrutinee() = value
)
)
}

Expand Down
14 changes: 14 additions & 0 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ module LocalFlow {
nodeTo.getCfgNode() = s.getPat()
)
or
// An edge from the right-hand side of a let expression to the left-hand side.
exists(LetExprCfgNode e |
nodeFrom.getCfgNode() = e.getScrutinee() and
nodeTo.getCfgNode() = e.getPat()
)
or
exists(IdentPatCfgNode p |
not p.isRef() and
nodeFrom.getCfgNode() = p and
Expand Down Expand Up @@ -379,6 +385,8 @@ module RustDataFlow implements InputSig<Location> {
predicate neverSkipInPathGraph(Node node) {
node.(Node::Node).getCfgNode() = any(LetStmtCfgNode s).getPat()
or
node.(Node::Node).getCfgNode() = any(LetExprCfgNode e).getPat()
or
node.(Node::Node).getCfgNode() = any(AssignmentExprCfgNode a).getLhs()
or
exists(MatchExprCfgNode match |
Expand Down Expand Up @@ -899,6 +907,12 @@ module VariableCapture {
v.getPat() = ls.getPat().getPat() and
ls.getInitializer() = source
)
or
exists(LetExprCfgNode le |
this = le and
v.getPat() = le.getPat().getPat() and
le.getScrutinee() = source
)
}

CapturedVariable getVariable() { result = v }
Expand Down
Loading