Skip to content

Commit 95cb753

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Fully perform CSE on presence tests where possible
PiperOrigin-RevId: 627071810
1 parent e694255 commit 95cb753

14 files changed

Lines changed: 392 additions & 365 deletions

‎optimizer/src/main/java/dev/cel/optimizer/optimizers/SubexpressionOptimizer.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,8 +555,6 @@ private boolean canEliminate(CelNavigableMutableExpr navigableExpr) {
555555
&& !navigableExpr.getKind().equals(Kind.IDENT)
556556
&& !(navigableExpr.getKind().equals(Kind.IDENT)
557557
&& navigableExpr.expr().ident().name().startsWith(BIND_IDENTIFIER_PREFIX))
558-
&& !(navigableExpr.getKind().equals(Kind.SELECT)
559-
&& navigableExpr.expr().select().testOnly())
560558
&& containsEliminableFunctionOnly(navigableExpr)
561559
&& isWithinInlineableComprehension(navigableExpr);
562560
}

‎optimizer/src/test/resources/constfold_before_subexpression_unparsed.baseline‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ Source: (has(msg.oneof_type.payload) ? msg.oneof_type.payload.single_int64 : 0)
452452
Result: true
453453
[CASCADED_BINDS]: cel.bind(@r0, msg.oneof_type, has(@r0.payload) ? @r0.payload.single_int64 : 0) == 10
454454
[BLOCK_COMMON_SUBEXPR_ONLY]: cel.@block([msg.oneof_type], (has(@index0.payload) ? @index0.payload.single_int64 : 0) == 10)
455-
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.single_int64], (has(@index0.payload) ? @index2 : 0) == 10)
455+
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, has(@index0.payload), @index0.payload, @index2.single_int64, @index1 ? @index3 : 0], @index4 == 10)
456456
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type, @index0.payload.single_int64, has(@index0.payload) ? @index1 : 0], @index2 == 10)
457457
[BLOCK_RECURSION_DEPTH_3]: cel.@block([msg.oneof_type, has(@index0.payload) ? @index0.payload.single_int64 : 0], @index1 == 10)
458458
[BLOCK_RECURSION_DEPTH_4]: cel.@block([msg.oneof_type], (has(@index0.payload) ? @index0.payload.single_int64 : 0) == 10)
@@ -468,8 +468,8 @@ Source: (has(msg.oneof_type.payload) ? msg.oneof_type.payload.single_int64 : msg
468468
Result: true
469469
[CASCADED_BINDS]: cel.bind(@r0, msg.oneof_type, cel.bind(@r1, @r0.payload.single_int64, has(@r0.payload) ? @r1 : (@r1 * 0))) == 10
470470
[BLOCK_COMMON_SUBEXPR_ONLY]: cel.@block([msg.oneof_type, @index0.payload.single_int64], (has(@index0.payload) ? @index1 : (@index1 * 0)) == 10)
471-
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.single_int64, @index2 * 0], (has(@index0.payload) ? @index2 : @index3) == 10)
472-
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type.payload, @index0.single_int64, msg.oneof_type, has(@index2.payload) ? @index1 : (@index1 * 0)], @index3 == 10)
471+
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.single_int64, has(@index0.payload), @index2 * 0, @index3 ? @index2 : @index4], @index5 == 10)
472+
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type.payload, @index0.single_int64, has(msg.oneof_type.payload), @index2 ? @index1 : (@index1 * 0)], @index3 == 10)
473473
[BLOCK_RECURSION_DEPTH_3]: cel.@block([msg.oneof_type.payload.single_int64, has(msg.oneof_type.payload) ? @index0 : (@index0 * 0)], @index1 == 10)
474474
[BLOCK_RECURSION_DEPTH_4]: cel.@block([msg.oneof_type.payload.single_int64], (has(msg.oneof_type.payload) ? @index0 : (@index0 * 0)) == 10)
475475
[BLOCK_RECURSION_DEPTH_5]: cel.@block([msg.oneof_type.payload.single_int64], (has(msg.oneof_type.payload) ? @index0 : (@index0 * 0)) == 10)
@@ -484,9 +484,9 @@ Source: (has(msg.oneof_type.payload.single_int64) ? msg.oneof_type.payload.singl
484484
Result: true
485485
[CASCADED_BINDS]: cel.bind(@r0, msg.oneof_type.payload, cel.bind(@r1, @r0.single_int64, has(@r0.single_int64) ? @r1 : (@r1 * 0))) == 10
486486
[BLOCK_COMMON_SUBEXPR_ONLY]: cel.@block([msg.oneof_type.payload, @index0.single_int64], (has(@index0.single_int64) ? @index1 : (@index1 * 0)) == 10)
487-
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.single_int64, @index2 * 0], (has(@index1.single_int64) ? @index2 : @index3) == 10)
487+
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.single_int64, has(@index1.single_int64), @index2 * 0, @index3 ? @index2 : @index4], @index5 == 10)
488488
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type.payload, @index0.single_int64, has(@index0.single_int64) ? @index1 : (@index1 * 0)], @index2 == 10)
489-
[BLOCK_RECURSION_DEPTH_3]: cel.@block([msg.oneof_type.payload.single_int64, msg.oneof_type.payload], (has(@index1.single_int64) ? @index0 : (@index0 * 0)) == 10)
489+
[BLOCK_RECURSION_DEPTH_3]: cel.@block([msg.oneof_type.payload.single_int64, has(msg.oneof_type.payload.single_int64)], (@index1 ? @index0 : (@index0 * 0)) == 10)
490490
[BLOCK_RECURSION_DEPTH_4]: cel.@block([msg.oneof_type.payload.single_int64, has(msg.oneof_type.payload.single_int64) ? @index0 : (@index0 * 0)], @index1 == 10)
491491
[BLOCK_RECURSION_DEPTH_5]: cel.@block([msg.oneof_type.payload.single_int64], (has(msg.oneof_type.payload.single_int64) ? @index0 : (@index0 * 0)) == 10)
492492
[BLOCK_RECURSION_DEPTH_6]: cel.@block([msg.oneof_type.payload.single_int64], (has(msg.oneof_type.payload.single_int64) ? @index0 : (@index0 * 0)) == 10)
@@ -500,8 +500,8 @@ Source: (has(msg.oneof_type) && has(msg.oneof_type.payload) && has(msg.oneof_typ
500500
Result: true
501501
[CASCADED_BINDS]: cel.bind(@r0, msg.oneof_type, cel.bind(@r1, @r0.payload, (has(msg.oneof_type) && has(@r0.payload) && has(@r1.single_int64)) ? cel.bind(@r2, @r1.map_string_string, (has(@r1.map_string_string) && has(@r2.key)) ? (@r2.key == "A") : false) : false))
502502
[BLOCK_COMMON_SUBEXPR_ONLY]: cel.@block([msg.oneof_type, @index0.payload, @index1.map_string_string], (has(msg.oneof_type) && has(@index0.payload) && has(@index1.single_int64)) ? ((has(@index1.map_string_string) && has(@index2.key)) ? (@index2.key == "A") : false) : false)
503-
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.map_string_string, @index2.key, @index3 == "A"], (has(msg.oneof_type) && has(@index0.payload) && has(@index1.single_int64)) ? ((has(@index1.map_string_string) && has(@index2.key)) ? @index4 : false) : false)
504-
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type.payload, @index0.map_string_string, has(@index0.map_string_string) && has(@index1.key), @index1.key == "A", msg.oneof_type, has(msg.oneof_type) && has(@index4.payload), @index5 && has(@index0.single_int64)], @index6 ? (@index2 ? @index3 : false) : false)
503+
[BLOCK_RECURSION_DEPTH_1]: cel.@block([msg.oneof_type, @index0.payload, @index1.map_string_string, has(msg.oneof_type), has(@index0.payload), @index3 && @index4, has(@index1.single_int64), @index5 && @index6, has(@index1.map_string_string), has(@index2.key), @index8 && @index9, @index2.key, @index11 == "A", @index10 ? @index12 : false], @index7 ? @index13 : false)
504+
[BLOCK_RECURSION_DEPTH_2]: cel.@block([msg.oneof_type.payload, @index0.map_string_string, has(msg.oneof_type.payload), has(msg.oneof_type) && @index2, @index3 && has(@index0.single_int64), has(@index0.map_string_string) && has(@index1.key), @index1.key == "A"], @index4 ? (@index5 ? @index6 : false) : false)
505505
[BLOCK_RECURSION_DEPTH_3]: cel.@block([msg.oneof_type.payload.map_string_string, msg.oneof_type.payload, has(msg.oneof_type) && has(msg.oneof_type.payload), (has(@index1.map_string_string) && has(@index0.key)) ? (@index0.key == "A") : false], (@index2 && has(@index1.single_int64)) ? @index3 : false)
506506
[BLOCK_RECURSION_DEPTH_4]: cel.@block([msg.oneof_type.payload.map_string_string, msg.oneof_type.payload, has(msg.oneof_type) && has(msg.oneof_type.payload) && has(@index1.single_int64)], @index2 ? ((has(@index1.map_string_string) && has(@index0.key)) ? (@index0.key == "A") : false) : false)
507507
[BLOCK_RECURSION_DEPTH_5]: cel.@block([msg.oneof_type.payload.map_string_string, msg.oneof_type.payload], (has(msg.oneof_type) && has(msg.oneof_type.payload) && has(@index1.single_int64)) ? ((has(@index1.map_string_string) && has(@index0.key)) ? (@index0.key == "A") : false) : false)

‎optimizer/src/test/resources/subexpression_ast_block_common_subexpr_only.baseline‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,30 +2236,28 @@ CALL [1] {
22362236
args: {
22372237
CREATE_LIST [2] {
22382238
elements: {
2239-
CREATE_MAP [3] {
2240-
MAP_ENTRY [4] {
2241-
key: {
2242-
CONSTANT [5] { value: "a" }
2243-
}
2244-
value: {
2245-
CONSTANT [6] { value: true }
2239+
SELECT [3] {
2240+
CREATE_MAP [4] {
2241+
MAP_ENTRY [5] {
2242+
key: {
2243+
CONSTANT [6] { value: "a" }
2244+
}
2245+
value: {
2246+
CONSTANT [7] { value: true }
2247+
}
22462248
}
2247-
}
2249+
}.a~presence_test
22482250
}
22492251
}
22502252
}
2251-
CALL [7] {
2253+
CALL [8] {
22522254
function: _&&_
22532255
args: {
2254-
SELECT [8] {
2255-
IDENT [9] {
2256-
name: @index0
2257-
}.a~presence_test
2256+
IDENT [9] {
2257+
name: @index0
22582258
}
2259-
SELECT [10] {
2260-
IDENT [11] {
2261-
name: @index0
2262-
}.a~presence_test
2259+
IDENT [10] {
2260+
name: @index0
22632261
}
22642262
}
22652263
}

0 commit comments

Comments
 (0)