Skip to content

Commit cf92fbf

Browse files
owen-mcowen-mc
committed
Go: adopt ForeachStmt with destructuring
1 parent e259487 commit cf92fbf

3 files changed

Lines changed: 37 additions & 34 deletions

File tree

‎go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,19 @@ module GoCfg {
212212
}
213213

214214
class ForeachStmt extends LoopStmt {
215-
ForeachStmt() { none() }
215+
ForeachStmt() { this instanceof Go::RangeStmt }
216216

217+
// Go's `range` statement binds its key and value by destructuring the
218+
// current element rather than by evaluating ordinary target expressions,
219+
// so it opts in to the synthesized element node (see
220+
// `foreachHasElementNode`) and does not use the shared variable routing.
217221
Expr getVariable() { none() }
218222

219-
Expr getCollection() { none() }
223+
Expr getCollection() { result = this.(Go::RangeStmt).getDomain() }
220224
}
221225

226+
predicate foreachHasElementNode(ForeachStmt foreach) { any() }
227+
222228
class BreakStmt = Go::BreakStmt;
223229

224230
class ContinueStmt = Go::ContinueStmt;
@@ -566,9 +572,6 @@ module GoCfg {
566572
tag = "result-zero-init:" + i.toString()
567573
)
568574
or
569-
// Next node for range statements
570-
n instanceof Go::RangeStmt and tag = "next"
571-
or
572575
// Send node
573576
n instanceof Go::SendStmt and
574577
not n = any(Go::CommClause cc).getComm() and
@@ -965,7 +968,7 @@ module GoCfg {
965968
predicate overridesCallableBodyExit(Ast::Callable c) { funcHasDefer(c.(Go::FuncDef)) }
966969

967970
predicate step(PreControlFlowNode n1, PreControlFlowNode n2) {
968-
rangeLoop(n1, n2) or
971+
rangeStmtStep(n1, n2) or
969972
selectStmt(n1, n2) or
970973
deferStmt(n1, n2) or
971974
goStmtStep(n1, n2) or
@@ -1424,17 +1427,15 @@ module GoCfg {
14241427
)
14251428
}
14261429

1427-
private predicate rangeLoop(PreControlFlowNode n1, PreControlFlowNode n2) {
1430+
private predicate rangeStmtStep(PreControlFlowNode n1, PreControlFlowNode n2) {
14281431
exists(Go::RangeStmt s |
1429-
// Use the shared library's auto-created `[LoopHeader]` additional node
1430-
// (created for every `LoopStmt`) as the join/branch point of the range loop.
1431-
n1.isBefore(s) and n2.isBefore(s.getDomain())
1432-
or
1433-
n1.isAfter(s.getDomain()) and n2.isAdditional(s, "[LoopHeader]")
1434-
or
1435-
n1.isAdditional(s, "[LoopHeader]") and n2.isAdditional(s, "next")
1436-
or
1437-
n1.isAdditional(s, "next") and
1432+
// The shared `ForeachStmt` model owns the loop skeleton (testing the
1433+
// domain for emptiness, the `[LoopHeader]` join/branch point, and the
1434+
// loop exit) and routes control flow into the synthesized
1435+
// `[ForeachElement]` node (see `foreachHasElementNode`). From there we
1436+
// destructure the current element into the key/value variables using
1437+
// the shared extract/assign epilogue machinery, then enter the body.
1438+
n1.isAdditional(s, "[ForeachElement]") and
14381439
(
14391440
exists(getFirstEpilogueTag(s)) and n2.isAdditional(s, getFirstEpilogueTag(s))
14401441
or
@@ -1448,10 +1449,6 @@ module GoCfg {
14481449
)
14491450
or
14501451
n1.isAdditional(s, getLastEpilogueTag(s)) and n2.isBefore(s.getBody())
1451-
or
1452-
n1.isAfter(s.getBody()) and n2.isAdditional(s, "[LoopHeader]")
1453-
or
1454-
n1.isAdditional(s, "[LoopHeader]") and n2.isAfter(s)
14551452
)
14561453
}
14571454

‎go/ql/lib/semmle/go/controlflow/IR.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ module IR {
790790
result = evalExprInstruction(baseExpr)
791791
)
792792
or
793-
result.(GetNextEntryInstruction).isAdditional(s, "next")
793+
result.(GetNextEntryInstruction).isAdditional(s, "[ForeachElement]")
794794
or
795795
result = evalExprInstruction(s.(ReturnStmt).getExpr())
796796
or
@@ -1148,7 +1148,7 @@ module IR {
11481148
class GetNextEntryInstruction extends Instruction {
11491149
RangeStmt rs;
11501150

1151-
GetNextEntryInstruction() { this.isAdditional(rs, "next") }
1151+
GetNextEntryInstruction() { this.isAdditional(rs, "[ForeachElement]") }
11521152

11531153
/**
11541154
* Gets the instruction computing the value whose key-value pairs this instruction reads.

‎go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3981,15 +3981,17 @@
39813981
| stmts.go:145:23:159:1 | block statement | stmts.go:145:23:159:1 | arg:0 block statement |
39823982
| stmts.go:145:23:159:1 | param-init:0 block statement | stmts.go:146:2:151:2 | range statement |
39833983
| stmts.go:146:2:151:2 | After range statement | stmts.go:153:2:155:2 | range statement |
3984+
| stmts.go:146:2:151:2 | [ForeachElement] range statement | stmts.go:146:2:151:2 | extract:0 range statement |
39843985
| stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | After range statement |
3985-
| stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | next range statement |
3986+
| stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | [ForeachElement] range statement |
39863987
| stmts.go:146:2:151:2 | assign:0 range statement | stmts.go:146:20:151:2 | block statement |
39873988
| stmts.go:146:2:151:2 | extract:0 range statement | stmts.go:146:2:151:2 | assign:0 range statement |
3988-
| stmts.go:146:2:151:2 | next range statement | stmts.go:146:2:151:2 | extract:0 range statement |
39893989
| stmts.go:146:2:151:2 | range statement | stmts.go:146:17:146:18 | Before xs |
3990-
| stmts.go:146:17:146:18 | After xs | stmts.go:146:2:151:2 | [LoopHeader] range statement |
3990+
| stmts.go:146:17:146:18 | After xs [empty] | stmts.go:146:2:151:2 | After range statement |
3991+
| stmts.go:146:17:146:18 | After xs [non-empty] | stmts.go:146:2:151:2 | [ForeachElement] range statement |
39913992
| stmts.go:146:17:146:18 | Before xs | stmts.go:146:17:146:18 | xs |
3992-
| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs |
3993+
| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs [empty] |
3994+
| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs [non-empty] |
39933995
| stmts.go:146:20:151:2 | After block statement | stmts.go:146:2:151:2 | [LoopHeader] range statement |
39943996
| stmts.go:146:20:151:2 | block statement | stmts.go:147:3:149:3 | if statement |
39953997
| stmts.go:147:3:149:3 | After if statement | stmts.go:150:3:150:14 | expression statement |
@@ -4021,17 +4023,19 @@
40214023
| stmts.go:150:13:150:13 | Before x | stmts.go:150:13:150:13 | x |
40224024
| stmts.go:150:13:150:13 | x | stmts.go:150:13:150:13 | After x |
40234025
| stmts.go:153:2:155:2 | After range statement | stmts.go:157:2:158:2 | range statement |
4026+
| stmts.go:153:2:155:2 | [ForeachElement] range statement | stmts.go:153:2:155:2 | extract:0 range statement |
40244027
| stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | After range statement |
4025-
| stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | next range statement |
4028+
| stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | [ForeachElement] range statement |
40264029
| stmts.go:153:2:155:2 | assign:0 range statement | stmts.go:153:2:155:2 | extract:1 range statement |
40274030
| stmts.go:153:2:155:2 | assign:1 range statement | stmts.go:153:23:155:2 | block statement |
40284031
| stmts.go:153:2:155:2 | extract:0 range statement | stmts.go:153:2:155:2 | assign:0 range statement |
40294032
| stmts.go:153:2:155:2 | extract:1 range statement | stmts.go:153:2:155:2 | assign:1 range statement |
4030-
| stmts.go:153:2:155:2 | next range statement | stmts.go:153:2:155:2 | extract:0 range statement |
40314033
| stmts.go:153:2:155:2 | range statement | stmts.go:153:20:153:21 | Before xs |
4032-
| stmts.go:153:20:153:21 | After xs | stmts.go:153:2:155:2 | [LoopHeader] range statement |
4034+
| stmts.go:153:20:153:21 | After xs [empty] | stmts.go:153:2:155:2 | After range statement |
4035+
| stmts.go:153:20:153:21 | After xs [non-empty] | stmts.go:153:2:155:2 | [ForeachElement] range statement |
40334036
| stmts.go:153:20:153:21 | Before xs | stmts.go:153:20:153:21 | xs |
4034-
| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs |
4037+
| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs [empty] |
4038+
| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs [non-empty] |
40354039
| stmts.go:153:23:155:2 | After block statement | stmts.go:153:2:155:2 | [LoopHeader] range statement |
40364040
| stmts.go:153:23:155:2 | block statement | stmts.go:154:3:154:17 | expression statement |
40374041
| stmts.go:154:3:154:11 | After selection of Print | stmts.go:154:13:154:13 | Before i |
@@ -4050,13 +4054,15 @@
40504054
| stmts.go:154:16:154:16 | Before v | stmts.go:154:16:154:16 | v |
40514055
| stmts.go:154:16:154:16 | v | stmts.go:154:16:154:16 | After v |
40524056
| stmts.go:157:2:158:2 | After range statement | stmts.go:145:23:159:1 | After block statement |
4057+
| stmts.go:157:2:158:2 | [ForeachElement] range statement | stmts.go:157:15:158:2 | block statement |
40534058
| stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | After range statement |
4054-
| stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | next range statement |
4055-
| stmts.go:157:2:158:2 | next range statement | stmts.go:157:15:158:2 | block statement |
4059+
| stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | [ForeachElement] range statement |
40564060
| stmts.go:157:2:158:2 | range statement | stmts.go:157:12:157:13 | Before xs |
4057-
| stmts.go:157:12:157:13 | After xs | stmts.go:157:2:158:2 | [LoopHeader] range statement |
4061+
| stmts.go:157:12:157:13 | After xs [empty] | stmts.go:157:2:158:2 | After range statement |
4062+
| stmts.go:157:12:157:13 | After xs [non-empty] | stmts.go:157:2:158:2 | [ForeachElement] range statement |
40584063
| stmts.go:157:12:157:13 | Before xs | stmts.go:157:12:157:13 | xs |
4059-
| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs |
4064+
| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs [empty] |
4065+
| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs [non-empty] |
40604066
| stmts.go:157:15:158:2 | block statement | stmts.go:157:2:158:2 | [LoopHeader] range statement |
40614067
| tst.go:0:0:0:0 | After tst.go | tst.go:0:0:0:0 | Normal Exit |
40624068
| tst.go:0:0:0:0 | Entry | tst.go:0:0:0:0 | tst.go |

0 commit comments

Comments
 (0)