Skip to content

Commit 6da3972

Browse files
hvitvedhvitved
committed
Rust: Simplify break/continue CFG labels
1 parent 1266f97 commit 6da3972

File tree

5 files changed

+42
-77
lines changed

5 files changed

+42
-77
lines changed

‎rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ final class BooleanSuccessor = BooleanSuccessorImpl;
1919

2020
final class MatchSuccessor = MatchSuccessorImpl;
2121

22-
final class LoopJumpSuccessor = LoopJumpSuccessorImpl;
22+
final class BreakSuccessor = BreakSuccessorImpl;
23+
24+
final class ContinueSuccessor = ContinueSuccessorImpl;
2325

2426
final class ReturnSuccessor = ReturnSuccessorImpl;
2527

‎rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,8 @@ newtype TCompletion =
77
TSimpleCompletion() or
88
TBooleanCompletion(Boolean b) or
99
TMatchCompletion(Boolean isMatch) or
10-
TLoopCompletion(TLoopJumpType kind, TLabelType label) {
11-
label = TNoLabel()
12-
or
13-
kind = TBreakJump() and label = TLabel(any(BreakExpr b).getLifetime().getText())
14-
or
15-
kind = TContinueJump() and label = TLabel(any(ContinueExpr b).getLifetime().getText())
16-
} or
10+
TBreakCompletion() or
11+
TContinueCompletion() or
1712
TReturnCompletion()
1813

1914
/** A completion of a statement or an expression. */
@@ -148,42 +143,23 @@ class MatchCompletion extends TMatchCompletion, ConditionalCompletion {
148143
}
149144

150145
/**
151-
* A completion that represents a break or a continue.
146+
* A completion that represents a `break`.
152147
*/
153-
class LoopJumpCompletion extends TLoopCompletion, Completion {
154-
override LoopJumpSuccessor getAMatchingSuccessorType() {
155-
result = TLoopSuccessor(this.getKind(), this.getLabelType())
156-
}
157-
158-
final TLoopJumpType getKind() { this = TLoopCompletion(result, _) }
159-
160-
final TLabelType getLabelType() { this = TLoopCompletion(_, result) }
148+
class BreakCompletion extends TBreakCompletion, Completion {
149+
override BreakSuccessor getAMatchingSuccessorType() { any() }
161150

162-
final predicate hasLabel() { this.getLabelType() = TLabel(_) }
151+
override predicate isValidForSpecific(AstNode e) { e instanceof BreakExpr }
163152

164-
final string getLabelName() { TLabel(result) = this.getLabelType() }
165-
166-
final predicate isContinue() { this.getKind() = TContinueJump() }
153+
override string toString() { result = this.getAMatchingSuccessorType().toString() }
154+
}
167155

168-
final predicate isBreak() { this.getKind() = TBreakJump() }
156+
/**
157+
* A completion that represents a `continue`.
158+
*/
159+
class ContinueCompletion extends TContinueCompletion, Completion {
160+
override ContinueSuccessor getAMatchingSuccessorType() { any() }
169161

170-
override predicate isValidForSpecific(AstNode e) {
171-
this.isBreak() and
172-
e instanceof BreakExpr and
173-
(
174-
not e.(BreakExpr).hasLifetime() and not this.hasLabel()
175-
or
176-
e.(BreakExpr).getLifetime().getText() = this.getLabelName()
177-
)
178-
or
179-
this.isContinue() and
180-
e instanceof ContinueExpr and
181-
(
182-
not e.(ContinueExpr).hasLifetime() and not this.hasLabel()
183-
or
184-
e.(ContinueExpr).getLifetime().getText() = this.getLabelName()
185-
)
186-
}
162+
override predicate isValidForSpecific(AstNode e) { e instanceof ContinueExpr }
187163

188164
override string toString() { result = this.getAMatchingSuccessorType().toString() }
189165
}

‎rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,9 @@ class ContinueExprTree extends LeafTree, ContinueExpr {
186186
override predicate last(AstNode last, Completion c) { none() }
187187

188188
override predicate succ(AstNode pred, AstNode succ, Completion c) {
189-
exists(Expr target |
190-
pred = this and
191-
c.isValidFor(pred) and
192-
target = this.getTarget() and
193-
first(target.(LoopingExprTree).getLoopContinue(), succ)
194-
)
189+
pred = this and
190+
c.isValidFor(pred) and
191+
first(this.getTarget().(LoopingExprTree).getLoopContinue(), succ)
195192
}
196193
}
197194

‎rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ newtype TSuccessorType =
1515
TSuccessorSuccessor() or
1616
TBooleanSuccessor(Boolean b) or
1717
TMatchSuccessor(Boolean b) or
18-
TLoopSuccessor(TLoopJumpType kind, TLabelType label) { exists(TLoopCompletion(kind, label)) } or
18+
TBreakSuccessor() or
19+
TContinueSuccessor() or
1920
TReturnSuccessor()
2021

2122
/** The type of a control flow successor. */
@@ -59,28 +60,17 @@ class MatchSuccessorImpl extends ConditionalSuccessorImpl, TMatchSuccessor {
5960
}
6061

6162
/**
62-
* A control flow successor of a loop control flow expression, `continue` or `break`.
63+
* A control flow successor of a `break` expression.
6364
*/
64-
class LoopJumpSuccessorImpl extends SuccessorTypeImpl, TLoopSuccessor {
65-
private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
66-
67-
private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
68-
69-
predicate hasLabel() { this.getLabelType() = TLabel(_) }
70-
71-
string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
72-
73-
predicate isContinue() { this.getKind() = TContinueJump() }
74-
75-
predicate isBreak() { this.getKind() = TBreakJump() }
65+
class BreakSuccessorImpl extends SuccessorTypeImpl, TBreakSuccessor {
66+
override string toString() { result = "break" }
67+
}
7668

77-
override string toString() {
78-
exists(string kind, string label |
79-
(if this.isContinue() then kind = "continue" else kind = "break") and
80-
(if this.hasLabel() then label = "(" + this.getLabelName() + ")" else label = "") and
81-
result = kind + label
82-
)
83-
}
69+
/**
70+
* A control flow successor of a `continue` expression.
71+
*/
72+
class ContinueSuccessorImpl extends SuccessorTypeImpl, TContinueSuccessor {
73+
override string toString() { result = "continue" }
8474
}
8575

8676
/**

‎rust/ql/test/library-tests/controlflow/Cfg.expected

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ edges
7777
| test.rs:31:24:33:17 | IfExpr | test.rs:29:17:33:17 | IfExpr | |
7878
| test.rs:31:27:31:27 | b | test.rs:31:24:33:17 | IfExpr | false |
7979
| test.rs:31:27:31:27 | b | test.rs:32:21:32:33 | ExprStmt | true |
80-
| test.rs:32:21:32:32 | BreakExpr | test.rs:27:9:36:9 | LoopExpr | break('outer) |
80+
| test.rs:32:21:32:32 | BreakExpr | test.rs:27:9:36:9 | LoopExpr | break |
8181
| test.rs:32:21:32:33 | ExprStmt | test.rs:32:21:32:32 | BreakExpr | |
82-
| test.rs:34:17:34:28 | BreakExpr | test.rs:28:13:35:13 | LoopExpr | break('inner) |
82+
| test.rs:34:17:34:28 | BreakExpr | test.rs:28:13:35:13 | LoopExpr | break |
8383
| test.rs:34:17:34:29 | ExprStmt | test.rs:34:17:34:28 | BreakExpr | |
8484
| test.rs:37:9:37:12 | true | test.rs:26:48:38:5 | BlockExpr | |
8585
| test.rs:40:5:52:5 | enter test_continue_with_labels | test.rs:42:13:42:14 | ExprStmt | |
@@ -94,9 +94,9 @@ edges
9494
| test.rs:46:24:48:17 | IfExpr | test.rs:44:17:48:17 | IfExpr | |
9595
| test.rs:46:27:46:27 | b | test.rs:46:24:48:17 | IfExpr | false |
9696
| test.rs:46:27:46:27 | b | test.rs:47:21:47:36 | ExprStmt | true |
97-
| test.rs:47:21:47:35 | ContinueExpr | test.rs:42:13:42:14 | ExprStmt | continue('outer) |
97+
| test.rs:47:21:47:35 | ContinueExpr | test.rs:42:13:42:14 | ExprStmt | continue |
9898
| test.rs:47:21:47:36 | ExprStmt | test.rs:47:21:47:35 | ContinueExpr | |
99-
| test.rs:49:17:49:31 | ContinueExpr | test.rs:44:17:48:17 | ExprStmt | continue('inner) |
99+
| test.rs:49:17:49:31 | ContinueExpr | test.rs:44:17:48:17 | ExprStmt | continue |
100100
| test.rs:49:17:49:32 | ExprStmt | test.rs:49:17:49:31 | ContinueExpr | |
101101
| test.rs:54:5:66:5 | enter test_loop_label_shadowing | test.rs:56:13:56:14 | ExprStmt | |
102102
| test.rs:56:13:56:13 | 1 | test.rs:58:17:62:17 | ExprStmt | |
@@ -110,9 +110,9 @@ edges
110110
| test.rs:60:24:62:17 | IfExpr | test.rs:58:17:62:17 | IfExpr | |
111111
| test.rs:60:27:60:27 | b | test.rs:60:24:62:17 | IfExpr | false |
112112
| test.rs:60:27:60:27 | b | test.rs:61:21:61:35 | ExprStmt | true |
113-
| test.rs:61:21:61:34 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue('loop) |
113+
| test.rs:61:21:61:34 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue |
114114
| test.rs:61:21:61:35 | ExprStmt | test.rs:61:21:61:34 | ContinueExpr | |
115-
| test.rs:63:17:63:30 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue('loop) |
115+
| test.rs:63:17:63:30 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue |
116116
| test.rs:63:17:63:31 | ExprStmt | test.rs:63:17:63:30 | ContinueExpr | |
117117
| test.rs:68:5:77:5 | enter test_while | test.rs:69:9:69:25 | LetStmt | |
118118
| test.rs:68:5:77:5 | exit test_while (normal) | test.rs:68:5:77:5 | exit test_while | |
@@ -338,7 +338,7 @@ edges
338338
| test.rs:182:16:182:20 | ... > ... | test.rs:182:13:184:13 | IfExpr | false |
339339
| test.rs:182:16:182:20 | ... > ... | test.rs:183:17:183:36 | ExprStmt | true |
340340
| test.rs:182:20:182:20 | 0 | test.rs:182:16:182:20 | ... > ... | |
341-
| test.rs:183:17:183:35 | BreakExpr | test.rs:181:13:186:9 | LoopExpr | break('label) |
341+
| test.rs:183:17:183:35 | BreakExpr | test.rs:181:13:186:9 | LoopExpr | break |
342342
| test.rs:183:17:183:36 | ExprStmt | test.rs:183:30:183:30 | a | |
343343
| test.rs:183:30:183:30 | a | test.rs:183:34:183:35 | 10 | |
344344
| test.rs:183:30:183:35 | ... > ... | test.rs:183:17:183:35 | BreakExpr | |
@@ -357,7 +357,7 @@ edges
357357
| test.rs:194:9:200:9 | IfExpr | test.rs:193:43:201:5 | BlockExpr | |
358358
| test.rs:194:13:196:9 | BlockExpr | test.rs:197:13:197:13 | 1 | true |
359359
| test.rs:194:13:196:9 | BlockExpr | test.rs:199:13:199:13 | 0 | false |
360-
| test.rs:195:13:195:30 | BreakExpr | test.rs:194:13:196:9 | BlockExpr | break('block) |
360+
| test.rs:195:13:195:30 | BreakExpr | test.rs:194:13:196:9 | BlockExpr | break |
361361
| test.rs:195:13:195:31 | ExprStmt | test.rs:195:26:195:26 | a | |
362362
| test.rs:195:26:195:26 | a | test.rs:195:30:195:30 | 0 | |
363363
| test.rs:195:26:195:30 | ... > ... | test.rs:195:13:195:30 | BreakExpr | |
@@ -511,7 +511,7 @@ edges
511511
| test.rs:283:12:283:28 | PathExpr | test.rs:283:12:283:30 | CallExpr | |
512512
| test.rs:283:12:283:30 | CallExpr | test.rs:283:9:285:9 | IfExpr | false |
513513
| test.rs:283:12:283:30 | CallExpr | test.rs:284:13:284:27 | ExprStmt | true |
514-
| test.rs:284:13:284:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break('block) |
514+
| test.rs:284:13:284:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break |
515515
| test.rs:284:13:284:27 | ExprStmt | test.rs:284:26:284:26 | 1 | |
516516
| test.rs:284:26:284:26 | 1 | test.rs:284:13:284:26 | BreakExpr | |
517517
| test.rs:286:9:286:21 | PathExpr | test.rs:286:9:286:23 | CallExpr | |
@@ -522,7 +522,7 @@ edges
522522
| test.rs:287:12:287:28 | PathExpr | test.rs:287:12:287:30 | CallExpr | |
523523
| test.rs:287:12:287:30 | CallExpr | test.rs:287:9:289:9 | IfExpr | false |
524524
| test.rs:287:12:287:30 | CallExpr | test.rs:288:13:288:27 | ExprStmt | true |
525-
| test.rs:288:13:288:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break('block) |
525+
| test.rs:288:13:288:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break |
526526
| test.rs:288:13:288:27 | ExprStmt | test.rs:288:26:288:26 | 2 | |
527527
| test.rs:288:26:288:26 | 2 | test.rs:288:13:288:26 | BreakExpr | |
528528
| test.rs:290:9:290:21 | PathExpr | test.rs:290:9:290:23 | CallExpr | |
@@ -542,7 +542,7 @@ edges
542542
| test.rs:298:13:298:19 | TupleStructPat | test.rs:299:13:299:27 | ExprStmt | no-match |
543543
| test.rs:298:13:298:19 | TupleStructPat | test.rs:301:9:301:9 | x | match |
544544
| test.rs:298:23:298:23 | x | test.rs:298:13:298:19 | TupleStructPat | |
545-
| test.rs:299:13:299:26 | BreakExpr | test.rs:296:18:302:5 | BlockExpr | break('block) |
545+
| test.rs:299:13:299:26 | BreakExpr | test.rs:296:18:302:5 | BlockExpr | break |
546546
| test.rs:299:13:299:27 | ExprStmt | test.rs:299:26:299:26 | 1 | |
547547
| test.rs:299:26:299:26 | 1 | test.rs:299:13:299:26 | BreakExpr | |
548548
| test.rs:301:9:301:9 | x | test.rs:296:18:302:5 | BlockExpr | |

0 commit comments

Comments
 (0)