Skip to content

Commit 76b55c4

Browse files
authored
QL: Split up Aggregate properly
Previously, we had `Aggregate` and `ExprAggregate` as separate classes, the latter of which representing aggregates that contain only an expression. This was a problem for the `rank` aggregate, as it inherited from `Aggregate`, but _could_ also contain just an expression (even if this is rather rare). To fix this, I renamed `Aggregate` to `FullAggregate` (to make the division clearer), and added a new type `Aggregate` that represents the union of these two types. Now `Rank` can inherit from the new class `Aggregate` and everything is dandy.
1 parent 8d17a95 commit 76b55c4

3 files changed

Lines changed: 30 additions & 16 deletions

File tree

ql/src/codeql_ql/ast/Ast.qll

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,11 +1392,17 @@ class HigherOrderFormula extends THigherOrderFormula, Formula {
13921392
}
13931393
}
13941394

1395+
class Aggregate extends TAggregate, Expr {
1396+
string getKind() { none() }
1397+
1398+
Generated::Aggregate getAggregate() { none() }
1399+
}
1400+
13951401
/**
13961402
* An aggregate containing an expression.
13971403
* E.g. `min(getAPredicate().getArity())`.
13981404
*/
1399-
class ExprAggregate extends TExprAggregate, Expr {
1405+
class ExprAggregate extends TExprAggregate, Aggregate {
14001406
Generated::Aggregate agg;
14011407
Generated::ExprAggregateBody body;
14021408
string kind;
@@ -1411,7 +1417,9 @@ class ExprAggregate extends TExprAggregate, Expr {
14111417
* Gets the kind of aggregate.
14121418
* E.g. for `min(foo())` the result is "min".
14131419
*/
1414-
string getKind() { result = kind }
1420+
override string getKind() { result = kind }
1421+
1422+
override Generated::Aggregate getAggregate() { result = agg }
14151423

14161424
/**
14171425
* Gets the ith "as" expression of this aggregate, if any.
@@ -1457,13 +1465,13 @@ class ExprAggregate extends TExprAggregate, Expr {
14571465
}
14581466

14591467
/** An aggregate expression, such as `count` or `sum`. */
1460-
class Aggregate extends TAggregate, Expr {
1468+
class FullAggregate extends TFullAggregate, Aggregate {
14611469
Generated::Aggregate agg;
14621470
string kind;
14631471
Generated::FullAggregateBody body;
14641472

1465-
Aggregate() {
1466-
this = TAggregate(agg) and
1473+
FullAggregate() {
1474+
this = TFullAggregate(agg) and
14671475
kind = agg.getChild(0).(Generated::AggId).getValue() and
14681476
body = agg.getChild(_)
14691477
}
@@ -1472,7 +1480,9 @@ class Aggregate extends TAggregate, Expr {
14721480
* Gets the kind of aggregate.
14731481
* E.g. for `min(int i | foo(i))` the result is "foo".
14741482
*/
1475-
string getKind() { result = kind }
1483+
override string getKind() { result = kind }
1484+
1485+
override Generated::Aggregate getAggregate() { result = agg }
14761486

14771487
/** Gets the ith declared argument of this quantifier. */
14781488
VarDecl getArgument(int i) { toGenerated(result) = body.getChild(i) }
@@ -1502,7 +1512,7 @@ class Aggregate extends TAggregate, Expr {
15021512
result = body.getOrderBys().getChild(i).getChild(1).(Generated::Direction).getValue()
15031513
}
15041514

1505-
override string getAPrimaryQlClass() { result = "Aggregate[" + kind + "]" }
1515+
override string getAPrimaryQlClass() { kind != "rank" and result = "FullAggregate[" + kind + "]" }
15061516

15071517
override Type getType() {
15081518
exists(PrimitiveType prim | prim = result |
@@ -1540,14 +1550,14 @@ class Aggregate extends TAggregate, Expr {
15401550
* A "rank" expression, such as `rank[4](int i | i = [5 .. 15] | i)`.
15411551
*/
15421552
class Rank extends Aggregate {
1543-
Rank() { kind = "rank" }
1553+
Rank() { this.getKind() = "rank" }
15441554

15451555
override string getAPrimaryQlClass() { result = "Rank" }
15461556

15471557
/**
15481558
* The `i` in `rank[i]( | | )`.
15491559
*/
1550-
Expr getRankExpr() { toGenerated(result) = agg.getChild(1) }
1560+
Expr getRankExpr() { toGenerated(result) = this.getAggregate().getChild(1) }
15511561

15521562
override AstNode getAChild(string pred) {
15531563
result = super.getAChild(pred)

ql/src/codeql_ql/ast/internal/AstNodes.qll

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ newtype TAstNode =
2121
TComparisonFormula(Generated::CompTerm comp) or
2222
TComparisonOp(Generated::Compop op) or
2323
TQuantifier(Generated::Quantified quant) or
24-
TAggregate(Generated::Aggregate agg) { agg.getChild(_) instanceof Generated::FullAggregateBody } or
24+
TFullAggregate(Generated::Aggregate agg) {
25+
agg.getChild(_) instanceof Generated::FullAggregateBody
26+
} or
2527
TExprAggregate(Generated::Aggregate agg) {
2628
agg.getChild(_) instanceof Generated::ExprAggregateBody
2729
} or
@@ -63,9 +65,11 @@ class TFormula =
6365

6466
class TBinOpExpr = TAddSubExpr or TMulDivModExpr;
6567

68+
class TAggregate = TFullAggregate or TExprAggregate;
69+
6670
class TExpr =
67-
TBinOpExpr or TLiteral or TAggregate or TExprAggregate or TIdentifier or TInlineCast or TCall or
68-
TUnaryExpr or TExprAnnotation or TDontCare or TRange or TSet or TAsExpr or TSuper;
71+
TBinOpExpr or TLiteral or TAggregate or TIdentifier or TInlineCast or
72+
TCall or TUnaryExpr or TExprAnnotation or TDontCare or TRange or TSet or TAsExpr or TSuper;
6973

7074
class TCall = TPredicateCall or TMemberCall or TNoneCall or TAnyCall;
7175

@@ -77,7 +81,7 @@ private Generated::AstNode toGeneratedFormula(AST::AstNode n) {
7781
n = TComparisonFormula(result) or
7882
n = TComparisonOp(result) or
7983
n = TQuantifier(result) or
80-
n = TAggregate(result) or
84+
n = TFullAggregate(result) or
8185
n = TIdentifier(result) or
8286
n = TNegation(result) or
8387
n = TIfFormula(result) or
@@ -94,7 +98,7 @@ private Generated::AstNode toGeneratedExpr(AST::AstNode n) {
9498
n = TSet(result) or
9599
n = TExprAnnotation(result) or
96100
n = TLiteral(result) or
97-
n = TAggregate(result) or
101+
n = TFullAggregate(result) or
98102
n = TExprAggregate(result) or
99103
n = TIdentifier(result) or
100104
n = TUnaryExpr(result) or

ql/src/codeql_ql/ast/internal/Variable.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import ql
22
import codeql_ql.ast.internal.AstNodes
33

44
private class TScope =
5-
TClass or TAggregate or TExprAggregate or TQuantifier or TSelect or TPredicate or TNewTypeBranch;
5+
TClass or TFullAggregate or TExprAggregate or TQuantifier or TSelect or TPredicate or TNewTypeBranch;
66

77
/** A variable scope. */
88
class VariableScope extends TScope, AstNode {
@@ -24,7 +24,7 @@ class VariableScope extends TScope, AstNode {
2424
or
2525
decl = this.(Select).getExpr(_).(AsExpr)
2626
or
27-
decl = this.(Aggregate).getExpr(_).(AsExpr)
27+
decl = this.(FullAggregate).getExpr(_).(AsExpr)
2828
or
2929
decl = this.(ExprAggregate).getExpr(_).(AsExpr)
3030
or

0 commit comments

Comments
 (0)