Skip to content

Commit bb64ec7

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Add an option for controlling short-circuiting behavior for logical operators.
PiperOrigin-RevId: 613276249
1 parent 366e749 commit bb64ec7

3 files changed

Lines changed: 190 additions & 12 deletions

File tree

‎common/src/main/java/dev/cel/common/CelOptions.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ public abstract class CelOptions {
7171

7272
public abstract boolean disableCelStandardEquality();
7373

74+
public abstract boolean enableShortCircuiting();
75+
7476
public abstract boolean enableRegexPartialMatch();
7577

7678
public abstract boolean enableUnsignedComparisonAndArithmeticIsUnsigned();
@@ -170,6 +172,7 @@ public static Builder newBuilder() {
170172
.enableNamespacedDeclarations(true)
171173
// Evaluation options
172174
.disableCelStandardEquality(true)
175+
.enableShortCircuiting(true)
173176
.enableRegexPartialMatch(false)
174177
.enableUnsignedComparisonAndArithmeticIsUnsigned(false)
175178
.enableUnsignedLongs(false)
@@ -364,6 +367,17 @@ public abstract static class Builder {
364367
*/
365368
public abstract Builder disableCelStandardEquality(boolean value);
366369

370+
/**
371+
* Enable short-circuiting of the logical operator evaluation. If enabled, AND, OR, and TERNARY
372+
* do not evaluate the entire expression once the resulting value is known from the left-hand
373+
* side.
374+
*
375+
* <p>This option is enabled by default. In most cases, this should not be disabled except for
376+
* debugging purposes or collecting results for all evaluated branches through {@link
377+
* dev.cel.runtime.CelEvaluationListener}.
378+
*/
379+
public abstract Builder enableShortCircuiting(boolean value);
380+
367381
/**
368382
* Treat regex {@code matches} calls as substring (unanchored) match patterns.
369383
*

‎runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,22 @@ private Optional<CelAttribute> maybeContainerIndexAttribute(
455455
private IntermediateResult evalConditional(ExecutionFrame frame, CelCall callExpr)
456456
throws InterpreterException {
457457
IntermediateResult condition = evalBooleanStrict(frame, callExpr.args().get(0));
458-
if (isUnknownValue(condition.value())) {
459-
return condition;
460-
}
461-
if ((boolean) condition.value()) {
462-
return evalInternal(frame, callExpr.args().get(1));
458+
if (celOptions.enableShortCircuiting()) {
459+
if (isUnknownValue(condition.value())) {
460+
return condition;
461+
}
462+
if ((boolean) condition.value()) {
463+
return evalInternal(frame, callExpr.args().get(1));
464+
}
465+
return evalInternal(frame, callExpr.args().get(2));
466+
} else {
467+
IntermediateResult lhs = evalInternal(frame, callExpr.args().get(1));
468+
IntermediateResult rhs = evalInternal(frame, callExpr.args().get(2));
469+
if (isUnknownValue(condition.value())) {
470+
return condition;
471+
}
472+
return (boolean) condition.value() ? lhs : rhs;
463473
}
464-
return evalInternal(frame, callExpr.args().get(2));
465474
}
466475

467476
private IntermediateResult mergeBooleanUnknowns(IntermediateResult lhs, IntermediateResult rhs)
@@ -482,15 +491,33 @@ private IntermediateResult mergeBooleanUnknowns(IntermediateResult lhs, Intermed
482491
InterpreterUtil.shortcircuitUnknownOrThrowable(lhs.value(), rhs.value()));
483492
}
484493

494+
private enum ShortCircuitableOperators {
495+
LOGICAL_OR,
496+
LOGICAL_AND
497+
}
498+
499+
private boolean canShortCircuit(IntermediateResult result, ShortCircuitableOperators operator) {
500+
if (!celOptions.enableShortCircuiting() || !(result.value() instanceof Boolean)) {
501+
return false;
502+
}
503+
504+
Boolean value = (Boolean) result.value();
505+
if (value && operator.equals(ShortCircuitableOperators.LOGICAL_OR)) {
506+
return true;
507+
}
508+
509+
return !value && operator.equals(ShortCircuitableOperators.LOGICAL_AND);
510+
}
511+
485512
private IntermediateResult evalLogicalOr(ExecutionFrame frame, CelCall callExpr)
486513
throws InterpreterException {
487514
IntermediateResult left = evalBooleanNonstrict(frame, callExpr.args().get(0));
488-
if (left.value() instanceof Boolean && (Boolean) left.value()) {
515+
if (canShortCircuit(left, ShortCircuitableOperators.LOGICAL_OR)) {
489516
return left;
490517
}
491518

492519
IntermediateResult right = evalBooleanNonstrict(frame, callExpr.args().get(1));
493-
if (right.value() instanceof Boolean && (Boolean) right.value()) {
520+
if (canShortCircuit(right, ShortCircuitableOperators.LOGICAL_OR)) {
494521
return right;
495522
}
496523

@@ -505,12 +532,12 @@ private IntermediateResult evalLogicalOr(ExecutionFrame frame, CelCall callExpr)
505532
private IntermediateResult evalLogicalAnd(ExecutionFrame frame, CelCall callExpr)
506533
throws InterpreterException {
507534
IntermediateResult left = evalBooleanNonstrict(frame, callExpr.args().get(0));
508-
if (left.value() instanceof Boolean && !((Boolean) left.value())) {
535+
if (canShortCircuit(left, ShortCircuitableOperators.LOGICAL_AND)) {
509536
return left;
510537
}
511538

512539
IntermediateResult right = evalBooleanNonstrict(frame, callExpr.args().get(1));
513-
if (right.value() instanceof Boolean && !((Boolean) right.value())) {
540+
if (canShortCircuit(right, ShortCircuitableOperators.LOGICAL_AND)) {
514541
return right;
515542
}
516543

‎runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java‎

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@
2020
import com.google.api.expr.v1alpha1.Constant;
2121
import com.google.api.expr.v1alpha1.Expr;
2222
import com.google.api.expr.v1alpha1.Type.PrimitiveType;
23+
import com.google.common.collect.ImmutableList;
2324
import com.google.common.collect.ImmutableMap;
2425
import com.google.protobuf.Any;
2526
import com.google.protobuf.BoolValue;
2627
import com.google.protobuf.ByteString;
2728
import com.google.protobuf.DescriptorProtos.FileDescriptorSet;
2829
import com.google.protobuf.DynamicMessage;
2930
import com.google.rpc.context.AttributeContext;
31+
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
32+
import com.google.testing.junit.testparameterinjector.TestParameters;
3033
import dev.cel.bundle.Cel;
3134
import dev.cel.bundle.CelFactory;
3235
import dev.cel.common.CelAbstractSyntaxTree;
@@ -50,9 +53,8 @@
5053
import java.util.concurrent.atomic.AtomicReference;
5154
import org.junit.Test;
5255
import org.junit.runner.RunWith;
53-
import org.junit.runners.JUnit4;
5456

55-
@RunWith(JUnit4.class)
57+
@RunWith(TestParameterInjector.class)
5658
public class CelRuntimeTest {
5759

5860
@Test
@@ -400,6 +402,141 @@ public void trace_withVariableResolver() throws Exception {
400402
assertThat(result).isEqualTo("hello");
401403
}
402404

405+
@Test
406+
public void trace_shortCircuitingDisabled_logicalAndAllBranchesVisited() throws Exception {
407+
ImmutableList.Builder<Boolean> branchResults = ImmutableList.builder();
408+
CelEvaluationListener listener =
409+
(expr, res) -> {
410+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)) {
411+
branchResults.add((Boolean) res);
412+
}
413+
};
414+
Cel cel =
415+
CelFactory.standardCelBuilder()
416+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
417+
.build();
418+
CelAbstractSyntaxTree ast = cel.compile("false && true && false").getAst();
419+
420+
boolean result = (boolean) cel.createProgram(ast).trace(listener);
421+
422+
assertThat(result).isFalse();
423+
assertThat(branchResults.build()).containsExactly(false, true, false);
424+
}
425+
426+
@Test
427+
public void trace_shortCircuitingDisabled_logicalAndWithUnknowns() throws Exception {
428+
ImmutableList.Builder<Object> branchResults = ImmutableList.builder();
429+
CelEvaluationListener listener =
430+
(expr, res) -> {
431+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
432+
|| expr.identOrDefault().name().equals("x")) {
433+
branchResults.add(res);
434+
}
435+
};
436+
Cel cel =
437+
CelFactory.standardCelBuilder()
438+
.addVar("x", SimpleType.BOOL)
439+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
440+
.build();
441+
CelAbstractSyntaxTree ast = cel.compile("false && false && x").getAst();
442+
443+
Object unknownResult = cel.createProgram(ast).trace(listener);
444+
445+
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
446+
assertThat(branchResults.build()).containsExactly(false, false, unknownResult);
447+
}
448+
449+
@Test
450+
public void trace_shortCircuitingDisabled_logicalOrAllBranchesVisited() throws Exception {
451+
ImmutableList.Builder<Boolean> branchResults = ImmutableList.builder();
452+
CelEvaluationListener listener =
453+
(expr, res) -> {
454+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)) {
455+
branchResults.add((Boolean) res);
456+
}
457+
};
458+
Cel cel =
459+
CelFactory.standardCelBuilder()
460+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
461+
.build();
462+
CelAbstractSyntaxTree ast = cel.compile("true || false || true").getAst();
463+
464+
boolean result = (boolean) cel.createProgram(ast).trace(listener);
465+
466+
assertThat(result).isTrue();
467+
assertThat(branchResults.build()).containsExactly(true, false, true);
468+
}
469+
470+
@Test
471+
public void trace_shortCircuitingDisabled_logicalOrWithUnknowns() throws Exception {
472+
ImmutableList.Builder<Object> branchResults = ImmutableList.builder();
473+
CelEvaluationListener listener =
474+
(expr, res) -> {
475+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
476+
|| expr.identOrDefault().name().equals("x")) {
477+
branchResults.add(res);
478+
}
479+
};
480+
Cel cel =
481+
CelFactory.standardCelBuilder()
482+
.addVar("x", SimpleType.BOOL)
483+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
484+
.build();
485+
CelAbstractSyntaxTree ast = cel.compile("false || false || x").getAst();
486+
487+
Object unknownResult = cel.createProgram(ast).trace(listener);
488+
489+
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
490+
assertThat(branchResults.build()).containsExactly(false, false, unknownResult);
491+
}
492+
493+
@Test
494+
public void trace_shortCircuitingDisabled_ternaryAllBranchesVisited() throws Exception {
495+
ImmutableList.Builder<Boolean> branchResults = ImmutableList.builder();
496+
CelEvaluationListener listener =
497+
(expr, res) -> {
498+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)) {
499+
branchResults.add((Boolean) res);
500+
}
501+
};
502+
Cel cel =
503+
CelFactory.standardCelBuilder()
504+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
505+
.build();
506+
CelAbstractSyntaxTree ast = cel.compile("true ? false : true").getAst();
507+
508+
boolean result = (boolean) cel.createProgram(ast).trace(listener);
509+
510+
assertThat(result).isFalse();
511+
assertThat(branchResults.build()).containsExactly(true, false, true);
512+
}
513+
514+
@Test
515+
@TestParameters("{source: 'false ? true : x'}")
516+
@TestParameters("{source: 'true ? x : false'}")
517+
@TestParameters("{source: 'x ? true : false'}")
518+
public void trace_shortCircuitingDisabled_ternaryWithUnknowns(String source) throws Exception {
519+
ImmutableList.Builder<Object> branchResults = ImmutableList.builder();
520+
CelEvaluationListener listener =
521+
(expr, res) -> {
522+
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
523+
|| expr.identOrDefault().name().equals("x")) {
524+
branchResults.add(res);
525+
}
526+
};
527+
Cel cel =
528+
CelFactory.standardCelBuilder()
529+
.addVar("x", SimpleType.BOOL)
530+
.setOptions(CelOptions.current().enableShortCircuiting(false).build())
531+
.build();
532+
CelAbstractSyntaxTree ast = cel.compile(source).getAst();
533+
534+
Object unknownResult = cel.createProgram(ast).trace(listener);
535+
536+
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
537+
assertThat(branchResults.build()).containsExactly(false, unknownResult, true);
538+
}
539+
403540
@Test
404541
public void standardEnvironmentDisabledForRuntime_throws() throws Exception {
405542
CelCompiler celCompiler =

0 commit comments

Comments
 (0)