Skip to content
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
7 changes: 3 additions & 4 deletions rust/ql/.generated.list

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion rust/ql/.gitattributes

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -266,23 +266,16 @@ module ExprTrees {
}
}

private AstNode getBlockChildNode(BlockExpr b, int i) {
result = b.getStmtList().getStatement(i)
or
i = b.getStmtList().getNumberOfStatements() and
result = b.getStmtList().getTailExpr()
}

class AsyncBlockExprTree extends StandardTree, PreOrderTree, PostOrderTree, AsyncBlockExpr {
override AstNode getChildNode(int i) { result = getBlockChildNode(this, i) }
override AstNode getChildNode(int i) { result = this.getStmtList().getStmtOrExpr(i) }

override predicate propagatesAbnormal(AstNode child) { none() }
}

class BlockExprTree extends StandardPostOrderTree, BlockExpr {
BlockExprTree() { not this.isAsync() }

override AstNode getChildNode(int i) { result = getBlockChildNode(this, i) }
override AstNode getChildNode(int i) { result = this.getStmtList().getStmtOrExpr(i) }

override predicate propagatesAbnormal(AstNode child) { child = this.getChildNode(_) }
}
Expand Down
4 changes: 3 additions & 1 deletion rust/ql/lib/codeql/rust/elements/StmtList.qll

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 33 additions & 3 deletions rust/ql/lib/codeql/rust/elements/internal/StmtListImpl.qll
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// generated by codegen, remove this comment if you wish to edit this file
/**
* This module provides a hand-modifiable wrapper around the generated class `StmtList`.
*
Expand All @@ -12,17 +11,48 @@ private import codeql.rust.elements.internal.generated.StmtList
* be referenced directly.
*/
module Impl {
// the following QLdoc is generated: if you need to edit it, do it in the schema file
/**
* A list of statements in a block.
* A list of statements in a block, with an optional tail expression at the
* end that determines the block's value.
*
* For example:
* ```rust
* {
* let x = 1;
* let y = 2;
* x + y
* }
* // ^^^^^^^^^
* ```
*/
class StmtList extends Generated::StmtList { }
class StmtList extends Generated::StmtList {
/**
* Gets the `index`th statement or expression of this statement list (0-based).
*
* This includes both the statements and any tail expression in the statement list. To access
* just the statements, use `getStatement`. To access just the tail expression, if any,
* use `getTailExpr`.
*/
AstNode getStmtOrExpr(int index) {
result = this.getStatement(index)
or
index = this.getNumberOfStatements() and
result = this.getTailExpr()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As per the above comment I'm not entirely convinced that we want to expose this predicate. But, if we do, then there is in fact already an implementation here. Given that I think we should:

  • Delete getBlockChildNode and use getStmtOrExpr in the control flow graph implementation.
  • Use the getBlockChildNode implementation for getStmtOrExpr as it is a bit simpler.
  • Remove the tests added in this PR as the CFG tests will then be exercising getStmtOrExpr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting, I will make some changes...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done the first two. I could be persuaded to delete the test, but I'm not convinced its redundant - it seems to me that each is testing slightly different areas of code in slightly different ways (there is a lot of overlap).

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense! Let's keep the tests.


/**
* Gets any of the statements or expressions of this statement list.
*
* This includes both the statements and any tail expression in the statement list. To access
* just the statements, use `getAStatement`. To access just the tail expression, if any,
* use `getTailExpr`.
*/
final AstNode getAStmtOrExpr() { result = this.getStmtOrExpr(_) }

/**
* Gets the number of statements or expressions of this statement list.
*/
final int getNumberOfStmtOrExpr() { result = count(int i | exists(this.getStmtOrExpr(i))) }
}
}
10 changes: 9 additions & 1 deletion rust/ql/lib/codeql/rust/elements/internal/generated/Raw.qll

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
instances
| gen_stmt_list.rs:3:27:12:1 | StmtList |
| gen_stmt_list.rs:7:5:10:5 | StmtList |
| gen_stmt_list.rs:3:27:14:1 | StmtList |
| gen_stmt_list.rs:8:5:12:5 | StmtList |
getAttr
getStatement
| gen_stmt_list.rs:7:5:10:5 | StmtList | 0 | gen_stmt_list.rs:8:9:8:18 | let ... = 1 |
| gen_stmt_list.rs:7:5:10:5 | StmtList | 1 | gen_stmt_list.rs:9:9:9:18 | let ... = 2 |
| gen_stmt_list.rs:8:5:12:5 | StmtList | 0 | gen_stmt_list.rs:9:9:9:18 | let ... = 1 |
| gen_stmt_list.rs:8:5:12:5 | StmtList | 1 | gen_stmt_list.rs:10:9:10:18 | let ... = 2 |
getTailExpr
| gen_stmt_list.rs:3:27:12:1 | StmtList | gen_stmt_list.rs:7:5:10:5 | { ... } |
| gen_stmt_list.rs:3:27:14:1 | StmtList | gen_stmt_list.rs:8:5:12:5 | { ... } |
| gen_stmt_list.rs:8:5:12:5 | StmtList | gen_stmt_list.rs:11:9:11:13 | ... + ... |

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions rust/ql/test/library-tests/elements/stmtlist/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
| StmtList.rs:4:19:9:1 | StmtList | 2 | hasTailExpr | 0:let ... = 1, 1:let ... = 2, 2:... + ... |
| StmtList.rs:11:18:15:1 | StmtList | 2 | | 0:let ... = 1, 1:let ... = 2 |
| StmtList.rs:17:19:20:1 | StmtList | 0 | hasTailExpr | 0:... + ... |
| StmtList.rs:22:18:25:1 | StmtList | 1 | | 0:ExprStmt |
| StmtList.rs:27:18:29:1 | StmtList | 0 | | |
| StmtList.rs:31:18:34:1 | StmtList | 0 | | |
| StmtList.rs:36:29:43:1 | StmtList | 0 | hasTailExpr | 0:if cond {...} else {...} |
| StmtList.rs:38:10:40:2 | StmtList | 0 | hasTailExpr | 0:1 |
| StmtList.rs:40:9:42:2 | StmtList | 0 | hasTailExpr | 0:2 |
13 changes: 13 additions & 0 deletions rust/ql/test/library-tests/elements/stmtlist/StmtList.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import rust
import TestUtils

from StmtList sl, string tail
where
toBeTested(sl) and
if sl.hasTailExpr() then tail = "hasTailExpr" else tail = ""
select sl, sl.getNumberOfStatements(), tail,
concat(int i, AstNode n |
n = sl.getStmtOrExpr(i)
|
i.toString() + ":" + n.toString(), ", " order by i
)
43 changes: 43 additions & 0 deletions rust/ql/test/library-tests/elements/stmtlist/StmtList.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

// --- tests ---

fn test1() -> i32 {
// two statements + tail expression
let a = 1;
let b = 2;
a + b
}

fn test2() -> () {
// two statements only
let a = 1;
let b = 2;
}

fn test3() -> i32 {
// tail expression only
1 + 2
}

fn test4() -> () {
// one statement only
1 + 2;
}

fn test5() -> () {
// neither
}

fn test6() -> () {
// neither
;
}

fn test7(cond: bool) -> i32 {
// nested blocks
if cond {
1
} else {
2
}
}
13 changes: 11 additions & 2 deletions rust/schema/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1828,18 +1828,27 @@ class _:
@annotate(StmtList)
class _:
"""
A list of statements in a block.
A list of statements in a block, with an optional tail expression at the
end that determines the block's value.

For example:
```rust
{
let x = 1;
let y = 2;
x + y
}
// ^^^^^^^^^
```
"""

statements: _ | doc("statements of this statement list") | desc("""
The statements of a `StmtList` do not include any tail expression, which
can be accessed with predicates such as `getTailExpr`.
""")
tail_expr: _ | doc("tail expression of this statement list") | desc("""
The tail expression is the expression at the end of a block, that
determines the block's value.
""")

@annotate(Struct, replace_bases={Item: None}) # still an Item via Adt
class _:
Expand Down