Skip to content

Commit 4d21eac

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
No public description
PiperOrigin-RevId: 998709753
1 parent cc35883 commit 4d21eac

10 files changed

Lines changed: 216 additions & 230 deletions

File tree

‎common/src/main/java/dev/cel/common/navigation/BaseNavigableExpr.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import dev.cel.common.ast.CelExpr;
2121
import dev.cel.common.ast.CelExpr.ExprKind;
2222
import dev.cel.common.ast.Expression;
23+
import dev.cel.common.navigation.ExprPropertyCalculator.ExprProperty;
2324
import java.util.Optional;
2425
import java.util.stream.Stream;
2526

@@ -34,8 +35,30 @@
3435
@SuppressWarnings("unchecked") // Generic types are properly bound to Expression
3536
public abstract class BaseNavigableExpr<E extends Expression> {
3637

38+
private ExprPropertyCalculator<E> exprPropertyCalculator;
39+
3740
BaseNavigableExpr() {}
3841

42+
ExprPropertyCalculator<E> exprPropertyCalculator() {
43+
if (exprPropertyCalculator == null) {
44+
exprPropertyCalculator = new ExprPropertyCalculator<>(expr());
45+
}
46+
return exprPropertyCalculator;
47+
}
48+
49+
static <E extends Expression, T extends BaseNavigableExpr<E>> T buildWithProperties(
50+
Builder<E, T> builder, E expr, ExprPropertyCalculator<E> exprPropertyCalculator) {
51+
ExprProperty exprProperty = exprPropertyCalculator.getProperty(expr);
52+
T navigableExpr =
53+
builder
54+
.setExpr(expr)
55+
.setHeight(exprProperty.height())
56+
.setMaxId(exprProperty.maxId())
57+
.build();
58+
((BaseNavigableExpr<E>) navigableExpr).exprPropertyCalculator = exprPropertyCalculator;
59+
return navigableExpr;
60+
}
61+
3962
public abstract E expr();
4063

4164
public long id() {

‎common/src/main/java/dev/cel/common/navigation/CelNavigableExpr.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.google.auto.value.AutoValue;
1818
import dev.cel.common.ast.CelExpr;
19-
import dev.cel.common.navigation.ExprPropertyCalculator.ExprProperty;
2019
import java.util.Optional;
2120
import java.util.stream.Stream;
2221

@@ -32,14 +31,7 @@ public abstract class CelNavigableExpr extends BaseNavigableExpr<CelExpr> {
3231

3332
/** Constructs a new instance of {@link CelNavigableExpr} from {@link CelExpr}. */
3433
public static CelNavigableExpr fromExpr(CelExpr expr) {
35-
ExprPropertyCalculator<CelExpr> exprHeightCalculator = new ExprPropertyCalculator<>(expr);
36-
ExprProperty exprProperty = exprHeightCalculator.getProperty(expr.id());
37-
38-
return builder()
39-
.setExpr(expr)
40-
.setHeight(exprProperty.height())
41-
.setMaxId(exprProperty.maxId())
42-
.build();
34+
return buildWithProperties(builder(), expr, new ExprPropertyCalculator<>(expr));
4335
}
4436

4537
@Override

‎common/src/main/java/dev/cel/common/navigation/CelNavigableExprVisitor.java‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414

1515
package dev.cel.common.navigation;
1616

17+
import static com.google.common.base.Preconditions.checkState;
18+
1719
import dev.cel.common.ast.Expression;
1820
import dev.cel.common.ast.Expression.List;
19-
import dev.cel.common.navigation.ExprPropertyCalculator.ExprProperty;
2021
import java.util.stream.Stream;
2122

2223
/** Visitor implementation to navigate an AST. */
@@ -28,16 +29,6 @@ final class CelNavigableExprVisitor<E extends Expression, T extends BaseNavigabl
2829
private final TraversalOrder traversalOrder;
2930
private final int maxDepth;
3031

31-
private CelNavigableExprVisitor(
32-
int maxDepth,
33-
ExprPropertyCalculator<E> exprPropertyCalculator,
34-
TraversalOrder traversalOrder) {
35-
this.maxDepth = maxDepth;
36-
this.exprPropertyCalculator = exprPropertyCalculator;
37-
this.traversalOrder = traversalOrder;
38-
this.streamBuilder = Stream.builder();
39-
}
40-
4132
/**
4233
* Returns a stream containing all the nodes using the specified traversal order. Note that the
4334
* collection occurs eagerly.
@@ -81,23 +72,24 @@ static <E extends Expression, T extends BaseNavigableExpr<E>> Stream<T> collect(
8172
*/
8273
static <E extends Expression, T extends BaseNavigableExpr<E>> Stream<T> collect(
8374
T navigableExpr, int maxDepth, TraversalOrder traversalOrder) {
84-
ExprPropertyCalculator<E> exprHeightCalculator =
85-
new ExprPropertyCalculator<>(navigableExpr.expr());
8675
CelNavigableExprVisitor<E, T> visitor =
87-
new CelNavigableExprVisitor<>(maxDepth, exprHeightCalculator, traversalOrder);
76+
new CelNavigableExprVisitor<>(
77+
maxDepth, navigableExpr.exprPropertyCalculator(), traversalOrder);
8878

8979
visitor.visit(navigableExpr);
9080

9181
return visitor.streamBuilder.build();
9282
}
9383

9484
private void visit(T navigableExpr) {
95-
if (navigableExpr.depth() > MAX_DESCENDANTS_RECURSION_DEPTH - 1) {
96-
throw new IllegalStateException("Max recursion depth reached.");
85+
checkState(
86+
navigableExpr.depth() <= MAX_DESCENDANTS_RECURSION_DEPTH - 1,
87+
"Max recursion depth reached.");
88+
if (navigableExpr.depth() > maxDepth) {
89+
return;
9790
}
9891

99-
boolean addToStream = navigableExpr.depth() <= maxDepth;
100-
if (addToStream && traversalOrder.equals(TraversalOrder.PRE_ORDER)) {
92+
if (traversalOrder.equals(TraversalOrder.PRE_ORDER)) {
10193
streamBuilder.add(navigableExpr);
10294
}
10395

@@ -124,7 +116,7 @@ private void visit(T navigableExpr) {
124116
break;
125117
}
126118

127-
if (addToStream && traversalOrder.equals(TraversalOrder.POST_ORDER)) {
119+
if (traversalOrder.equals(TraversalOrder.POST_ORDER)) {
128120
streamBuilder.add(navigableExpr);
129121
}
130122
}
@@ -177,15 +169,19 @@ private void visitExprList(java.util.List<E> list, T parent) {
177169
}
178170

179171
private T newNavigableChild(T parent, E expr) {
180-
ExprProperty exprProperty = exprPropertyCalculator.getProperty(expr.id());
181-
182-
return parent
183-
.<T>builderFromInstance()
184-
.setExpr(expr)
185-
.setDepth(parent.depth() + 1)
186-
.setHeight(exprProperty.height())
187-
.setMaxId(exprProperty.maxId())
188-
.setParent(parent)
189-
.build();
172+
return BaseNavigableExpr.buildWithProperties(
173+
parent.<T>builderFromInstance().setDepth(parent.depth() + 1).setParent(parent),
174+
expr,
175+
exprPropertyCalculator);
176+
}
177+
178+
private CelNavigableExprVisitor(
179+
int maxDepth,
180+
ExprPropertyCalculator<E> exprPropertyCalculator,
181+
TraversalOrder traversalOrder) {
182+
this.maxDepth = maxDepth;
183+
this.exprPropertyCalculator = exprPropertyCalculator;
184+
this.traversalOrder = traversalOrder;
185+
this.streamBuilder = Stream.builder();
190186
}
191187
}

‎common/src/main/java/dev/cel/common/navigation/CelNavigableMutableExpr.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.google.auto.value.AutoValue;
1818
import dev.cel.common.ast.CelMutableExpr;
19-
import dev.cel.common.navigation.ExprPropertyCalculator.ExprProperty;
2019
import java.util.Optional;
2120
import java.util.stream.Stream;
2221

@@ -32,15 +31,7 @@ public abstract class CelNavigableMutableExpr extends BaseNavigableExpr<CelMutab
3231

3332
/** Constructs a new instance of {@link CelNavigableMutableExpr} from {@link CelMutableExpr}. */
3433
public static CelNavigableMutableExpr fromExpr(CelMutableExpr expr) {
35-
ExprPropertyCalculator<CelMutableExpr> exprHeightCalculator =
36-
new ExprPropertyCalculator<>(expr);
37-
ExprProperty exprProperty = exprHeightCalculator.getProperty(expr.id());
38-
39-
return builder()
40-
.setExpr(expr)
41-
.setHeight(exprProperty.height())
42-
.setMaxId(exprProperty.maxId())
43-
.build();
34+
return buildWithProperties(builder(), expr, new ExprPropertyCalculator<>(expr));
4435
}
4536

4637
@Override

‎common/src/main/java/dev/cel/common/navigation/ExprPropertyCalculator.java‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,30 +21,17 @@
2121
import dev.cel.common.ast.Expression.List;
2222
import dev.cel.common.ast.Expression.Map;
2323
import dev.cel.common.ast.Expression.Struct;
24-
import java.util.HashMap;
24+
import java.util.IdentityHashMap;
2525

2626
/** Package-private class to assist computing the height and the max ID of expression nodes. */
2727
final class ExprPropertyCalculator<E extends Expression> {
28-
// Store hashmap instead of immutable map for performance, such that this helper class can be
29-
// instantiated faster.
30-
private final HashMap<Long, ExprProperty> idToProperty;
28+
// Store IdentityHashMap instead of immutable map for performance, such that this helper class can
29+
// be instantiated faster and nodes with duplicate IDs (e.g. 0) do not collide.
30+
private final IdentityHashMap<E, ExprProperty> exprToProperty;
3131

32-
ExprPropertyCalculator(E celExpr) {
33-
this.idToProperty = new HashMap<>();
34-
visit(celExpr);
35-
}
36-
37-
/**
38-
* Retrieves the property containing the expression's maximum ID and the height of the subtree.
39-
*
40-
* @throws IllegalArgumentException If the provided expression ID does not exist.
41-
*/
42-
ExprProperty getProperty(Long exprId) {
43-
if (!idToProperty.containsKey(exprId)) {
44-
throw new IllegalArgumentException("Property not found for expression id: " + exprId);
45-
}
46-
47-
return idToProperty.get(exprId);
32+
ExprProperty getProperty(E expr) {
33+
ExprProperty property = exprToProperty.get(expr);
34+
return property != null ? property : visit(expr);
4835
}
4936

5037
private ExprProperty visit(E expr) {
@@ -79,7 +66,7 @@ private ExprProperty visit(E expr) {
7966
ExprProperty exprProperty =
8067
ExprProperty.create(
8168
baseHeight + visitedProperty.height(), max(visitedProperty.maxId(), expr.id()));
82-
idToProperty.put(expr.id(), exprProperty);
69+
exprToProperty.put(expr, exprProperty);
8370

8471
return exprProperty;
8572
}
@@ -137,6 +124,11 @@ private ExprProperty visitExprList(java.util.List<E> list) {
137124
return visitedProperty;
138125
}
139126

127+
ExprPropertyCalculator(E celExpr) {
128+
this.exprToProperty = new IdentityHashMap<>();
129+
visit(celExpr);
130+
}
131+
140132
/** Value class to store the height and the max ID at a specific expression ID. */
141133
@AutoValue
142134
abstract static class ExprProperty {

‎common/src/test/java/dev/cel/common/navigation/CelNavigableMutableExprTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public void construct_withoutParent() {
3939
assertThat(navigableExpr.expr()).isEqualTo(constExpr);
4040
assertThat(navigableExpr.depth()).isEqualTo(2);
4141
assertThat(navigableExpr.parent()).isEmpty();
42+
assertThat(navigableExpr.allNodes()).containsExactly(navigableExpr);
4243
}
4344

4445
@Test
@@ -147,4 +148,24 @@ public void children_filteredConstants_returnsSingleConstant() {
147148
assertThat(allNodes)
148149
.containsExactly(CelMutableExpr.ofConstant(CelConstant.ofValue("element1")));
149150
}
151+
152+
@Test
153+
public void allNodes_zeroIds_heightsPopulated() {
154+
CelNavigableMutableExpr mutableExpr =
155+
CelNavigableMutableExpr.fromExpr(
156+
CelMutableExpr.ofList(
157+
CelMutableList.create(
158+
CelMutableExpr.ofConstant(CelConstant.ofValue("element1")),
159+
CelMutableExpr.ofList(
160+
CelMutableList.create(
161+
CelMutableExpr.ofConstant(CelConstant.ofValue("element2")))))));
162+
163+
ImmutableList<Integer> heights =
164+
mutableExpr
165+
.allNodes(TraversalOrder.PRE_ORDER)
166+
.map(BaseNavigableExpr::height)
167+
.collect(toImmutableList());
168+
169+
assertThat(heights).containsExactly(2, 0, 1, 0).inOrder();
170+
}
150171
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ java_library(
7070
"//common/ast:mutable_expr",
7171
"//common/navigation",
7272
"//common/navigation:common",
73+
"//common/navigation:expr_util",
7374
"//common/navigation:mutable_navigation",
7475
"//common/types",
75-
"//common/types:type_providers",
7676
"//extensions:bindings",
7777
"//optimizer:ast_optimizer",
7878
"//optimizer:mutable_ast",

0 commit comments

Comments
 (0)