Skip to content

Commit 237b499

Browse files
CEL Dev Teamcopybara-github
authored andcommitted
Internal Change
PiperOrigin-RevId: 934764659
1 parent bb0b958 commit 237b499

3 files changed

Lines changed: 22 additions & 12 deletions

File tree

optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ java_library(
2828
"//common/ast",
2929
"//common/ast:mutable_expr",
3030
"//common/internal:date_time_helpers",
31+
"//common/navigation:common",
3132
"//common/navigation:mutable_navigation",
3233
"//common/types",
3334
"//extensions:optional_library",

optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import dev.cel.common.internal.DateTimeHelpers;
4242
import dev.cel.common.navigation.CelNavigableMutableAst;
4343
import dev.cel.common.navigation.CelNavigableMutableExpr;
44+
import dev.cel.common.navigation.TraversalOrder;
4445
import dev.cel.common.types.SimpleType;
4546
import dev.cel.extensions.CelOptionalLibrary.Function;
4647
import dev.cel.optimizer.AstMutator;
@@ -111,7 +112,7 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel)
111112
ImmutableList<CelNavigableMutableExpr> foldableExprs =
112113
CelNavigableMutableAst.fromAst(mutableAst)
113114
.getRoot()
114-
.allNodes()
115+
.allNodes(TraversalOrder.PRE_ORDER)
115116
.filter(this::canFold)
116117
.collect(toImmutableList());
117118
for (CelNavigableMutableExpr foldableExpr : foldableExprs) {
@@ -122,7 +123,13 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel)
122123
mutatedResult = maybePruneBranches(mutableAst, foldableExpr.expr());
123124
if (!mutatedResult.isPresent()) {
124125
// Evaluate the call then fold
125-
mutatedResult = maybeFold(optimizerEnv, mutableAst, foldableExpr);
126+
try {
127+
mutatedResult = maybeFold(optimizerEnv, mutableAst, foldableExpr);
128+
} catch (CelEvaluationException e) {
129+
throw new CelOptimizationException(
130+
"Constant folding failure. Failed to evaluate subtree due to: " + e.getMessage(),
131+
e);
132+
}
126133
}
127134

128135
if (!mutatedResult.isPresent()) {
@@ -132,12 +139,17 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel)
132139

133140
continueFolding = true;
134141
mutableAst = mutatedResult.get();
142+
// Break the loop because we mutated the AST. Since we traverse in PRE_ORDER (top-down),
143+
// mutating a parent node means its children are now obsolete or folded.
144+
// We restart the traversal to gather a fresh list of foldable expressions.
145+
break;
135146
}
136147
}
137148

138149
// If the output is a list, map, or struct which contains optional entries, then prune it
139150
// to make sure that the optionals, if resolved, do not surface in the output literal.
140151
mutableAst = pruneOptionalElements(mutableAst);
152+
141153
return OptimizationResult.create(astMutator.renumberIdsConsecutively(mutableAst).toParsedAst());
142154
}
143155

@@ -280,11 +292,11 @@ private static boolean isNestedComprehension(CelNavigableMutableExpr expr) {
280292

281293
private Optional<CelMutableAst> maybeFold(
282294
Cel cel, CelMutableAst mutableAst, CelNavigableMutableExpr node)
283-
throws CelOptimizationException {
295+
throws CelOptimizationException, CelEvaluationException {
284296
Object result;
285297
try {
286298
result = evaluateExpr(cel, node);
287-
} catch (CelValidationException | CelEvaluationException e) {
299+
} catch (CelValidationException e) {
288300
throw new CelOptimizationException(
289301
"Constant folding failure. Failed to evaluate subtree due to: " + e.getMessage(), e);
290302
}

optimizer/src/test/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizerTest.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ private static Cel setupEnv(CelBuilder celBuilder) {
252252
@TestParameters(
253253
"{source: 'timestamp(\"2000-01-01T00:02:03.2123Z\") + duration(\"25h2m32s42ms53us29ns\")',"
254254
+ " expected: 'timestamp(\"2000-01-02T01:04:35.254353029Z\")'}")
255+
@TestParameters(
256+
"{source: 'has({\"req\": \"Avail\"}.opt) ? ({\"req\": \"Avail\"}.req + \" \" +"
257+
+ " {\"req\": \"Avail\"}.opt) : {\"req\": \"Avail\"}.req', expected: '\"Avail\"'}")
255258
// TODO: Support folding lists with mixed types. This requires mutable lists.
256259
// @TestParameters("{source: 'dyn([1]) + [1.0]'}")
257260
public void constantFold_success(String source, String expected) throws Exception {
@@ -534,26 +537,20 @@ public void constantFold_astProducesConsistentlyNumberedIds() throws Exception {
534537

535538
@Test
536539
public void iterationLimitReached_throws() throws Exception {
537-
StringBuilder sb = new StringBuilder();
538-
sb.append("0");
539-
for (int i = 1; i < 200; i++) {
540-
sb.append(" + ").append(i);
541-
} // 0 + 1 + 2 + 3 + ... 200
542540
Cel cel =
543541
runtimeFlavor
544542
.builder()
545543
.setOptions(
546544
CelOptions.current()
547545
.enableHeterogeneousNumericComparisons(true)
548-
.maxParseRecursionDepth(200)
549546
.build())
550547
.build();
551-
CelAbstractSyntaxTree ast = cel.compile(sb.toString()).getAst();
548+
CelAbstractSyntaxTree ast = cel.compile("1 + 1").getAst();
552549
CelOptimizer optimizer =
553550
CelOptimizerFactory.standardCelOptimizerBuilder(cel)
554551
.addAstOptimizers(
555552
ConstantFoldingOptimizer.newInstance(
556-
ConstantFoldingOptions.newBuilder().maxIterationLimit(200).build()))
553+
ConstantFoldingOptions.newBuilder().maxIterationLimit(1).build()))
557554
.build();
558555

559556
CelOptimizationException e =

0 commit comments

Comments
 (0)