diff --git a/pkl-core/src/main/java/org/pkl/core/EvaluatorImpl.java b/pkl-core/src/main/java/org/pkl/core/EvaluatorImpl.java index 74ce01e1..a5649a68 100644 --- a/pkl-core/src/main/java/org/pkl/core/EvaluatorImpl.java +++ b/pkl-core/src/main/java/org/pkl/core/EvaluatorImpl.java @@ -336,6 +336,16 @@ public final class EvaluatorImpl implements Evaluator { } } + // for use in tests to determine whether an evaluator ever triggered instrumentation + boolean isInstrumentationEverUsed() { + polyglotContext.enter(); + try { + return VmLanguage.get(null).localContext.get().isInstrumentationEverUsed(); + } finally { + polyglotContext.leave(); + } + } + String evaluateOutputText(VmTyped fileOutput) { return doEvaluate(() -> VmUtils.readTextProperty(fileOutput)); } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java b/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java index 8e87f929..78c8240b 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java @@ -985,6 +985,11 @@ public abstract class TypeNode extends PklNode { // escape analysis should remove this allocation in compiled code var typeMismatches = new VmTypeMismatchException[elementTypeNodes.length]; + // disallow power assertions from triggering in case one union member checks successfully + var localContext = VmLanguage.get(this).localContext.get(); + var wasInTypeTest = localContext.isInTypeTest(); + localContext.setInTypeTest(true); + // Do eager checks (shallow-force) if there are two listings or two mappings represented. // (we can't know that `new Listing { 0; "hi" }[0]` fails for `Listing|Listing` // without checking both index 0 and index 1). @@ -992,15 +997,36 @@ public abstract class TypeNode extends PklNode { for (var i = 0; i < elementTypeNodes.length; i++) { var elementTypeNode = elementTypeNodes[i]; try { - if (shouldEagerCheck) { - return elementTypeNode.executeEagerly(frame, value); - } else { - return elementTypeNode.executeLazily(frame, value); - } + var result = + shouldEagerCheck + ? elementTypeNode.executeEagerly(frame, value) + : elementTypeNode.executeLazily(frame, value); + localContext.setInTypeTest(wasInTypeTest); + return result; } catch (VmTypeMismatchException e) { typeMismatches[i] = e; } } + + // all members failed to type check + // if enabled, re-execute type checks to generate power assertions + localContext.setInTypeTest(wasInTypeTest); + if (VmContext.get(this).getPowerAssertionsEnabled() + && (!wasInTypeTest || localContext.hasActiveTracker())) { + for (var i = 0; i < elementTypeNodes.length; i++) { + var elementTypeNode = elementTypeNodes[i]; + try { + if (shouldEagerCheck) { + elementTypeNode.executeEagerly(frame, value); + } else { + elementTypeNode.executeLazily(frame, value); + } + } catch (VmTypeMismatchException e) { + typeMismatches[i] = e; + } + } + } + throw new VmTypeMismatchException.Union(sourceSection, value, this, typeMismatches); } @@ -1011,14 +1037,36 @@ public abstract class TypeNode extends PklNode { // escape analysis should remove this allocation in compiled code var typeMismatches = new VmTypeMismatchException[elementTypeNodes.length]; + // disallow power assertions from triggering in case one union member checks successfully + var localContext = VmLanguage.get(this).localContext.get(); + var wasInTypeTest = localContext.isInTypeTest(); + localContext.setInTypeTest(true); + for (var i = 0; i < elementTypeNodes.length; i++) { // eager checks try { - return elementTypeNodes[i].executeEagerly(frame, value); + var result = elementTypeNodes[i].executeEagerly(frame, value); + localContext.setInTypeTest(wasInTypeTest); + return result; } catch (VmTypeMismatchException e) { typeMismatches[i] = e; } } + + // all members failed to type check + // if enabled, re-execute type checks to generate power assertions + localContext.setInTypeTest(wasInTypeTest); + if (VmContext.get(this).getPowerAssertionsEnabled() + && (!wasInTypeTest || localContext.hasActiveTracker())) { + for (var i = 0; i < elementTypeNodes.length; i++) { + try { + elementTypeNodes[i].executeEagerly(frame, value); + } catch (VmTypeMismatchException e) { + typeMismatches[i] = e; + } + } + } + throw new VmTypeMismatchException.Union(sourceSection, value, this, typeMismatches); } } diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java index 1125e863..f3f799ce 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java @@ -28,6 +28,8 @@ public class VmLocalContext { */ private int activeTrackerDepth = 0; + private boolean instrumentationEverUsed = false; + public VmLocalContext() {} public void shouldEagerTypecheck(boolean shouldEagerTypecheck) { @@ -48,6 +50,7 @@ public class VmLocalContext { public void enterTracker() { activeTrackerDepth++; + instrumentationEverUsed = true; } public void exitTracker() { @@ -57,4 +60,8 @@ public class VmLocalContext { public boolean hasActiveTracker() { return activeTrackerDepth > 0; } + + public boolean isInstrumentationEverUsed() { + return instrumentationEverUsed; + } } diff --git a/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt index 45fa40e7..97b1a5bf 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt @@ -566,6 +566,69 @@ class EvaluatorTest { ) } + @Test + fun `constraint failures activate instrumentation`() { + val evaluator = + with(EvaluatorBuilder.preconfigured()) { + powerAssertionsEnabled = true + build() + } + + val exc = + assertThrows { + evaluator.evaluate( + text( + """ + foo: String(chars.first == "a") = "boo" + """ + .trimIndent() + ) + ) + } + + assertThat((evaluator as EvaluatorImpl).isInstrumentationEverUsed()).isTrue + } + + @Test + fun `union single-member constraint failures do not activate instrumentation`() { + val evaluator = + with(EvaluatorBuilder.preconfigured()) { + powerAssertionsEnabled = true + build() + } + + evaluator.evaluate( + text( + """ + foo: String(startsWith("a")) | String(startsWith("b")) | String(startsWith("c")) = "cool" + """ + .trimIndent() + ) + ) + + assertThat((evaluator as EvaluatorImpl).isInstrumentationEverUsed()).isFalse + } + + @Test + fun `type test failures do not activate instrumentation`() { + val evaluator = + with(EvaluatorBuilder.preconfigured()) { + powerAssertionsEnabled = true + build() + } + + evaluator.evaluate( + text( + """ + foo = "bar" is Int(this > 0) + """ + .trimIndent() + ) + ) + + assertThat((evaluator as EvaluatorImpl).isInstrumentationEverUsed()).isFalse + } + private fun checkModule(module: PModule) { assertThat(module.properties.size).isEqualTo(2) assertThat(module.getProperty("name")).isEqualTo("pigeon")