diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java b/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java deleted file mode 100644 index 8883ac12c..000000000 --- a/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2026 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dev.cel.runtime.planner; - -import dev.cel.runtime.GlobalResolver; - -/** Identifies a resolver that can be unwrapped to bypass local variable state. */ -public interface ActivationWrapper extends GlobalResolver { - GlobalResolver unwrap(); - - /** Returns true if the given name is bound by this local activation wrapper. */ - boolean isLocallyBound(String name); -} diff --git a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel index 3e7e4d21e..c25bd9ca0 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel @@ -135,7 +135,6 @@ java_library( "StringQualifier.java", ], deps = [ - ":activation_wrapper", ":eval_helpers", ":planned_interpretable", "//common:container", @@ -278,12 +277,6 @@ java_library( ], ) -java_library( - name = "activation_wrapper", - srcs = ["ActivationWrapper.java"], - deps = ["//runtime:interpretable"], -) - java_library( name = "error_metadata", srcs = ["ErrorMetadata.java"], @@ -431,10 +424,8 @@ java_library( name = "eval_fold", srcs = ["EvalFold.java"], deps = [ - ":activation_wrapper", ":planned_interpretable", "//common/ast", - "//common/exceptions:runtime_exception", "//common/values:mutable_map_value", "//runtime:accumulated_unknowns", "//runtime:concatenated_list_view", @@ -807,7 +798,6 @@ cel_android_library( "StringQualifier.java", ], deps = [ - ":activation_wrapper_android", ":eval_helpers_android", ":planned_interpretable_android", "//common:container_android", @@ -952,12 +942,6 @@ cel_android_library( ], ) -cel_android_library( - name = "activation_wrapper_android", - srcs = ["ActivationWrapper.java"], - deps = ["//runtime:interpretable_android"], -) - cel_android_library( name = "error_metadata_android", srcs = ["ErrorMetadata.java"], @@ -1105,10 +1089,8 @@ cel_android_library( name = "eval_fold_android", srcs = ["EvalFold.java"], deps = [ - ":activation_wrapper_android", ":planned_interpretable_android", "//common/ast:ast_android", - "//common/exceptions:runtime_exception", "//common/values:mutable_map_value_android", "//runtime:concatenated_list_view", "//runtime:evaluation_exception", diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java index 5547dc315..785673658 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java @@ -18,51 +18,41 @@ import com.google.common.collect.ImmutableMap; import com.google.errorprone.annotations.Immutable; import dev.cel.common.ast.CelExpr; -import dev.cel.common.exceptions.CelRuntimeException; import dev.cel.common.values.MutableMapValue; import dev.cel.runtime.AccumulatedUnknowns; import dev.cel.runtime.CelEvaluationException; import dev.cel.runtime.ConcatenatedListView; import dev.cel.runtime.GlobalResolver; import java.util.Collection; +import java.util.List; import java.util.Map; +import java.util.RandomAccess; import org.jspecify.annotations.Nullable; @Immutable final class EvalFold extends PlannedInterpretable { - private final String accuVar; + private final int accuSlot; private final PlannedInterpretable accuInit; - private final String iterVar; - private final String iterVar2; + private final int iterSlot; + private final int iter2Slot; private final PlannedInterpretable iterRange; private final PlannedInterpretable condition; private final PlannedInterpretable loopStep; private final PlannedInterpretable result; - private final boolean mutableAccu; static EvalFold create( CelExpr expr, - String accuVar, + int accuSlot, PlannedInterpretable accuInit, - String iterVar, - String iterVar2, + int iterSlot, + int iter2Slot, PlannedInterpretable iterRange, PlannedInterpretable loopCondition, PlannedInterpretable loopStep, - PlannedInterpretable result, - boolean mutableAccu) { + PlannedInterpretable result) { return new EvalFold( - expr, - accuVar, - accuInit, - iterVar, - iterVar2, - iterRange, - loopCondition, - loopStep, - result, - mutableAccu); + expr, accuSlot, accuInit, iterSlot, iter2Slot, iterRange, loopCondition, loopStep, result); } @Override @@ -71,79 +61,90 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEva if (iterRangeRaw instanceof AccumulatedUnknowns) { return iterRangeRaw; } - Folder folder = new Folder(resolver, frame, accuInit, accuVar, iterVar, iterVar2, mutableAccu); - + frame.setComprehensionSlot(accuSlot, accuInit); if (iterRangeRaw instanceof Map) { - return evalMap((Map) iterRangeRaw, folder, frame); - } else if (iterRangeRaw instanceof Collection) { - return evalList((Collection) iterRangeRaw, folder, frame); + return evalMap((Map) iterRangeRaw, resolver, frame); + } else if (iterRangeRaw instanceof List) { + List list = (List) iterRangeRaw; + // ConcatenatedListView (a mutable accumulator) does not support get(int). + return evalList( + list instanceof RandomAccess ? list : ImmutableList.copyOf(list), resolver, frame); } else { throw new IllegalArgumentException("Unexpected iter_range type: " + iterRangeRaw.getClass()); } } - private Object evalMap(Map iterRange, Folder folder, ExecutionFrame frame) + private Object evalMap(Map iterRange, GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException { for (Map.Entry entry : iterRange.entrySet()) { frame.incrementIterations(); - folder.iterVarVal = entry.getKey(); - if (!iterVar2.isEmpty()) { - folder.iterVar2Val = entry.getValue(); + frame.setComprehensionSlot(iterSlot, entry.getKey()); + if (iter2Slot >= 0) { + frame.setComprehensionSlot(iter2Slot, entry.getValue()); } - Object condResult = condition.eval(folder, frame); + Object condResult = condition.eval(resolver, frame); if (condResult instanceof AccumulatedUnknowns) { - return mergeAccumulator(folder.accuVal, condResult); + return mergeAccumulator(frame.getComprehensionSlot(accuSlot), condResult); } if (!(condResult instanceof Boolean)) { throw new IllegalArgumentException( String.format("Expected boolean value, found :%s", condResult)); } - boolean cond = (boolean) condResult; - if (!cond) { + if (!(boolean) condResult) { break; } - Object stepResult = loopStep.eval(folder, frame); - folder.accuVal = mergeAccumulator(folder.accuVal, stepResult); - folder.initialized = true; + Object stepResult = loopStep.eval(resolver, frame); + frame.setComprehensionSlot( + accuSlot, mergeAccumulator(frame.getComprehensionSlot(accuSlot), stepResult)); } - return folder.evalResult(result); + return evalResult(resolver, frame); } - private Object evalList(Collection iterRange, Folder folder, ExecutionFrame frame) + private Object evalList(List iterRange, GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException { - int index = 0; - for (Object item : iterRange) { + for (int index = 0; index < iterRange.size(); index++) { frame.incrementIterations(); - if (iterVar2.isEmpty()) { - folder.iterVarVal = item; + Object item = iterRange.get(index); + if (iter2Slot < 0) { + frame.setComprehensionSlot(iterSlot, item); } else { - folder.iterVarVal = (long) index; - folder.iterVar2Val = item; + frame.setComprehensionSlot(iterSlot, (long) index); + frame.setComprehensionSlot(iter2Slot, item); } - Object condResult = condition.eval(folder, frame); + Object condResult = condition.eval(resolver, frame); if (condResult instanceof AccumulatedUnknowns) { - return mergeAccumulator(folder.accuVal, condResult); + return mergeAccumulator(frame.getComprehensionSlot(accuSlot), condResult); } if (!(condResult instanceof Boolean)) { throw new IllegalArgumentException( String.format("Expected boolean value, found :%s", condResult)); } - boolean cond = (boolean) condResult; - if (!cond) { + if (!(boolean) condResult) { break; } - Object stepResult = loopStep.eval(folder, frame); - folder.accuVal = mergeAccumulator(folder.accuVal, stepResult); - folder.initialized = true; - index++; + Object stepResult = loopStep.eval(resolver, frame); + frame.setComprehensionSlot( + accuSlot, mergeAccumulator(frame.getComprehensionSlot(accuSlot), stepResult)); + } + return evalResult(resolver, frame); + } + + private Object evalResult(GlobalResolver resolver, ExecutionFrame frame) + throws CelEvaluationException { + Object accuVal = frame.getComprehensionSlot(accuSlot); + // Materialize the mutable accumulator so the result expr never observes in-place mutation. + if (accuVal instanceof ConcatenatedListView) { + frame.setComprehensionSlot(accuSlot, ImmutableList.copyOf((ConcatenatedListView) accuVal)); + } else if (accuVal instanceof MutableMapValue) { + frame.setComprehensionSlot(accuSlot, ImmutableMap.copyOf((MutableMapValue) accuVal)); } - return folder.evalResult(result); + return result.eval(resolver, frame); } private static Object mergeAccumulator(@Nullable Object currentAccu, Object newVal) { @@ -163,117 +164,55 @@ private static Object maybeWrapAccumulator(Object val) { return val; } + /** Evaluates a comprehension variable stored in an {@link ExecutionFrame} slot. */ + @Immutable + static final class EvalComprehensionSlot extends PlannedInterpretable { + private final int slotIndex; + private final boolean wrapMutableAccu; + + @Override + Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) + throws CelEvaluationException { + Object val = frame.getComprehensionSlot(slotIndex); + if (val instanceof PlannedInterpretable) { + val = ((PlannedInterpretable) val).eval(resolver, frame); + if (wrapMutableAccu && frame.enableShortCircuiting()) { + val = maybeWrapAccumulator(val); + } + frame.setComprehensionSlot(slotIndex, val); + } + return val; + } + + static EvalComprehensionSlot create(CelExpr expr, int slotIndex, boolean wrapMutableAccu) { + return new EvalComprehensionSlot(expr, slotIndex, wrapMutableAccu); + } + + private EvalComprehensionSlot(CelExpr expr, int slotIndex, boolean wrapMutableAccu) { + super(expr); + this.slotIndex = slotIndex; + this.wrapMutableAccu = wrapMutableAccu; + } + } + private EvalFold( CelExpr expr, - String accuVar, + int accuSlot, PlannedInterpretable accuInit, - String iterVar, - String iterVar2, + int iterSlot, + int iter2Slot, PlannedInterpretable iterRange, PlannedInterpretable condition, PlannedInterpretable loopStep, - PlannedInterpretable result, - boolean mutableAccu) { + PlannedInterpretable result) { super(expr); - this.accuVar = accuVar; + this.accuSlot = accuSlot; this.accuInit = accuInit; - this.iterVar = iterVar; - this.iterVar2 = iterVar2; + this.iterSlot = iterSlot; + this.iter2Slot = iter2Slot; this.iterRange = iterRange; this.condition = condition; this.loopStep = loopStep; this.result = result; - this.mutableAccu = mutableAccu; - } - - private static final class Folder implements ActivationWrapper { - private final GlobalResolver resolver; - private final ExecutionFrame frame; - private final PlannedInterpretable accuInit; - private final String accuVar; - private final String iterVar; - private final String iterVar2; - private final boolean mutableAccu; - - private @Nullable Object iterVarVal; - private @Nullable Object iterVar2Val; - private @Nullable Object accuVal; - private boolean initialized = false; - private boolean computeResult = false; - - @Override - public GlobalResolver unwrap() { - return resolver; - } - - @Override - public boolean isLocallyBound(String name) { - return name.equals(accuVar) || name.equals(iterVar) || name.equals(iterVar2); - } - - @Override - public @Nullable Object resolve(String name) { - if (name.equals(accuVar)) { - if (!initialized) { - initialized = true; - try { - Object initVal = accuInit.eval(resolver, frame); - accuVal = - mutableAccu && !computeResult && frame.enableShortCircuiting() - ? maybeWrapAccumulator(initVal) - : initVal; - } catch (CelEvaluationException e) { - throw new LazyEvaluationRuntimeException(e); - } - } - return accuVal; - } - - if (!computeResult) { - if (name.equals(iterVar)) { - return this.iterVarVal; - } - - if (name.equals(iterVar2)) { - return this.iterVar2Val; - } - } - - return resolver.resolve(name); - } - - private Object evalResult(PlannedInterpretable result) throws CelEvaluationException { - computeResult = true; - // Materialize the mutable accumulator so the result expr never observes in-place mutation. - if (accuVal instanceof ConcatenatedListView) { - accuVal = ImmutableList.copyOf((ConcatenatedListView) accuVal); - } else if (accuVal instanceof MutableMapValue) { - accuVal = ImmutableMap.copyOf((MutableMapValue) accuVal); - } - return result.eval(this, frame); - } - - private Folder( - GlobalResolver resolver, - ExecutionFrame frame, - PlannedInterpretable accuInit, - String accuVar, - String iterVar, - String iterVar2, - boolean mutableAccu) { - this.resolver = resolver; - this.frame = frame; - this.accuInit = accuInit; - this.accuVar = accuVar; - this.iterVar = iterVar; - this.iterVar2 = iterVar2; - this.mutableAccu = mutableAccu; - } - } - - private static final class LazyEvaluationRuntimeException extends CelRuntimeException { - private LazyEvaluationRuntimeException(CelEvaluationException cause) { - super(cause, cause.getErrorCode()); - } } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java b/runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java index 3ef19f3e9..601f33063 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java @@ -31,6 +31,7 @@ /** Tracks execution context within a planned program. */ final class ExecutionFrame { + private static final Object[] EMPTY_COMPREHENSION_SLOTS = new Object[0]; private final int comprehensionIterationLimit; private final boolean enableShortCircuiting; @@ -38,6 +39,7 @@ final class ExecutionFrame { private final @Nullable PartialVars partialVars; private final @Nullable CelEvaluationListener listener; private final @Nullable AsyncCallStateTracker asyncTracker; + private final Object[] comprehensionSlots; private int iterationCount; private @Nullable BlockMemoizer blockMemoizer; // The last select chain root that adapted to a new object in this evaluation, and that object. @@ -48,9 +50,15 @@ static ExecutionFrame create( CelFunctionResolver functionResolver, CelOptions celOptions, @Nullable PartialVars partialVars, - @Nullable CelEvaluationListener listener) { + @Nullable CelEvaluationListener listener, + int maxComprehensionSlots) { return new ExecutionFrame( - functionResolver, celOptions, partialVars, listener, /* asyncTracker= */ null); + functionResolver, + celOptions, + partialVars, + listener, + /* asyncTracker= */ null, + maxComprehensionSlots); } static ExecutionFrame createForAsync( @@ -58,9 +66,11 @@ static ExecutionFrame createForAsync( CelOptions celOptions, @Nullable PartialVars partialVars, @Nullable CelEvaluationListener listener, - AsyncCallStateTracker asyncTracker) { + AsyncCallStateTracker asyncTracker, + int maxComprehensionSlots) { checkNotNull(asyncTracker, "asyncTracker"); - return new ExecutionFrame(functionResolver, celOptions, partialVars, listener, asyncTracker); + return new ExecutionFrame( + functionResolver, celOptions, partialVars, listener, asyncTracker, maxComprehensionSlots); } private static int getComprehensionMaxIterations(CelOptions celOptions) { @@ -96,6 +106,14 @@ BlockMemoizer getBlockMemoizer() { return blockMemoizer; } + Object getComprehensionSlot(int index) { + return comprehensionSlots[index]; + } + + void setComprehensionSlot(int index, Object value) { + comprehensionSlots[index] = value; + } + /** Adapts a select chain root, reusing the last adaptation if the root is the same object. */ @SuppressWarnings("ReferenceEquality") // Identity is the point: value equality would be O(size). Object toTraversalTarget(Object root, CelValueConverter celValueConverter) { @@ -138,12 +156,15 @@ private ExecutionFrame( CelOptions celOptions, @Nullable PartialVars partialVars, @Nullable CelEvaluationListener listener, - @Nullable AsyncCallStateTracker asyncTracker) { + @Nullable AsyncCallStateTracker asyncTracker, + int maxComprehensionSlots) { this.comprehensionIterationLimit = getComprehensionMaxIterations(celOptions); this.enableShortCircuiting = celOptions.enableShortCircuiting(); this.functionResolver = functionResolver; this.partialVars = partialVars; this.listener = listener; this.asyncTracker = asyncTracker; + this.comprehensionSlots = + maxComprehensionSlots == 0 ? EMPTY_COMPREHENSION_SLOTS : new Object[maxComprehensionSlots]; } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java index 52ec72c89..d83e74f00 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java @@ -37,7 +37,6 @@ @Immutable final class NamespacedAttribute implements Attribute { - private final boolean disambiguateNames; private final ImmutableMap candidateAttributes; private final ImmutableList qualifiers; private final CelValueConverter celValueConverter; @@ -53,29 +52,20 @@ ImmutableSet candidateVariableNames() { @Override public Object resolve(long exprId, GlobalResolver ctx, ExecutionFrame frame) { - GlobalResolver inputVars = ctx; - // Unwrap any local activations to ensure that we reach the variables provided as input - // to the expression in the event that we need to disambiguate between global and local - // variables. - if (disambiguateNames) { - inputVars = unwrapToNonLocal(ctx); - } - - for (Map.Entry entry : candidateAttributes.entrySet()) { + ImmutableList> candidateEntries = + candidateAttributes.entrySet().asList(); + // Avoid enhanced for loop to prevent UnmodifiableIterator from being allocated + for (int entryIndex = 0; entryIndex < candidateEntries.size(); entryIndex++) { + Map.Entry entry = candidateEntries.get(entryIndex); String name = entry.getKey(); CelAttribute attr = entry.getValue(); - GlobalResolver resolver = ctx; - if (disambiguateNames) { - resolver = inputVars; - } - - Object value = resolver.resolve(name); + Object value = ctx.resolve(name); value = InterpreterUtil.maybeAdaptToAccumulatedUnknowns(value); PartialVars partialVars = frame.partialVars().orElse(null); - if (partialVars != null && !isLocallyBound(resolver, name)) { + if (partialVars != null) { ImmutableList patterns = partialVars.unknowns(); // Avoid enhanced for loop to prevent UnmodifiableIterator from being allocated for (int i = 0; i < qualifiers.size(); i++) { @@ -151,31 +141,12 @@ private static Long getEnumValue(EnumType enumType, String field) { String.format("Field %s was not found on enum %s", enumType.name(), field))); } - private boolean isLocallyBound(GlobalResolver resolver, String name) { - while (resolver instanceof ActivationWrapper) { - ActivationWrapper wrapper = (ActivationWrapper) resolver; - if (wrapper.isLocallyBound(name)) { - return true; - } - resolver = wrapper.unwrap(); - } - return false; - } - - private GlobalResolver unwrapToNonLocal(GlobalResolver resolver) { - while (resolver instanceof ActivationWrapper) { - resolver = ((ActivationWrapper) resolver).unwrap(); - } - return resolver; - } - @Override public NamespacedAttribute addQualifier(Qualifier qualifier) { return new NamespacedAttribute( typeProvider, celValueConverter, candidateAttributes, - disambiguateNames, ImmutableList.builderWithExpectedSize(qualifiers.size() + 1) .addAll(qualifiers) .add(qualifier) @@ -219,35 +190,24 @@ static NamespacedAttribute create( CelValueConverter celValueConverter, ImmutableSet namespacedNames) { ImmutableMap.Builder attributesBuilder = ImmutableMap.builder(); - boolean disambiguateNames = false; for (String name : namespacedNames) { - String baseName = name; - if (name.startsWith(".")) { - disambiguateNames = true; - baseName = name.substring(1); - } + String baseName = name.startsWith(".") ? name.substring(1) : name; attributesBuilder.put(baseName, CelAttribute.fromQualifiedIdentifier(baseName)); } return new NamespacedAttribute( - typeProvider, - celValueConverter, - attributesBuilder.buildOrThrow(), - disambiguateNames, - ImmutableList.of()); + typeProvider, celValueConverter, attributesBuilder.buildOrThrow(), ImmutableList.of()); } private NamespacedAttribute( CelTypeProvider typeProvider, CelValueConverter celValueConverter, ImmutableMap candidateAttributes, - boolean disambiguateNames, ImmutableList qualifiers) { this.typeProvider = typeProvider; this.celValueConverter = celValueConverter; this.candidateAttributes = candidateAttributes; - this.disambiguateNames = disambiguateNames; this.qualifiers = qualifiers; } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java b/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java index 20c88edcc..95e1ca30f 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java @@ -73,20 +73,24 @@ public abstract class PlannedProgram implements Program { @AutoValue.CopyAnnotations abstract Optional asyncExecutor(); + abstract int maxComprehensionSlots(); + static PlannedProgram create( PlannedInterpretable interpretable, ErrorMetadata metadata, CelOptions options, RuntimeEquality runtimeEquality, CelAsyncEvaluationOptions asyncOptions, - @Nullable ListeningExecutorService asyncExecutor) { + @Nullable ListeningExecutorService asyncExecutor, + int maxComprehensionSlots) { return new AutoValue_PlannedProgram( interpretable, metadata, options, runtimeEquality, asyncOptions, - Optional.ofNullable(asyncExecutor)); + Optional.ofNullable(asyncExecutor), + maxComprehensionSlots); } @Override @@ -224,7 +228,8 @@ public Object evalOrThrow( throws CelEvaluationException { try { ExecutionFrame frame = - ExecutionFrame.create(functionResolver, options(), partialVars, listener); + ExecutionFrame.create( + functionResolver, options(), partialVars, listener, maxComprehensionSlots()); Object evalResult = interpretable().eval(resolver, frame); if (evalResult instanceof ErrorValue) { ErrorValue errorValue = (ErrorValue) evalResult; @@ -294,7 +299,12 @@ private void step() { ExecutionFrame frame = ExecutionFrame.createForAsync( - lateBoundResolver, options(), partialVars, /* listener= */ null, tracker); + lateBoundResolver, + options(), + partialVars, + /* listener= */ null, + tracker, + maxComprehensionSlots()); Object evalResult = interpretable().eval(resolver, frame); if (evalResult instanceof AccumulatedUnknowns) { diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java index 5851ee423..d48726cf2 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java @@ -19,10 +19,10 @@ import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.errorprone.annotations.CheckReturnValue; import com.google.errorprone.annotations.Immutable; @@ -62,9 +62,9 @@ import dev.cel.runtime.CelResolvedOverload; import dev.cel.runtime.DefaultDispatcher; import dev.cel.runtime.RuntimeEquality; +import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; +import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; import org.jspecify.annotations.Nullable; @@ -113,10 +113,11 @@ public final class ProgramPlanner { */ public PlannedProgram plan(CelAbstractSyntaxTree ast) throws CelEvaluationException { PlannedInterpretable plannedInterpretable; + PlannerContext ctx; ErrorMetadata errorMetadata = ErrorMetadata.create(ast.getSource().getPositionsMap(), ast.getSource().getDescription()); try { - PlannerContext ctx = PlannerContext.create(ast, options); + ctx = PlannerContext.create(ast, options); plannedInterpretable = CelBlock.extract(ast) .map(celBlock -> planBlock(celBlock, ctx)) @@ -134,7 +135,13 @@ public PlannedProgram plan(CelAbstractSyntaxTree ast) throws CelEvaluationExcept } return PlannedProgram.create( - plannedInterpretable, errorMetadata, options, runtimeEquality, asyncOptions, asyncExecutor); + plannedInterpretable, + errorMetadata, + options, + runtimeEquality, + asyncOptions, + asyncExecutor, + ctx.maxComprehensionSlots()); } private PlannedInterpretable plan(CelExpr celExpr, PlannerContext ctx) { @@ -185,29 +192,22 @@ private PlannedInterpretable planConstant(CelExpr expr, CelConstant celConstant) private PlannedInterpretable planIdent(CelExpr celExpr, PlannerContext ctx) { CelReference ref = ctx.referenceMap().get(celExpr.id()); if (ref != null) { - return planCheckedIdent(celExpr, ref, ctx.typeMap()); + return planCheckedIdent(celExpr, ref, ctx); } String identName = celExpr.ident().name(); - PlannedInterpretable blockSlot = maybeInterceptBlockSlot(celExpr, identName).orElse(null); - if (blockSlot != null) { - return blockSlot; - } - - if (ctx.isLocalVar(identName)) { - return EvalAttribute.create(celExpr, attributeFactory.newAbsoluteAttribute(identName)); - } - - return EvalAttribute.create(celExpr, attributeFactory.newMaybeAttribute(identName)); + return planLocalIdent(celExpr, identName, ctx) + .orElseGet( + () -> EvalAttribute.create(celExpr, attributeFactory.newMaybeAttribute(identName))); } private PlannedInterpretable planCheckedIdent( - CelExpr expr, CelReference identRef, ImmutableMap typeMap) { + CelExpr expr, CelReference identRef, PlannerContext ctx) { if (identRef.value().isPresent()) { return planConstant(expr, identRef.value().get()); } - CelType type = typeMap.get(expr.id()); + CelType type = ctx.typeMap().get(expr.id()); if (type.kind().equals(CelKind.TYPE)) { TypeType identType = typeProvider @@ -227,12 +227,18 @@ private PlannedInterpretable planCheckedIdent( } String identName = identRef.name(); - PlannedInterpretable blockSlot = maybeInterceptBlockSlot(expr, identName).orElse(null); - if (blockSlot != null) { + return planLocalIdent(expr, identName, ctx) + .orElseGet( + () -> EvalAttribute.create(expr, attributeFactory.newAbsoluteAttribute(identName))); + } + + private Optional planLocalIdent( + CelExpr expr, String identName, PlannerContext ctx) { + Optional blockSlot = maybeInterceptBlockSlot(expr, identName); + if (blockSlot.isPresent()) { return blockSlot; } - - return EvalAttribute.create(expr, attributeFactory.newAbsoluteAttribute(identRef.name())); + return ctx.lookupComprehensionSlot(expr, identName); } private Optional maybeInterceptBlockSlot(CelExpr expr, String identName) { @@ -528,31 +534,34 @@ private PlannedInterpretable planCreateMap(CelExpr celExpr, PlannerContext ctx) private PlannedInterpretable planComprehension(CelExpr expr, PlannerContext ctx) { CelComprehension comprehension = expr.comprehension(); - PlannedInterpretable accuInit = plan(comprehension.accuInit(), ctx); PlannedInterpretable iterRange = plan(comprehension.iterRange(), ctx); + PlannedInterpretable accuInit = plan(comprehension.accuInit(), ctx); + + boolean mutableAccu = isMutableAccuSafe(comprehension); + int accuSlot = ctx.allocateSlot(); + int iterSlot = ctx.allocateSlot(); + int iter2Slot = comprehension.iterVar2().isEmpty() ? -1 : ctx.allocateSlot(); - ctx.pushLocalVars(comprehension.accuVar(), comprehension.iterVar(), comprehension.iterVar2()); + ctx.pushComprehensionScope( + comprehension.accuVar(), + accuSlot, + comprehension.iterVar(), + iterSlot, + comprehension.iterVar2(), + iter2Slot, + mutableAccu); PlannedInterpretable loopCondition = plan(comprehension.loopCondition(), ctx); PlannedInterpretable loopStep = plan(comprehension.loopStep(), ctx); - ctx.popLocalVars(comprehension.iterVar(), comprehension.iterVar2()); + ctx.exitComprehensionLoop(); PlannedInterpretable result = plan(comprehension.result(), ctx); - ctx.popLocalVars(comprehension.accuVar()); + ctx.popComprehensionScope(); return EvalFold.create( - expr, - comprehension.accuVar(), - accuInit, - comprehension.iterVar(), - comprehension.iterVar2(), - iterRange, - loopCondition, - loopStep, - result, - isMutableAccuSafe(comprehension)); + expr, accuSlot, accuInit, iterSlot, iter2Slot, iterRange, loopCondition, loopStep, result); } @VisibleForTesting @@ -740,10 +749,40 @@ private static Builder newBuilder() { } } + private static final class ComprehensionScope { + private final String accuVar; + private final int accuSlot; + private final String iterVar; + private final int iterSlot; + private final String iterVar2; + private final int iter2Slot; + private final boolean mutableAccu; + private boolean loopActive; + + private ComprehensionScope( + String accuVar, + int accuSlot, + String iterVar, + int iterSlot, + String iterVar2, + int iter2Slot, + boolean mutableAccu) { + this.accuVar = accuVar; + this.accuSlot = accuSlot; + this.iterVar = iterVar; + this.iterSlot = iterSlot; + this.iterVar2 = iterVar2; + this.iter2Slot = iter2Slot; + this.mutableAccu = mutableAccu; + this.loopActive = true; + } + } + private static final class PlannerContext { private final CelAbstractSyntaxTree ast; private final ImmutableSet runtimeExtensions; - private final Map localVars; + private final List comprehensionScopes; + private int maxComprehensionSlots; private ImmutableMap referenceMap() { return ast.getReferenceMap(); @@ -757,34 +796,66 @@ private boolean isChecked() { return ast.isChecked(); } - private void pushLocalVars(String... names) { - for (String name : names) { - if (Strings.isNullOrEmpty(name)) { - continue; - } - localVars.merge(name, 1, Integer::sum); - } + private int maxComprehensionSlots() { + return maxComprehensionSlots; } - private void popLocalVars(String... names) { - for (String name : names) { - if (Strings.isNullOrEmpty(name)) { - continue; + private int allocateSlot() { + return maxComprehensionSlots++; + } + + private void pushComprehensionScope( + String accuVar, + int accuSlot, + String iterVar, + int iterSlot, + String iterVar2, + int iter2Slot, + boolean mutableAccu) { + comprehensionScopes.add( + new ComprehensionScope( + accuVar, accuSlot, iterVar, iterSlot, iterVar2, iter2Slot, mutableAccu)); + } + + private void exitComprehensionLoop() { + Iterables.getLast(comprehensionScopes).loopActive = false; + } + + private void popComprehensionScope() { + comprehensionScopes.remove(comprehensionScopes.size() - 1); + } + + /** + * Resolves an identifier against active comprehension scopes from innermost to outermost. + * + *

Leading-dot identifiers (e.g. {@code .x}) explicitly disambiguate to the global scope and + * never match comprehension slots. + */ + private Optional lookupComprehensionSlot(CelExpr expr, String name) { + if (name.startsWith(".")) { + return Optional.empty(); + } + for (int i = comprehensionScopes.size() - 1; i >= 0; i--) { + ComprehensionScope scope = comprehensionScopes.get(i); + if (name.equals(scope.accuVar)) { + return Optional.of( + EvalFold.EvalComprehensionSlot.create( + expr, scope.accuSlot, scope.mutableAccu && scope.loopActive)); } - Integer count = localVars.get(name); - if (count != null) { - if (count == 1) { - localVars.remove(name); - } else { - localVars.put(name, count - 1); + if (scope.loopActive) { + if (name.equals(scope.iterVar)) { + return Optional.of( + EvalFold.EvalComprehensionSlot.create( + expr, scope.iterSlot, /* wrapMutableAccu= */ false)); + } + if (scope.iter2Slot >= 0 && name.equals(scope.iterVar2)) { + return Optional.of( + EvalFold.EvalComprehensionSlot.create( + expr, scope.iter2Slot, /* wrapMutableAccu= */ false)); } } } - } - - /** Checks if the given name is a local variable in the current scope. */ - private boolean isLocalVar(String name) { - return localVars.containsKey(name); + return Optional.empty(); } private static PlannerContext create(CelAbstractSyntaxTree ast, CelOptions options) { @@ -815,7 +886,7 @@ private static PlannerContext create(CelAbstractSyntaxTree ast, CelOptions optio private PlannerContext(CelAbstractSyntaxTree ast, ImmutableSet runtimeExtensions) { this.ast = checkNotNull(ast); this.runtimeExtensions = runtimeExtensions; - this.localVars = new HashMap<>(); + this.comprehensionScopes = new ArrayList<>(); } } diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ExecutionFrameTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ExecutionFrameTest.java index a1a5d3c26..a34b6a2c7 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ExecutionFrameTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ExecutionFrameTest.java @@ -35,7 +35,8 @@ public void toTraversalTarget_sameRootAfterSelfAdaptingRoot_reusesAdaptation() { CelFunctionResolver.EMPTY, CelOptions.DEFAULT, /* partialVars= */ null, - /* listener= */ null); + /* listener= */ null, + /* maxComprehensionSlots= */ 0); CelValueConverter converter = CelValueConverter.getDefaultInstance(); // Adapts to a new OptionalValue each time, while a map adapts to itself. Optional root = Optional.of("a"); diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index db0a9cf55..7921d80b6 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -1136,6 +1136,9 @@ public void plan_select_badPresenceTest_throws() throws Exception { @TestParameters("{expression: '[1,2,3].map(x, x + 1) == [2,3,4]'}") @TestParameters( "{expression: 'cel.bind(x, [1, 2], [x + [3], x + [4]]) == [[1, 2, 3], [1, 2, 4]]'}") + @TestParameters( + "{expression: 'cel.bind(x, [1, 2, 3].map(y, y + 1), [4, 5, 6].map(z, x[0] + z)) == [6, 7," + + " 8]'}") @TestParameters("{expression: 'cel.bind(m, {\"a\": 1}, [m]) == [{\"a\": 1}]'}") @TestParameters( "{expression: 'compre(i, acc, [1, 2], [], true, acc + [size(acc + [0])], acc) == [1, 2]'}") @@ -2291,7 +2294,88 @@ public Object toRuntimeValue(Object value) { boolean result = (boolean) program.eval(); assertThat(result).isTrue(); - assertThat(containerConversions[0]).isEqualTo(1); + assertThat(containerConversions[0]).isEqualTo(0); + } + + @Test + public void plan_comprehension_lazyBlockSlotWithInnerComprehension_preservesSlots() + throws Exception { + CelCompiler compilerWithSlotMacro = + CEL_COMPILER + .toCompilerBuilder() + .addMacros( + CelMacro.newGlobalMacro( + "slot0", + 0, + (factory, unused, args) -> Optional.of(factory.newIdentifier("@index0")))) + .build(); + CelAbstractSyntaxTree subExprAst = + compilerWithSlotMacro + .parse("cel.bind(x, [1, 2, 3].map(y, y + 1), [4, 5, 6].map(z, slot0()[0] + z))") + .getAst(); + CelExpr bindComprehension = subExprAst.getExpr().comprehension().accuInit(); + CelExpr mainComprehension = subExprAst.getExpr().comprehension().result(); + CelExpr blockExpr = + CelExpr.ofCall( + 100L, + Optional.empty(), + "cel.@block", + ImmutableList.of( + CelExpr.ofList(101L, ImmutableList.of(bindComprehension), ImmutableList.of()), + mainComprehension)); + CelAbstractSyntaxTree blockAst = + CelAbstractSyntaxTree.newParsedAst( + blockExpr, + CelSource.newBuilder() + .addAllExtensions( + Extension.create("cel_block", Version.of(1, 1), Component.COMPONENT_RUNTIME)) + .build()); + PlannedProgram program = PLANNER.plan(blockAst); + + Object result = program.eval(); + + assertThat(result).isEqualTo(ImmutableList.of(6L, 7L, 8L)); + } + + @Test + public void plan_comprehension_nestedShadowingWithOuterVarInInnerResult() throws Exception { + CelAbstractSyntaxTree ast = + compile( + "compre(x, acc, [1, 2], 0, true, acc + compre(x, innerAcc, [10, 20], 0, true," + + " innerAcc + x, innerAcc + x), acc)"); + PlannedProgram program = PLANNER.plan(ast); + + Object result = program.eval(); + + // Outer x=1: innerAcc sums 10+20=30, inner result is innerAcc + outer x = 31. + // Outer x=2: innerAcc sums 10+20=30, inner result is innerAcc + outer x = 32. + // Total acc = 31 + 32 = 63. + assertThat(result).isEqualTo(63L); + assertThat(program.maxComprehensionSlots()).isEqualTo(4); + } + + @Test + public void plan_comprehension_iterRangeIsMutableAccumulator_iteratesElements() throws Exception { + // '@'-prefixed accumulators are accumulated in place as a ConcatenatedListView. + CelCompiler compilerWithAccuMacro = + CEL_COMPILER + .toCompilerBuilder() + .addMacros( + CelMacro.newGlobalMacro( + "accu", + 0, + (factory, unused, args) -> Optional.of(factory.newIdentifier("@accu")))) + .build(); + CelAbstractSyntaxTree ast = + compile( + compilerWithAccuMacro, + "compre(i, accu(), [1, 2, 3], [], true," + + " accu() + [compre(j, n, accu(), 0, true, n + 1, n)], accu())"); + Program program = PLANNER.plan(ast); + + Object result = program.eval(); + + assertThat(result).isEqualTo(ImmutableList.of(0L, 1L, 2L)); } @Test