Skip to content

Commit 8fa396a

Browse files
HT154HT154
committed
review feedback round 4
1 parent 2453d72 commit 8fa396a

11 files changed

Lines changed: 66 additions & 53 deletions

File tree

‎.idea/codeStyles/Project.xml‎

Lines changed: 0 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkl-core/src/main/java/org/pkl/core/ast/internal/ToStringNode.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717

1818
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
1919
import com.oracle.truffle.api.dsl.Cached;
20-
import com.oracle.truffle.api.dsl.Cached.Shared;
2120
import com.oracle.truffle.api.dsl.Fallback;
2221
import com.oracle.truffle.api.dsl.Specialization;
2322
import com.oracle.truffle.api.frame.VirtualFrame;
23+
import com.oracle.truffle.api.nodes.DirectCallNode;
2424
import com.oracle.truffle.api.source.SourceSection;
2525
import org.pkl.core.ast.ExpressionNode;
2626
import org.pkl.core.ast.MemberLookupMode;
@@ -61,7 +61,7 @@ protected String evalBoolean(boolean value) {
6161
protected String evalTyped(
6262
VirtualFrame frame,
6363
VmTyped value,
64-
@Shared("invokeToString") @Cached(value = "createInvokeNode()", neverDefault = true)
64+
@Cached(value = "createInvokeNode()", neverDefault = true)
6565
InvokeMethodVirtualNode invokeNode) {
6666
return (String) invokeNode.executeWith(frame, value, value.getVmClass());
6767
}
@@ -70,9 +70,9 @@ protected String evalTyped(
7070
protected String evalReference(
7171
VirtualFrame frame,
7272
VmReference value,
73-
@Shared("invokeToString") @Cached(value = "createInvokeNode()", neverDefault = true)
74-
InvokeMethodVirtualNode invokeNode) {
75-
return (String) invokeNode.executeWith(frame, value, value.getVmClass());
73+
@Cached(value = "createReferenceCallNode(value)", neverDefault = true)
74+
DirectCallNode callNode) {
75+
return (String) callNode.call(value, value.getVmClass().getPrototype());
7676
}
7777

7878
@Fallback
@@ -91,4 +91,10 @@ protected InvokeMethodVirtualNode createInvokeNode() {
9191
null,
9292
null);
9393
}
94+
95+
protected DirectCallNode createReferenceCallNode(VmReference reference) {
96+
var toStringMethod = reference.getVmClass().getDeclaredMethod(Identifier.TO_STRING);
97+
assert toStringMethod != null;
98+
return DirectCallNode.create(toStringMethod.getCallTarget());
99+
}
94100
}

‎pkl-core/src/main/java/org/pkl/core/runtime/VmReference.java‎

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,22 @@
1515
*/
1616
package org.pkl.core.runtime;
1717

18-
import com.oracle.truffle.api.frame.VirtualFrame;
1918
import com.oracle.truffle.api.nodes.DirectCallNode;
20-
import com.oracle.truffle.api.source.SourceSection;
2119
import java.util.ArrayList;
2220
import java.util.Collections;
2321
import java.util.Comparator;
2422
import java.util.HashSet;
2523
import java.util.List;
2624
import java.util.Set;
27-
import org.organicdesign.fp.collections.RrbTree;
28-
import org.organicdesign.fp.collections.RrbTree.ImRrbt;
25+
import org.jspecify.annotations.Nullable;
2926
import org.pkl.core.Composite;
3027
import org.pkl.core.PClass;
3128
import org.pkl.core.PClassInfo;
3229
import org.pkl.core.PType;
3330
import org.pkl.core.Reference;
3431
import org.pkl.core.TypeAlias;
35-
import org.pkl.core.ast.ExpressionNode;
36-
import org.pkl.core.ast.MemberLookupMode;
37-
import org.pkl.core.ast.expression.member.InvokeMethodVirtualNodeGen;
38-
import org.pkl.core.util.Nullable;
32+
import org.pkl.core.util.paguro.RrbTree;
33+
import org.pkl.core.util.paguro.RrbTree.ImRrbt;
3934

4035
public final class VmReference extends VmValue {
4136

@@ -59,6 +54,14 @@ public final class VmReference extends VmValue {
5954
}
6055
}
6156

57+
private static final DirectCallNode toStringCallNode;
58+
59+
static {
60+
var toStringMethod = RefModule.getReferenceClass().getMethod(Identifier.TO_STRING);
61+
assert toStringMethod != null;
62+
toStringCallNode = DirectCallNode.create(toStringMethod.getCallTarget());
63+
}
64+
6265
private static VmTyped newAccess(@Nullable String property, @Nullable Object key) {
6366
return new VmObjectBuilder()
6467
.addProperty(Identifier.PROPERTY, property == null ? VmNull.withoutDefault() : property)
@@ -202,7 +205,8 @@ private static void getCandidatePropertyType(PType type, String property, Set<PT
202205
// containing a property can be subclassed.
203206
// And this can't check prop.getOwner().isExternal() because fully overriding the property with
204207
// a new type annotation means the owner isn't Module.
205-
if (clazz.getPClass().isModuleClass() && property.equals("output")) return;
208+
if (clazz.getPClass().isSubclassOf(BaseModule.getModuleClass().export())
209+
&& property.equals("output")) return;
206210

207211
var prop = clazz.getPClass().getAllProperties().get(property);
208212
// restriction: cannot reference external properties
@@ -432,22 +436,7 @@ public int hashCode() {
432436
return result;
433437
}
434438

435-
public String toPklString(@Nullable VirtualFrame frame, @Nullable SourceSection sourceSection) {
436-
if (frame == null) {
437-
frame = VmUtils.createEmptyMaterializedFrame();
438-
}
439-
if (sourceSection == null) {
440-
sourceSection = VmUtils.unavailableSourceSection();
441-
}
442-
443-
return (String)
444-
InvokeMethodVirtualNodeGen.create(
445-
sourceSection,
446-
Identifier.TO_STRING,
447-
new ExpressionNode[] {},
448-
MemberLookupMode.EXPLICIT_RECEIVER,
449-
null,
450-
null)
451-
.executeWith(frame, this, getVmClass());
439+
public String toPklString() {
440+
return (String) toStringCallNode.call(this, getVmClass().getPrototype());
452441
}
453442
}

‎pkl-core/src/main/java/org/pkl/core/stdlib/AbstractStringRenderer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected void decreaseIndent() {
5454

5555
@Override
5656
public void visitReference(VmReference value) {
57-
visitString(value.toPklString(null, currSourceSection));
57+
visitString(value.toPklString());
5858
}
5959

6060
// override these to mark them final

‎pkl-core/src/main/java/org/pkl/core/stdlib/ref/package-info.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
@NonnullByDefault
1+
@NullMarked
22
package org.pkl.core.stdlib.ref;
33

4-
import org.pkl.core.util.NonnullByDefault;
4+
import org.jspecify.annotations.NullMarked;

‎pkl-core/src/test/files/LanguageSnippetTests/input-helper/errors/ReferencedModuleWithOutputOverride.pkl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
open module ReferencedModuleWithOutputOverride
2+
13
foo: String
24

35
hidden output: ModuleOutput = new {

‎pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference18.pkl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import "pkl:ref"
22

3-
class A {
4-
foo: String | Int | Boolean
5-
}
3+
import ".../input-helper/errors/ReferencedModuleWithOutputOverride.pkl"
4+
5+
class ModuleSubclass extends ReferencedModuleWithOutputOverride
66

77
class D extends ref.Domain {
88
function referenceToString(data: Any, path: List<ref.Access>): String = throw("not supported")
99
}
1010
local d: D = new {}
1111

12-
test = ref.Reference(d, A, "").foo as ref.Reference<D, Int | Boolean>
12+
test = ref.Reference(d, ModuleSubclass, "").output

‎pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference19.pkl‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import "pkl:ref"
2+
3+
class A {
4+
foo: String | Int | Boolean
5+
}
6+
7+
class D extends ref.Domain {
8+
function referenceToString(data: Any, path: List<ref.Access>): String = throw("not supported")
9+
}
10+
local d: D = new {}
11+
12+
test = ref.Reference(d, A, "").foo as ref.Reference<D, Int | Boolean>

‎pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference20.pkl‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import "pkl:ref"
2+
3+
class A {
4+
foo: String | Int | Boolean
5+
}
6+
7+
class D extends ref.Domain {
8+
function referenceToString(data: Any, path: List<ref.Access>): String = throw("not supported")
9+
}
10+
local d: D = new {}
11+
12+
local test = ref.Reference(d, A, "").foo
13+
testInterpolation = "test:\(test)"
14+
15+
// this tests that the interpolation appears in the output when referenceToString throws

‎pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference18.err‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
–– Pkl Error ––
2-
Expected value of type `pkl.ref#Reference<reference18#D, Int | Boolean>`, but got type `pkl.ref#Reference<reference18#D, Boolean | Int | String>`.
3-
Value: Reference(new D {}, Boolean | Int | String, "").foo
2+
Cannot find property `output` in object of type `pkl.ref#Reference<reference18#D, reference18#ModuleSubclass>`.
43

5-
xx | test = ref.Reference(d, A, "").foo as ref.Reference<D, Int | Boolean>
6-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
xx | test = ref.Reference(d, ModuleSubclass, "").output
5+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
76
at reference18#test (file:///$snippetsDir/input/errors/reference18.pkl)
87

98
xxx | renderer.renderDocument(value)

0 commit comments

Comments
 (0)