Skip to content

Commit d4e4984

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Invert the nesting level sequencing when mangling comprehension identifiers
PiperOrigin-RevId: 868859295
1 parent b3ee6a6 commit d4e4984

19 files changed

+2453
-1126
lines changed

‎optimizer/src/main/java/dev/cel/optimizer/AstMutator.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,16 +404,23 @@ private static MangledComprehensionName getMangledComprehensionName(
404404
}
405405

406406
private static int countComprehensionNestingLevel(CelNavigableMutableExpr comprehensionExpr) {
407-
int nestedLevel = 0;
408-
Optional<CelNavigableMutableExpr> maybeParent = comprehensionExpr.parent();
409-
while (maybeParent.isPresent()) {
410-
if (maybeParent.get().getKind().equals(Kind.COMPREHENSION)) {
411-
nestedLevel++;
412-
}
413-
414-
maybeParent = maybeParent.get().parent();
415-
}
416-
return nestedLevel;
407+
return comprehensionExpr
408+
.descendants()
409+
.filter(node -> node.getKind().equals(Kind.COMPREHENSION))
410+
.mapToInt(
411+
node -> {
412+
int nestedLevel = 1;
413+
CelNavigableMutableExpr maybeParent = node.parent().orElse(null);
414+
while (maybeParent != null && maybeParent.id() != comprehensionExpr.id()) {
415+
if (maybeParent.getKind().equals(Kind.COMPREHENSION)) {
416+
nestedLevel++;
417+
}
418+
maybeParent = maybeParent.parent().orElse(null);
419+
}
420+
return nestedLevel;
421+
})
422+
.max()
423+
.orElse(0);
417424
}
418425

419426
/**

‎optimizer/src/test/java/dev/cel/optimizer/AstMutatorTest.java‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -807,8 +807,8 @@ public void mangleComprehensionVariable_adjacentMacros_sameIterVarTypes() throws
807807

808808
assertThat(CEL_UNPARSER.unparse(mangledAst))
809809
.isEqualTo(
810-
"[1, 2, 3].map(@it:0:0, [1, 2, 3].map(@it:1:0, @it:1:0 + 1)) == "
811-
+ "[1, 2, 3].map(@it:0:0, [1, 2, 3].map(@it:1:0, @it:1:0 + 1))");
810+
"[1, 2, 3].map(@it:1:0, [1, 2, 3].map(@it:0:0, @it:0:0 + 1)) == "
811+
+ "[1, 2, 3].map(@it:1:0, [1, 2, 3].map(@it:0:0, @it:0:0 + 1))");
812812
assertThat(CEL.createProgram(CEL.check(mangledAst).getAst()).eval()).isEqualTo(true);
813813
assertConsistentMacroCalls(ast);
814814
}
@@ -831,9 +831,9 @@ public void mangleComprehensionVariable_withTwoIterVars_adjacentMacros_sameIterV
831831

832832
assertThat(CEL_UNPARSER.unparse(mangledAst))
833833
.isEqualTo(
834-
"[1, 2].transformMap(@it:0:0, @it2:0:0, [1, 2].transformMap(@it:1:0, @it2:1:0,"
835-
+ " @it:1:0)) == [1, 2].transformMap(@it:0:0, @it2:0:0, [1,"
836-
+ " 2].transformMap(@it:1:0, @it2:1:0, @it:1:0))");
834+
"[1, 2].transformMap(@it:1:0, @it2:1:0, [1, 2].transformMap(@it:0:0, @it2:0:0,"
835+
+ " @it:0:0)) == [1, 2].transformMap(@it:1:0, @it2:1:0, [1,"
836+
+ " 2].transformMap(@it:0:0, @it2:0:0, @it:0:0))");
837837
assertThat(CEL.createProgram(CEL.check(mangledAst).getAst()).eval()).isEqualTo(true);
838838
assertConsistentMacroCalls(ast);
839839
}
@@ -854,8 +854,8 @@ public void mangleComprehensionVariable_adjacentMacros_differentIterVarTypes() t
854854

855855
assertThat(CEL_UNPARSER.unparse(mangledAst))
856856
.isEqualTo(
857-
"[1, 2, 3].map(@it:0:0, [1, 2, 3].map(@it:1:0, @it:1:0)) == "
858-
+ "dyn([1u, 2u, 3u].map(@it:0:1, [1u, 2u, 3u].map(@it:1:1, @it:1:1)))");
857+
"[1, 2, 3].map(@it:1:0, [1, 2, 3].map(@it:0:0, @it:0:0)) == "
858+
+ "dyn([1u, 2u, 3u].map(@it:1:1, [1u, 2u, 3u].map(@it:0:1, @it:0:1)))");
859859
assertThat(CEL.createProgram(CEL.check(mangledAst).getAst()).eval()).isEqualTo(true);
860860
assertConsistentMacroCalls(ast);
861861
}
@@ -892,7 +892,7 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
892892
assertThat(mangledAst.getExpr().toString())
893893
.isEqualTo(
894894
"COMPREHENSION [27] {\n"
895-
+ " iter_var: @it:0:0\n"
895+
+ " iter_var: @it:1:0\n"
896896
+ " iter_range: {\n"
897897
+ " LIST [1] {\n"
898898
+ " elements: {\n"
@@ -902,7 +902,7 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
902902
+ " }\n"
903903
+ " }\n"
904904
+ " }\n"
905-
+ " accu_var: @ac:0:0\n"
905+
+ " accu_var: @ac:1:0\n"
906906
+ " accu_init: {\n"
907907
+ " CONSTANT [20] { value: false }\n"
908908
+ " }\n"
@@ -914,7 +914,7 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
914914
+ " function: !_\n"
915915
+ " args: {\n"
916916
+ " IDENT [21] {\n"
917-
+ " name: @ac:0:0\n"
917+
+ " name: @ac:1:0\n"
918918
+ " }\n"
919919
+ " }\n"
920920
+ " }\n"
@@ -926,20 +926,20 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
926926
+ " function: _||_\n"
927927
+ " args: {\n"
928928
+ " IDENT [24] {\n"
929-
+ " name: @ac:0:0\n"
929+
+ " name: @ac:1:0\n"
930930
+ " }\n"
931931
+ " COMPREHENSION [19] {\n"
932-
+ " iter_var: @it:1:0\n"
932+
+ " iter_var: @it:0:0\n"
933933
+ " iter_range: {\n"
934934
+ " LIST [5] {\n"
935935
+ " elements: {\n"
936936
+ " IDENT [6] {\n"
937-
+ " name: @it:0:0\n"
937+
+ " name: @it:1:0\n"
938938
+ " }\n"
939939
+ " }\n"
940940
+ " }\n"
941941
+ " }\n"
942-
+ " accu_var: @ac:1:0\n"
942+
+ " accu_var: @ac:0:0\n"
943943
+ " accu_init: {\n"
944944
+ " CONSTANT [12] { value: false }\n"
945945
+ " }\n"
@@ -951,7 +951,7 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
951951
+ " function: !_\n"
952952
+ " args: {\n"
953953
+ " IDENT [13] {\n"
954-
+ " name: @ac:1:0\n"
954+
+ " name: @ac:0:0\n"
955955
+ " }\n"
956956
+ " }\n"
957957
+ " }\n"
@@ -963,13 +963,13 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
963963
+ " function: _||_\n"
964964
+ " args: {\n"
965965
+ " IDENT [16] {\n"
966-
+ " name: @ac:1:0\n"
966+
+ " name: @ac:0:0\n"
967967
+ " }\n"
968968
+ " CALL [10] {\n"
969969
+ " function: _==_\n"
970970
+ " args: {\n"
971971
+ " IDENT [9] {\n"
972-
+ " name: @it:1:0\n"
972+
+ " name: @it:0:0\n"
973973
+ " }\n"
974974
+ " CONSTANT [11] { value: 1 }\n"
975975
+ " }\n"
@@ -979,7 +979,7 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
979979
+ " }\n"
980980
+ " result: {\n"
981981
+ " IDENT [18] {\n"
982-
+ " name: @ac:1:0\n"
982+
+ " name: @ac:0:0\n"
983983
+ " }\n"
984984
+ " }\n"
985985
+ " }\n"
@@ -988,12 +988,12 @@ public void mangleComprehensionVariable_nestedMacroWithShadowedVariables() throw
988988
+ " }\n"
989989
+ " result: {\n"
990990
+ " IDENT [26] {\n"
991-
+ " name: @ac:0:0\n"
991+
+ " name: @ac:1:0\n"
992992
+ " }\n"
993993
+ " }\n"
994994
+ "}");
995995
assertThat(CEL_UNPARSER.unparse(mangledAst))
996-
.isEqualTo("[x].exists(@it:0:0, [@it:0:0].exists(@it:1:0, @it:1:0 == 1))");
996+
.isEqualTo("[x].exists(@it:1:0, [@it:1:0].exists(@it:0:0, @it:0:0 == 1))");
997997
assertThat(CEL.createProgram(CEL.check(mangledAst).getAst()).eval(ImmutableMap.of("x", 1)))
998998
.isEqualTo(true);
999999
assertConsistentMacroCalls(ast);

‎optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerBaselineTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ private enum CseTestCase {
415415
+ "[1].exists(k, z, k > 1 && z > 0) && [2].exists(l, w, l > 1 && w > 0)"),
416416
NESTED_MACROS("[1,2,3].map(i, [1, 2, 3].map(i, i + 1)) == [[2, 3, 4], [2, 3, 4], [2, 3, 4]]"),
417417
NESTED_MACROS_2("[1, 2].map(y, [1, 2, 3].filter(x, x == y)) == [[1], [2]]"),
418+
NESTED_MACROS_3("[1,2,3].map(i, i * 2) + [1,2,3].map(i, i * 2).map(i, i * 2)"),
418419
NESTED_MACROS_COMP_V2_1(
419420
"[1,2,3].transformList(i, v, [1, 2, 3].transformList(i, v, i + v + 1)) == "
420421
+ "[[2, 4, 6], [2, 4, 6], [2, 4, 6]]"),

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,21 @@ Result: true
358358
[BLOCK_RECURSION_DEPTH_8]: true
359359
[BLOCK_RECURSION_DEPTH_9]: true
360360

361+
Test case: NESTED_MACROS_3
362+
Source: [1,2,3].map(i, i * 2) + [1,2,3].map(i, i * 2).map(i, i * 2)
363+
=====>
364+
Result: [2, 4, 6, 4, 8, 12]
365+
[BLOCK_COMMON_SUBEXPR_ONLY]: [2, 4, 6, 4, 8, 12]
366+
[BLOCK_RECURSION_DEPTH_1]: [2, 4, 6, 4, 8, 12]
367+
[BLOCK_RECURSION_DEPTH_2]: [2, 4, 6, 4, 8, 12]
368+
[BLOCK_RECURSION_DEPTH_3]: [2, 4, 6, 4, 8, 12]
369+
[BLOCK_RECURSION_DEPTH_4]: [2, 4, 6, 4, 8, 12]
370+
[BLOCK_RECURSION_DEPTH_5]: [2, 4, 6, 4, 8, 12]
371+
[BLOCK_RECURSION_DEPTH_6]: [2, 4, 6, 4, 8, 12]
372+
[BLOCK_RECURSION_DEPTH_7]: [2, 4, 6, 4, 8, 12]
373+
[BLOCK_RECURSION_DEPTH_8]: [2, 4, 6, 4, 8, 12]
374+
[BLOCK_RECURSION_DEPTH_9]: [2, 4, 6, 4, 8, 12]
375+
361376
Test case: NESTED_MACROS_COMP_V2_1
362377
Source: [1,2,3].transformList(i, v, [1, 2, 3].transformList(i, v, i + v + 1)) == [[2, 4, 6], [2, 4, 6], [2, 4, 6]]
363378
=====>

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

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)