Avoid using Object.toString() for in-language API (#1850)

This changes code so that the in-language `toString()` doesn't use
Java's `Object.toString()`.
Additionally, the existing `toString()` implementation is switched over
to using `VmValueRenderer`, which does not force (leaves `?` in place of
unforced values).

This helps improve ergonomics when working on the language (can enable
automatically calling `toString()` on debug values).

Additionally, external tools might call toString(); for example,
Truffle's compiler sometimes will stringify node fields (e.g.
when -Dengine.CompilationFailureAction=Throw) is set.
This commit is contained in:
Daniel Chao
2026-09-11 23:08:26 +00:00
committed by GitHub
parent e3881f6149
commit d0de24b85b
40 changed files with 166 additions and 58 deletions
@@ -386,7 +386,7 @@ public final class SymbolTable {
|| value instanceof Boolean || value instanceof Boolean
|| value instanceof VmDuration || value instanceof VmDuration
|| value instanceof VmDataSize) { || value instanceof VmDataSize) {
return "[" + value + "]"; return "[" + VmUtils.toPklString(value) + "]";
} }
} }
@@ -17,7 +17,6 @@ package org.pkl.core.ast.internal;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.DirectCallNode; import com.oracle.truffle.api.nodes.DirectCallNode;
@@ -75,11 +74,10 @@ public abstract class ToStringNode extends UnaryExpressionNode {
return (String) callNode.call(value, value.getVmClass().getPrototype()); return (String) callNode.call(value, value.getVmClass().getPrototype());
} }
@Fallback @Specialization
@Override
@TruffleBoundary @TruffleBoundary
protected Object fallback(Object value) { protected String evalVmValue(VmValue value) {
return value.toString(); return value.toPklString();
} }
protected InvokeMethodVirtualNode createInvokeNode() { protected InvokeMethodVirtualNode createInvokeNode() {
@@ -119,7 +119,7 @@ public abstract class VmTypeMismatchException extends ControlFlowException {
.map((l) -> valueFormatter.formatStringValue(l, "")) .map((l) -> valueFormatter.formatStringValue(l, ""))
.collect(Collectors.joining("|")); .collect(Collectors.joining("|"));
} else { } else {
renderedType = expectedType.toString(); renderedType = VmUtils.toPklString(expectedType);
} }
if (actualValue instanceof VmNull if (actualValue instanceof VmNull
@@ -156,7 +156,10 @@ public abstract class VmTypeMismatchException extends ControlFlowException {
builder builder
.append( .append(
ErrorMessages.createIndented( ErrorMessages.createIndented(
"typeMismatch", indent, renderedType, VmUtils.getClass(actualValue))) "typeMismatch",
indent,
renderedType,
VmUtils.getClass(actualValue).toPklString()))
.append("\n") .append("\n")
.append(indent) .append(indent)
.append("Value: ") .append("Value: ")
@@ -177,7 +180,7 @@ public abstract class VmTypeMismatchException extends ControlFlowException {
public ClassType(SourceSection sourceSection, VmClass actualClass, VmClass expectedClass) { public ClassType(SourceSection sourceSection, VmClass actualClass, VmClass expectedClass) {
super(sourceSection, actualClass); super(sourceSection, actualClass);
this.expectedClass = expectedClass; this.expectedClass = expectedClass;
renderedExpected = "Class<" + expectedClass + ">"; renderedExpected = "Class<" + expectedClass.toPklString() + ">";
} }
public ClassType(SourceSection sourceSection, VmClass actualClass, PType expectedType) { public ClassType(SourceSection sourceSection, VmClass actualClass, PType expectedType) {
@@ -191,7 +194,7 @@ public abstract class VmTypeMismatchException extends ControlFlowException {
public void buildMessage( public void buildMessage(
AnsiStringBuilder builder, String indent, boolean withPowerAssertions) { AnsiStringBuilder builder, String indent, boolean withPowerAssertions) {
var actualClass = (VmClass) actualValue; var actualClass = (VmClass) actualValue;
var renderedActualClass = "Class<" + actualClass + ">"; var renderedActualClass = "Class<" + actualClass.toPklString() + ">";
// give better error than "expected Class<foo.Bar>, but got Class<foo.Bar>" in case of naming // give better error than "expected Class<foo.Bar>, but got Class<foo.Bar>" in case of naming
// conflict // conflict
@@ -168,7 +168,7 @@ public final class VmBytes extends VmValue implements Iterable<Long> {
} }
@Override @Override
public String toString() { public String toPklString() {
var sb = new StringBuilder("Bytes("); var sb = new StringBuilder("Bytes(");
var isFirst = true; var isFirst = true;
for (var byt : bytes) { for (var byt : bytes) {
@@ -748,7 +748,7 @@ public final class VmClass extends VmValue {
} }
@Override @Override
public String toString() { public String toPklString() {
return getDisplayName(); return getDisplayName();
} }
@@ -1,5 +1,5 @@
/* /*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -166,17 +166,19 @@ public abstract class VmCollection extends VmValue implements Iterable<Object> {
var iter = iterator(); var iter = iterator();
var builder = new StringBuilder(); var builder = new StringBuilder();
builder.append(iter.next()); // TODO: use ToStringNode
builder.append(VmUtils.toPklString(iter.next()));
while (iter.hasNext()) { while (iter.hasNext()) {
builder.append(separator); builder.append(separator);
builder.append(iter.next()); builder.append(VmUtils.toPklString(iter.next()));
} }
return builder.toString(); return builder.toString();
} }
public final String toString() { @TruffleBoundary
public final String toPklString() {
return VmValueRenderer.multiLine(Integer.MAX_VALUE).render(this); return VmValueRenderer.multiLine(Integer.MAX_VALUE).render(this);
} }
} }
@@ -156,7 +156,7 @@ public final class VmDataSize extends VmValue implements Comparable<VmDataSize>
} }
@Override @Override
public String toString() { public String toPklString() {
return MathUtils.isMathematicalInteger(value) ? (long) value + "." + unit : value + "." + unit; return MathUtils.isMathematicalInteger(value) ? (long) value + "." + unit : value + "." + unit;
} }
@@ -15,6 +15,7 @@
*/ */
package org.pkl.core.runtime; package org.pkl.core.runtime;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.CompilerDirectives.ValueType; import com.oracle.truffle.api.CompilerDirectives.ValueType;
import java.util.*; import java.util.*;
import org.jspecify.annotations.Nullable; import org.jspecify.annotations.Nullable;
@@ -152,7 +153,8 @@ public final class VmDuration extends VmValue implements Comparable<VmDuration>
} }
@Override @Override
public String toString() { @TruffleBoundary
public String toPklString() {
return DurationUtils.toPklString(value, unit); return DurationUtils.toPklString(value, unit);
} }
} }
@@ -83,7 +83,7 @@ public final class VmDynamic extends VmObject {
iterateAlreadyForcedMemberValues( iterateAlreadyForcedMemberValues(
(key, member, value) -> { (key, member, value) -> {
properties.put(key.toString(), VmValue.export(value)); properties.put(VmUtils.toPklString(key), VmValue.export(value));
return true; return true;
}); });
@@ -199,14 +199,14 @@ public final class VmFunction extends VmObjectLike {
return System.identityHashCode(this); return System.identityHashCode(this);
} }
@Override
@TruffleBoundary
public String toString() {
return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this);
}
@Override @Override
public boolean isParseTimeInvisibleScope() { public boolean isParseTimeInvisibleScope() {
return isFunctionAmend && !hasObjectParams; return isFunctionAmend && !hasObjectParams;
} }
@Override
@TruffleBoundary
public String toPklString() {
return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this);
}
} }
@@ -127,7 +127,7 @@ public final class VmIntSeq extends VmValue implements Iterable<Long> {
@Override @Override
@TruffleBoundary @TruffleBoundary
public String toString() { public String toPklString() {
return step == 1 return step == 1
? "IntSeq(" + start + ", " + end + ")" ? "IntSeq(" + start + ", " + end + ")"
: "IntSeq(" + start + ", " + end + ").step(" + step + ")"; : "IntSeq(" + start + ", " + end + ").step(" + step + ")";
@@ -286,7 +286,8 @@ public final class VmMap extends VmValue implements Iterable<Map.Entry<Object, O
} }
@TruffleBoundary @TruffleBoundary
public String toString() { @Override
public String toPklString() {
return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this); return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this);
} }
@@ -87,7 +87,7 @@ public final class VmNull extends VmValue {
} }
@Override @Override
public String toString() { public String toPklString() {
return "null"; return "null";
} }
} }
@@ -204,7 +204,8 @@ public abstract class VmObject extends VmObjectLike {
force(allowUndefinedValues, true); force(allowUndefinedValues, true);
} }
public final String toString() { @TruffleBoundary
public final String toPklString() {
force(true, true); force(true, true);
return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this); return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this);
} }
@@ -113,7 +113,7 @@ public final class VmPair extends VmValue implements Iterable<Object> {
@Override @Override
@TruffleBoundary @TruffleBoundary
public String toString() { public String toPklString() {
force(true); force(true);
return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this); return VmValueRenderer.singleLine(Integer.MAX_VALUE).render(this);
} }
@@ -539,7 +539,7 @@ public final class VmReference extends VmValue {
// in-language calls _should_ all go through `ToStringNode`. // in-language calls _should_ all go through `ToStringNode`.
// however, some calls escape through to here currently (e.g. `Listing.join`). // however, some calls escape through to here currently (e.g. `Listing.join`).
@Override @Override
public String toString() { public String toPklString() {
var toStringMethod = getVmClass().getDeclaredMethod(Identifier.TO_STRING); var toStringMethod = getVmClass().getDeclaredMethod(Identifier.TO_STRING);
assert toStringMethod != null; assert toStringMethod != null;
var callNode = DirectCallNode.create(toStringMethod.getCallTarget()); var callNode = DirectCallNode.create(toStringMethod.getCallTarget());
@@ -77,7 +77,8 @@ public final class VmRegex extends VmValue {
} }
@Override @Override
public String toString() { @TruffleBoundary
public String toPklString() {
var builder = new StringBuilder(); var builder = new StringBuilder();
builder.append("Regex("); builder.append("Regex(");
ValueFormatter.withCustomStringDelimiters().formatStringValue(pattern.pattern(), "", builder); ValueFormatter.withCustomStringDelimiters().formatStringValue(pattern.pattern(), "", builder);
@@ -308,7 +308,7 @@ public final class VmTypeAlias extends VmValue {
} }
@Override @Override
public String toString() { public String toPklString() {
return qualifiedName.startsWith("pkl.base#") ? simpleName : qualifiedName; return qualifiedName.startsWith("pkl.base#") ? simpleName : qualifiedName;
} }
@@ -98,7 +98,7 @@ public final class VmUndefinedValueException extends VmEvalException {
} }
builder.append(Lexer.maybeQuoteIdentifier(pathPart.toString())); builder.append(Lexer.maybeQuoteIdentifier(pathPart.toString()));
} else { } else {
builder.append('[').append(pathPart).append(']'); builder.append('[').append(VmUtils.toPklString(pathPart)).append(']');
} }
} }
} }
@@ -1125,6 +1125,18 @@ public final class VmUtils {
return value; return value;
} }
public static String toPklString(Object value) {
if (value instanceof VmValue vmValue) {
return vmValue.toPklString();
}
return toString(value);
}
@TruffleBoundary
private static String toString(Object value) {
return value.toString();
}
public static boolean isPklBug(VmStackOverflowException e) { public static boolean isPklBug(VmStackOverflowException e) {
// There's no good way to tell if a StackOverflowError came from Pkl, or from our // There's no good way to tell if a StackOverflowError came from Pkl, or from our
// implementation. // implementation.
@@ -15,9 +15,12 @@
*/ */
package org.pkl.core.runtime; package org.pkl.core.runtime;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import org.jspecify.annotations.Nullable; import org.jspecify.annotations.Nullable;
public abstract class VmValue { public abstract class VmValue {
private final VmValueRenderer vmValueRenderer = VmValueRenderer.multiLine(Integer.MAX_VALUE);
public abstract VmClass getVmClass(); public abstract VmClass getVmClass();
public VmTyped getPrototype() { public VmTyped getPrototype() {
@@ -90,4 +93,16 @@ public abstract class VmValue {
/** Enables calling `vmValue.equals()` when not behind a Truffle boundary. */ /** Enables calling `vmValue.equals()` when not behind a Truffle boundary. */
@Override @Override
public abstract boolean equals(Object obj); public abstract boolean equals(Object obj);
@TruffleBoundary
public abstract String toPklString();
/**
* Override default implementation because it calls {@link #hashCode()}, which will do object eval
* on certain types.
*/
@Override
public final String toString() {
return vmValueRenderer.render(this);
}
} }
@@ -111,12 +111,12 @@ public final class VmValueRenderer {
@Override @Override
public void visitDuration(VmDuration value) { public void visitDuration(VmDuration value) {
append(value); append(value.toPklString());
} }
@Override @Override
public void visitDataSize(VmDataSize value) { public void visitDataSize(VmDataSize value) {
append(value); append(value.toPklString());
} }
private void renderByteSize(VmDataSize size) { private void renderByteSize(VmDataSize size) {
@@ -169,12 +169,12 @@ public final class VmValueRenderer {
@Override @Override
public void visitRegex(VmRegex value) { public void visitRegex(VmRegex value) {
append(value); append(value.toPklString());
} }
@Override @Override
public void visitIntSeq(VmIntSeq value) { public void visitIntSeq(VmIntSeq value) {
append(value); append(value.toPklString());
} }
@Override @Override
@@ -258,12 +258,12 @@ public final class VmValueRenderer {
@Override @Override
public void visitClass(VmClass value) { public void visitClass(VmClass value) {
append(value); append(value.toPklString());
} }
@Override @Override
public void visitTypeAlias(VmTypeAlias value) { public void visitTypeAlias(VmTypeAlias value) {
append(value); append(value.toPklString());
} }
@Override @Override
@@ -296,7 +296,7 @@ public final class VmValueRenderer {
} }
private void append(Object value) { private void append(Object value) {
builder.append(value); builder.append(VmUtils.toPklString(value));
checkLengthLimit(); checkLengthLimit();
} }
@@ -1,5 +1,5 @@
/* /*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -38,10 +38,16 @@ public final class AnyNodes {
} }
public abstract static class toString extends ExternalMethod0Node { public abstract static class toString extends ExternalMethod0Node {
@Specialization
@TruffleBoundary
protected String evalString(VmValue vmValue) {
return vmValue.toPklString();
}
@Specialization @Specialization
@TruffleBoundary @TruffleBoundary
protected String evalString(Object self) { protected String evalString(Object self) {
return self.toString(); return VmUtils.toPklString(self);
} }
} }
@@ -50,7 +56,7 @@ public final class AnyNodes {
@Specialization @Specialization
@SuppressWarnings("UnusedParameters") @SuppressWarnings("UnusedParameters")
protected VmNull eval(VmNull self, VmFunction function) { protected VmNull eval(VmNull self, VmFunction ignored) {
return self; return self;
} }
@@ -93,7 +93,9 @@ public final class JsonRendererNodes {
@Override @Override
public void visitFloat(Double value) { public void visitFloat(Double value) {
if (value.isNaN() || value.isInfinite()) { if (value.isNaN() || value.isInfinite()) {
throw new VmExceptionBuilder().evalError("cannotRenderValue", value, name).build(); throw new VmExceptionBuilder()
.evalError("cannotRenderValue", VmUtils.toPklString(value), name)
.build();
} }
builder.append((double) value); builder.append((double) value);
} }
@@ -292,7 +292,8 @@ public final class ListingNodes {
if (!key.equals(0L)) { if (!key.equals(0L)) {
builder.append(separator); builder.append(separator);
} }
builder.append(value); // TODO: use ToStringNode
builder.append(VmUtils.toPklString(value));
return true; return true;
}); });
LoopNode.reportLoopCount(this, self.getLength()); LoopNode.reportLoopCount(this, self.getLength());
@@ -92,17 +92,17 @@ public final class PcfRenderer extends AbstractStringRenderer {
@Override @Override
public void visitDuration(VmDuration value) { public void visitDuration(VmDuration value) {
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitDataSize(VmDataSize value) { public void visitDataSize(VmDataSize value) {
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitBytes(VmBytes value) { public void visitBytes(VmBytes value) {
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
@@ -116,12 +116,12 @@ public final class PcfRenderer extends AbstractStringRenderer {
@Override @Override
public void visitRegex(VmRegex value) { public void visitRegex(VmRegex value) {
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitIntSeq(VmIntSeq value) { public void visitIntSeq(VmIntSeq value) {
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
@@ -307,7 +307,7 @@ public final class PropertiesRendererNodes {
} else { } else {
builder.append( builder.append(
PropertiesUtils.renderPropertiesKeyOrValue( PropertiesUtils.renderPropertiesKeyOrValue(
path.toString(), true, restrictCharset)); VmUtils.toPklString(path), true, restrictCharset));
} }
isFollowing.set(true); isFollowing.set(true);
}); });
@@ -534,31 +534,31 @@ public final class RendererNodes {
@Override @Override
public void visitIntSeq(VmIntSeq value) { public void visitIntSeq(VmIntSeq value) {
writePropertyName(); writePropertyName();
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitPair(VmPair value) { public void visitPair(VmPair value) {
writePropertyName(); writePropertyName();
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitRegex(VmRegex value) { public void visitRegex(VmRegex value) {
writePropertyName(); writePropertyName();
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitNull(VmNull value) { public void visitNull(VmNull value) {
writePropertyName(); writePropertyName();
builder.append(value); builder.append(value.toPklString());
} }
@Override @Override
public void visitReference(VmReference value) { public void visitReference(VmReference value) {
writePropertyName(); writePropertyName();
builder.append(value); builder.append(value.toPklString());
} }
/** /**
@@ -508,4 +508,9 @@ examples {
List(1, "Pigeon", 3.d).reverse() List(1, "Pigeon", 3.d).reverse()
List().reverse() List().reverse()
} }
["join()"] {
List(1.s, 2.s).join(", ")
List(1.mb, 2.gb).join(", ")
}
} }
@@ -0,0 +1,4 @@
output {
value = Map(5.s, "value")
renderer = new PropertiesRenderer {}
}
@@ -440,4 +440,9 @@ examples {
Set(1, "Pigeon", 3.d).reverse() Set(1, "Pigeon", 3.d).reverse()
Set().reverse() Set().reverse()
} }
["join()"] {
Set(1.s, 2.s).join(", ")
Set(1.mb, 2.gb).join(", ")
}
} }
@@ -0,0 +1,3 @@
one {
[5.mb] = throw("ouch")
}
@@ -0,0 +1,3 @@
one {
[5.s] = throw("ouch")
}
@@ -438,4 +438,8 @@ examples {
List(3.d, "Pigeon", 1) List(3.d, "Pigeon", 1)
List() List()
} }
["join()"] {
"1.s, 2.s"
"1.mb, 2.gb"
}
} }
@@ -0,0 +1 @@
5.s = value
@@ -377,4 +377,8 @@ examples {
List(3.d, "Pigeon", 1) List(3.d, "Pigeon", 1)
List() List()
} }
["join()"] {
"1.s, 2.s"
"1.mb, 2.gb"
}
} }
@@ -0,0 +1,14 @@
–– Pkl Error ––
ouch
x | [5.mb] = throw("ouch")
^^^^^^^^^^^^^
at stackTraceWithDataSizeEntryName#one[5.mb] (file:///$snippetsDir/input/errors/stackTraceWithDataSizeEntryName.pkl)
xxx | renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8")
^^^^
at pkl.base#Module.output.bytes (pkl:base)
@@ -0,0 +1,14 @@
–– Pkl Error ––
ouch
x | [5.s] = throw("ouch")
^^^^^^^^^^^^^
at stackTraceWithDurationEntryName#one[5.s] (file:///$snippetsDir/input/errors/stackTraceWithDurationEntryName.pkl)
xxx | renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8")
^^^^
at pkl.base#Module.output.bytes (pkl:base)
@@ -94,6 +94,13 @@ class EvaluatorTest {
checkModule(module) checkModule(module)
} }
@Test
fun `evaluate text with dynamic entry keyed by a non-identifier value`() {
val module = evaluator.evaluate(text("""result = new Dynamic { [5.s] = "value" }"""))
val result = module.getProperty("result") as PObject
assertThat(result.properties).containsEntry("5.s", "value")
}
@Test @Test
fun `evaluate text with relative import`() { fun `evaluate text with relative import`() {
val e = assertThrows<PklException> { evaluator.evaluate(text("import \"foo.bar\"")) } val e = assertThrows<PklException> { evaluator.evaluate(text("import \"foo.bar\"")) }
@@ -42,7 +42,7 @@ class ErrorMessagesTest {
override fun equals(obj: Any?): Boolean = this === obj override fun equals(obj: Any?): Boolean = this === obj
override fun toString(): String { override fun toPklString(): String {
force(true) force(true)
return "lazy" return "lazy"
} }