Skip to content

Commit 1db2c59

Browse files
l46kokcopybara-github
authored andcommitted
Fold constant list and map literals into constants at plan time
PiperOrigin-RevId: 1005908785
1 parent 06bdd22 commit 1db2c59

6 files changed

Lines changed: 146 additions & 38 deletions

File tree

‎runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ java_library(
170170
name = "eval_create_list",
171171
srcs = ["EvalCreateList.java"],
172172
deps = [
173+
":eval_const",
173174
":eval_helpers",
174175
":planned_interpretable",
175176
"//common/ast",
@@ -184,14 +185,14 @@ java_library(
184185
name = "eval_create_map",
185186
srcs = ["EvalCreateMap.java"],
186187
deps = [
188+
":eval_const",
187189
":eval_helpers",
188190
":localized_evaluation_exception",
189191
":planned_interpretable",
190192
"//common/ast",
191193
"//common/exceptions:duplicate_key",
192194
"//common/exceptions:invalid_argument",
193195
"//runtime:accumulated_unknowns",
194-
"//runtime:evaluation_exception",
195196
"//runtime:interpretable",
196197
"@maven//:com_google_errorprone_error_prone_annotations",
197198
"@maven//:com_google_guava_guava",
@@ -840,6 +841,7 @@ cel_android_library(
840841
name = "eval_create_list_android",
841842
srcs = ["EvalCreateList.java"],
842843
deps = [
844+
":eval_const_android",
843845
":eval_helpers_android",
844846
":planned_interpretable_android",
845847
"//common/ast:ast_android",
@@ -854,13 +856,13 @@ cel_android_library(
854856
name = "eval_create_map_android",
855857
srcs = ["EvalCreateMap.java"],
856858
deps = [
859+
":eval_const_android",
857860
":eval_helpers_android",
858861
":localized_evaluation_exception_android",
859862
":planned_interpretable_android",
860863
"//common/ast:ast_android",
861864
"//common/exceptions:duplicate_key",
862865
"//common/exceptions:invalid_argument",
863-
"//runtime:evaluation_exception",
864866
"//runtime:interpretable_android",
865867
"//runtime/src/main/java/dev/cel/runtime:accumulated_unknowns_android",
866868
"@maven//:com_google_errorprone_error_prone_annotations",

‎runtime/src/main/java/dev/cel/runtime/planner/EvalCreateList.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,18 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) {
6666
return builder.build();
6767
}
6868

69-
static EvalCreateList create(CelExpr expr, PlannedInterpretable[] values, boolean[] isOptional) {
70-
return new EvalCreateList(expr, values, isOptional);
69+
static PlannedInterpretable create(
70+
CelExpr expr, PlannedInterpretable[] values, boolean[] isOptional) {
71+
for (int i = 0; i < values.length; i++) {
72+
if (!(values[i] instanceof EvalConstant) || isOptional[i]) {
73+
return new EvalCreateList(expr, values, isOptional);
74+
}
75+
}
76+
ImmutableList.Builder<Object> builder = ImmutableList.builderWithExpectedSize(values.length);
77+
for (PlannedInterpretable value : values) {
78+
builder.add(((EvalConstant) value).constantValue());
79+
}
80+
return EvalConstant.create(expr, builder.build());
7181
}
7282

7383
private EvalCreateList(CelExpr expr, PlannedInterpretable[] values, boolean[] isOptional) {

‎runtime/src/main/java/dev/cel/runtime/planner/EvalCreateMap.java‎

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414

1515
package dev.cel.runtime.planner;
1616

17-
import com.google.common.base.Preconditions;
17+
import static com.google.common.base.Preconditions.checkArgument;
18+
1819
import com.google.common.collect.ImmutableMap;
1920
import com.google.common.collect.Sets;
2021
import com.google.common.primitives.UnsignedLong;
@@ -23,10 +24,10 @@
2324
import dev.cel.common.exceptions.CelDuplicateKeyException;
2425
import dev.cel.common.exceptions.CelInvalidArgumentException;
2526
import dev.cel.runtime.AccumulatedUnknowns;
26-
import dev.cel.runtime.CelEvaluationException;
2727
import dev.cel.runtime.GlobalResolver;
2828
import java.util.HashSet;
2929
import java.util.Optional;
30+
import java.util.Set;
3031

3132
@Immutable
3233
final class EvalCreateMap extends PlannedInterpretable {
@@ -44,45 +45,20 @@ final class EvalCreateMap extends PlannedInterpretable {
4445
private final boolean[] isOptional;
4546

4647
@Override
47-
Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException {
48+
Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) {
4849
ImmutableMap.Builder<Object, Object> builder =
4950
ImmutableMap.builderWithExpectedSize(keys.length);
5051
HashSet<Object> keysSeen = Sets.newHashSetWithExpectedSize(keys.length);
5152
AccumulatedUnknowns unknowns = null;
5253

5354
for (int i = 0; i < keys.length; i++) {
5455
PlannedInterpretable keyInterpretable = keys[i];
55-
Object key = keyInterpretable.eval(resolver, frame);
56+
Object key = EvalHelpers.evalStrictly(keyInterpretable, resolver, frame);
5657

5758
if (key instanceof AccumulatedUnknowns) {
5859
unknowns = AccumulatedUnknowns.maybeMerge(unknowns, key);
5960
} else {
60-
if (!(key instanceof String
61-
|| key instanceof Long
62-
|| key instanceof UnsignedLong
63-
|| key instanceof Boolean)) {
64-
throw new LocalizedEvaluationException(
65-
new CelInvalidArgumentException("Unsupported key type: " + key),
66-
keyInterpretable.expr().id());
67-
}
68-
69-
boolean isDuplicate = !keysSeen.add(key);
70-
if (!isDuplicate) {
71-
if (key instanceof Long) {
72-
long longVal = (Long) key;
73-
if (longVal >= 0) {
74-
isDuplicate = keysSeen.contains(UnsignedLong.valueOf(longVal));
75-
}
76-
} else if (key instanceof UnsignedLong) {
77-
UnsignedLong ulongVal = (UnsignedLong) key;
78-
isDuplicate = keysSeen.contains(ulongVal.longValue());
79-
}
80-
}
81-
82-
if (isDuplicate) {
83-
throw new LocalizedEvaluationException(
84-
CelDuplicateKeyException.of(key), keyInterpretable.expr().id());
85-
}
61+
validateAndTrackKey(keysSeen, keyInterpretable, key);
8662
}
8763

8864
Object val = EvalHelpers.evalStrictly(values[i], resolver, frame);
@@ -119,12 +95,61 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEva
11995
return builder.buildOrThrow();
12096
}
12197

122-
static EvalCreateMap create(
98+
private static void validateAndTrackKey(
99+
Set<Object> keysSeen, PlannedInterpretable keyInterpretable, Object key) {
100+
if (!(key instanceof String
101+
|| key instanceof Long
102+
|| key instanceof UnsignedLong
103+
|| key instanceof Boolean)) {
104+
throw new LocalizedEvaluationException(
105+
new CelInvalidArgumentException("Unsupported key type: " + key),
106+
keyInterpretable.expr().id());
107+
}
108+
109+
boolean isDuplicate = !keysSeen.add(key);
110+
if (!isDuplicate) {
111+
if (key instanceof Long) {
112+
long longVal = (Long) key;
113+
if (longVal >= 0) {
114+
isDuplicate = keysSeen.contains(UnsignedLong.valueOf(longVal));
115+
}
116+
} else if (key instanceof UnsignedLong) {
117+
UnsignedLong ulongVal = (UnsignedLong) key;
118+
isDuplicate = keysSeen.contains(ulongVal.longValue());
119+
}
120+
}
121+
122+
if (isDuplicate) {
123+
throw new LocalizedEvaluationException(
124+
CelDuplicateKeyException.of(key), keyInterpretable.expr().id());
125+
}
126+
}
127+
128+
static PlannedInterpretable create(
123129
CelExpr expr,
124130
PlannedInterpretable[] keys,
125131
PlannedInterpretable[] values,
126132
boolean[] isOptional) {
127-
return new EvalCreateMap(expr, keys, values, isOptional);
133+
for (int i = 0; i < keys.length; i++) {
134+
if (!(keys[i] instanceof EvalConstant)
135+
|| !(values[i] instanceof EvalConstant)
136+
|| isOptional[i]) {
137+
return new EvalCreateMap(expr, keys, values, isOptional);
138+
}
139+
}
140+
ImmutableMap.Builder<Object, Object> builder =
141+
ImmutableMap.builderWithExpectedSize(keys.length);
142+
HashSet<Object> keysSeen = Sets.newHashSetWithExpectedSize(keys.length);
143+
try {
144+
for (int i = 0; i < keys.length; i++) {
145+
Object key = ((EvalConstant) keys[i]).constantValue();
146+
validateAndTrackKey(keysSeen, keys[i], key);
147+
builder.put(key, ((EvalConstant) values[i]).constantValue());
148+
}
149+
} catch (LocalizedEvaluationException e) {
150+
return new EvalCreateMap(expr, keys, values, isOptional);
151+
}
152+
return EvalConstant.create(expr, builder.buildOrThrow());
128153
}
129154

130155
private EvalCreateMap(
@@ -133,8 +158,8 @@ private EvalCreateMap(
133158
PlannedInterpretable[] values,
134159
boolean[] isOptional) {
135160
super(expr);
136-
Preconditions.checkArgument(keys.length == values.length);
137-
Preconditions.checkArgument(keys.length == isOptional.length);
161+
checkArgument(keys.length == values.length);
162+
checkArgument(keys.length == isOptional.length);
138163
this.keys = keys;
139164
this.values = values;
140165
this.isOptional = isOptional;

‎runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,19 @@ public void plan_createList() throws Exception {
562562
ImmutableList<Object> result = (ImmutableList<Object>) program.eval();
563563

564564
assertThat(result).containsExactly(1L, "foo", true, ImmutableList.of(2L, false)).inOrder();
565+
assertThat(program.eval()).isSameInstanceAs(result);
566+
}
567+
568+
@Test
569+
@SuppressWarnings("unchecked") // test only
570+
public void plan_createList_withVariable() throws Exception {
571+
CelAbstractSyntaxTree ast = compile("[1, int_var]");
572+
Program program = PLANNER.plan(ast);
573+
574+
ImmutableList<Object> result =
575+
(ImmutableList<Object>) program.eval(ImmutableMap.of("int_var", 2L));
576+
577+
assertThat(result).containsExactly(1L, 2L).inOrder();
565578
}
566579

567580
@Test
@@ -573,6 +586,40 @@ public void plan_createMap() throws Exception {
573586
ImmutableMap<Object, Object> result = (ImmutableMap<Object, Object>) program.eval();
574587

575588
assertThat(result).containsExactly("foo", 1L, true, "bar").inOrder();
589+
assertThat(program.eval()).isSameInstanceAs(result);
590+
}
591+
592+
@Test
593+
@TestParameters({
594+
"{expression: \"{'foo': 1, dyn_var: 2}\"}",
595+
"{expression: \"{'foo': 1, 'bar': int_var}\"}"
596+
})
597+
@SuppressWarnings("unchecked") // test only
598+
public void plan_createMap_withVariable(String expression) throws Exception {
599+
CelAbstractSyntaxTree ast = compile(expression);
600+
Program program = PLANNER.plan(ast);
601+
602+
ImmutableMap<Object, Object> result =
603+
(ImmutableMap<Object, Object>)
604+
program.eval(ImmutableMap.of("dyn_var", "bar", "int_var", 2L));
605+
606+
assertThat(result).containsExactly("foo", 1L, "bar", 2L).inOrder();
607+
}
608+
609+
@Test
610+
@TestParameters({
611+
"{expression: '[?1]',"
612+
+ " expectedError: 'Cannot initialize optional list element from non-optional value 1'}",
613+
"{expression: '{?1: \"one\"}',"
614+
+ " expectedError: \"Cannot initialize optional entry '1' from non-optional value one\"}"
615+
})
616+
public void plan_createContainer_optionalNonOptionalConstant_throwsAtEval(
617+
String expression, String expectedError) throws Exception {
618+
CelAbstractSyntaxTree ast = CEL_COMPILER.parse(expression).getAst();
619+
Program program = PLANNER.plan(ast);
620+
621+
CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval);
622+
assertThat(e).hasMessageThat().contains(expectedError);
576623
}
577624

578625
@Test

‎testing/src/main/java/dev/cel/testing/testrunner/CelCoverageIndex.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,13 @@ private void traverseAndCalculateCoverage(
257257
nodeId, stats, exprText, precedingTabs, logUnencountered, reportBuilder);
258258
}
259259

260+
boolean propagateChildCoverage =
261+
stats.covered.get()
262+
&& (node.getKind() == ExprKind.Kind.LIST || node.getKind() == ExprKind.Kind.MAP);
260263
for (CelNavigableExpr child : node.children().collect(toImmutableList())) {
264+
if (propagateChildCoverage) {
265+
statsMap.get(child.id()).covered.set(true);
266+
}
261267
dotGraphBuilder.append(String.format("%d -> %d;\n", nodeId, child.id()));
262268
traverseAndCalculateCoverage(
263269
child, statsMap, logUnencountered, precedingTabs, reportBuilder, dotGraphBuilder);

‎testing/src/test/java/dev/cel/testing/testrunner/CelCoverageIndexTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,4 +178,22 @@ public void getCoverageReport_fullCoverage_multipleEvaluations() throws Exceptio
178178
assertThat(report.unencounteredNodes()).isEmpty();
179179
assertThat(report.unencounteredBranches()).isEmpty();
180180
}
181+
182+
@Test
183+
public void getCoverageReport_constantContainers_propagatesCoverageToEvaluatedBranchesOnly()
184+
throws Exception {
185+
Cel cel = CelFactory.plannerCelBuilder().addVar("x", SimpleType.INT).build();
186+
CelAbstractSyntaxTree ast = cel.compile("x > 1 ? {'a': 1}['a'] == 1 : [1, 2][0] == 1").getAst();
187+
CelRuntime.Program program = cel.createProgram(ast);
188+
CelCoverageIndex coverageIndex = new CelCoverageIndex();
189+
coverageIndex.init(ast);
190+
CelEvaluationListener listener = coverageIndex.newEvaluationListener();
191+
192+
program.trace(ImmutableMap.of("x", 2L), listener);
193+
CoverageReport report = coverageIndex.generateCoverageReport();
194+
195+
assertThat(report.coveredNodes()).isEqualTo(11);
196+
assertThat(report.nodes()).isEqualTo(18);
197+
assertThat(report.unencounteredNodes()).containsExactly("Expression ID 18 ('[1, 2][0] == 1')");
198+
}
181199
}

0 commit comments

Comments
 (0)