From f5042c85bdc0e103f525a8b306e746256337689a Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Fri, 19 Apr 2024 12:30:57 -0700 Subject: [PATCH 1/8] Split MemberNode into (Regular/Shared)MemberNode SharedMemberNode enables generating non-constant object members at run time without generating an unbounded number of Truffle root nodes. --- .../java/org/pkl/core/ast/MemberNode.java | 24 +------ .../org/pkl/core/ast/member/FunctionNode.java | 3 +- .../ast/member/LocalTypedPropertyNode.java | 3 +- .../pkl/core/ast/member/ObjectMethodNode.java | 3 +- .../core/ast/member/RegularMemberNode.java | 52 +++++++++++++++ .../pkl/core/ast/member/SharedMemberNode.java | 64 +++++++++++++++++++ .../ast/member/TypeCheckedPropertyNode.java | 3 +- .../core/ast/member/TypedPropertyNode.java | 3 +- .../ast/member/UntypedObjectMemberNode.java | 3 +- 9 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 pkl-core/src/main/java/org/pkl/core/ast/member/RegularMemberNode.java create mode 100644 pkl-core/src/main/java/org/pkl/core/ast/member/SharedMemberNode.java diff --git a/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java b/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java index a13d48e52..3f4d82d1b 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java @@ -20,35 +20,22 @@ import com.oracle.truffle.api.source.SourceSection; import java.util.function.Function; import org.pkl.core.ast.member.DefaultPropertyBodyNode; -import org.pkl.core.ast.member.Member; import org.pkl.core.runtime.VmExceptionBuilder; import org.pkl.core.runtime.VmLanguage; import org.pkl.core.runtime.VmUtils; import org.pkl.core.util.Nullable; public abstract class MemberNode extends PklRootNode { - protected final Member member; @Child protected ExpressionNode bodyNode; protected MemberNode( - @Nullable VmLanguage language, - FrameDescriptor descriptor, - Member member, - ExpressionNode bodyNode) { + @Nullable VmLanguage language, FrameDescriptor descriptor, ExpressionNode bodyNode) { super(language, descriptor); - this.member = member; this.bodyNode = bodyNode; } - @Override - public final SourceSection getSourceSection() { - return member.getSourceSection(); - } - - public final SourceSection getHeaderSection() { - return member.getHeaderSection(); - } + public abstract SourceSection getHeaderSection(); public final SourceSection getBodySection() { return bodyNode.getSourceSection(); @@ -58,11 +45,6 @@ public final ExpressionNode getBodyNode() { return bodyNode; } - @Override - public final String getName() { - return member.getQualifiedName(); - } - public final void replaceBody(Function replacer) { bodyNode = insert(replacer.apply(bodyNode)); } @@ -72,7 +54,7 @@ protected final Object executeBody(VirtualFrame frame) { } protected final VmExceptionBuilder exceptionBuilder() { - return new VmExceptionBuilder().withSourceSection(member.getHeaderSection()); + return new VmExceptionBuilder().withSourceSection(getHeaderSection()); } /** diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/FunctionNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/FunctionNode.java index 31c7d8791..ef42cda00 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/FunctionNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/FunctionNode.java @@ -27,7 +27,6 @@ import org.pkl.core.PType; import org.pkl.core.TypeParameter; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.ast.VmModifier; import org.pkl.core.ast.type.TypeNode; import org.pkl.core.ast.type.VmTypeMismatchException; @@ -36,7 +35,7 @@ import org.pkl.core.util.Nullable; import org.pkl.core.util.Pair; -public final class FunctionNode extends MemberNode { +public final class FunctionNode extends RegularMemberNode { // Every function (and property) call passes two implicit arguments at positions // frame.getArguments()[0] and [1]: // - the receiver (target) of the call, of type Object (see VmUtils.getReceiver()) diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/LocalTypedPropertyNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/LocalTypedPropertyNode.java index 39a564e7a..87784d7ee 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/LocalTypedPropertyNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/LocalTypedPropertyNode.java @@ -19,7 +19,6 @@ import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.frame.VirtualFrame; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.ast.type.TypeNode; import org.pkl.core.ast.type.UnresolvedTypeNode; import org.pkl.core.ast.type.VmTypeMismatchException; @@ -28,7 +27,7 @@ import org.pkl.core.util.LateInit; import org.pkl.core.util.Nullable; -public final class LocalTypedPropertyNode extends MemberNode { +public final class LocalTypedPropertyNode extends RegularMemberNode { private final VmLanguage language; @Child private UnresolvedTypeNode unresolvedTypeNode; @Child @LateInit private TypeNode typeNode; diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/ObjectMethodNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/ObjectMethodNode.java index 5bae1241f..35a9740e4 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/ObjectMethodNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/ObjectMethodNode.java @@ -21,14 +21,13 @@ import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.frame.VirtualFrame; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.ast.type.TypeNode; import org.pkl.core.ast.type.UnresolvedTypeNode; import org.pkl.core.runtime.*; import org.pkl.core.util.LateInit; import org.pkl.core.util.Nullable; -public final class ObjectMethodNode extends MemberNode { +public final class ObjectMethodNode extends RegularMemberNode { private final VmLanguage language; private final int parameterCount; @Children private final @Nullable UnresolvedTypeNode[] unresolvedParameterTypeNodes; diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/RegularMemberNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/RegularMemberNode.java new file mode 100644 index 000000000..6c3a122c8 --- /dev/null +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/RegularMemberNode.java @@ -0,0 +1,52 @@ +/** + * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * + * 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 org.pkl.core.ast.member; + +import com.oracle.truffle.api.frame.FrameDescriptor; +import com.oracle.truffle.api.source.SourceSection; +import org.pkl.core.ast.ExpressionNode; +import org.pkl.core.ast.MemberNode; +import org.pkl.core.runtime.VmLanguage; +import org.pkl.core.util.Nullable; + +/** A {@code MemberNode} that belongs to a single {@link Member}. */ +public abstract class RegularMemberNode extends MemberNode { + protected final Member member; + + protected RegularMemberNode( + @Nullable VmLanguage language, + FrameDescriptor descriptor, + Member member, + ExpressionNode bodyNode) { + + super(language, descriptor, bodyNode); + this.member = member; + } + + @Override + public final SourceSection getSourceSection() { + return member.getSourceSection(); + } + + public final SourceSection getHeaderSection() { + return member.getHeaderSection(); + } + + @Override + public final String getName() { + return member.getQualifiedName(); + } +} diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/SharedMemberNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/SharedMemberNode.java new file mode 100644 index 000000000..827e9e049 --- /dev/null +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/SharedMemberNode.java @@ -0,0 +1,64 @@ +/** + * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * + * 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 org.pkl.core.ast.member; + +import com.oracle.truffle.api.frame.FrameDescriptor; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.source.SourceSection; +import org.pkl.core.ast.ExpressionNode; +import org.pkl.core.ast.MemberNode; +import org.pkl.core.runtime.VmLanguage; +import org.pkl.core.util.Nullable; + +/** A {@code MemberNode} that is shared between multiple {@linkplain Member members}. */ +public class SharedMemberNode extends MemberNode { + private final SourceSection sourceSection; + private final SourceSection headerSection; + private final @Nullable String qualifiedName; + + public SharedMemberNode( + SourceSection sourceSection, + SourceSection headerSection, + @Nullable String qualifiedName, + @Nullable VmLanguage language, + FrameDescriptor descriptor, + ExpressionNode bodyNode) { + + super(language, descriptor, bodyNode); + this.sourceSection = sourceSection; + this.headerSection = headerSection; + this.qualifiedName = qualifiedName; + } + + @Override + public SourceSection getSourceSection() { + return sourceSection; + } + + public SourceSection getHeaderSection() { + return headerSection; + } + + @Override + public @Nullable String getName() { + return qualifiedName; + } + + @Override + public Object execute(VirtualFrame frame) { + return executeBody(frame); + } +} diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java index c3014d908..27fc4ffbb 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java @@ -23,13 +23,12 @@ import com.oracle.truffle.api.nodes.DirectCallNode; import com.oracle.truffle.api.nodes.IndirectCallNode; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.ast.expression.primary.GetOwnerNode; import org.pkl.core.runtime.*; import org.pkl.core.util.Nullable; /** A property definition that does not have a type annotation but should be type-checked. */ -public abstract class TypeCheckedPropertyNode extends MemberNode { +public abstract class TypeCheckedPropertyNode extends RegularMemberNode { @Child @Executed protected ExpressionNode ownerNode = new GetOwnerNode(); protected TypeCheckedPropertyNode( diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java index 495844a20..e95261bc7 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java @@ -20,12 +20,11 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.DirectCallNode; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.runtime.VmLanguage; import org.pkl.core.runtime.VmUtils; /** A property definition that has a type annotation. */ -public final class TypedPropertyNode extends MemberNode { +public final class TypedPropertyNode extends RegularMemberNode { @Child private DirectCallNode typeCheckCallNode; @TruffleBoundary diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/UntypedObjectMemberNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/UntypedObjectMemberNode.java index 6aeed8797..ea599eab8 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/UntypedObjectMemberNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/UntypedObjectMemberNode.java @@ -18,11 +18,10 @@ import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.frame.VirtualFrame; import org.pkl.core.ast.ExpressionNode; -import org.pkl.core.ast.MemberNode; import org.pkl.core.runtime.VmLanguage; import org.pkl.core.util.Nullable; -public final class UntypedObjectMemberNode extends MemberNode { +public final class UntypedObjectMemberNode extends RegularMemberNode { public UntypedObjectMemberNode( @Nullable VmLanguage language, FrameDescriptor descriptor, From 879d853941b0196c4ad5da0953eabd0d1489c857 Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Fri, 19 Apr 2024 12:39:21 -0700 Subject: [PATCH 2/8] Invert shouldRunTypeCheck to match its name --- pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java | 6 +++--- .../org/pkl/core/ast/member/TypeCheckedPropertyNode.java | 4 ++-- .../java/org/pkl/core/ast/member/TypedPropertyNode.java | 2 +- pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java b/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java index 3f4d82d1b..548b542c6 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java @@ -65,9 +65,9 @@ protected final VmExceptionBuilder exceptionBuilder() { * org.pkl.core.runtime.VmUtils#SKIP_TYPECHECK_MARKER}. IDEA: might be more appropriate to only * skip constraints check */ - protected final boolean shouldRunTypecheck(VirtualFrame frame) { - return frame.getArguments().length == 4 - && frame.getArguments()[3] == VmUtils.SKIP_TYPECHECK_MARKER; + protected final boolean shouldRunTypeCheck(VirtualFrame frame) { + return frame.getArguments().length != 4 + || frame.getArguments()[3] != VmUtils.SKIP_TYPECHECK_MARKER; } public boolean isUndefined() { diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java index 27fc4ffbb..0b0038fa0 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java @@ -54,7 +54,7 @@ protected Object evalTypedObjectCached( var result = executeBody(frame); // TODO: propagate SUPER_CALL_MARKER to disable constraint (but not type) check - if (callNode != null && !shouldRunTypecheck(frame)) { + if (callNode != null && shouldRunTypeCheck(frame)) { callNode.call(VmUtils.getReceiverOrNull(frame), property.getOwner(), result); } @@ -67,7 +67,7 @@ protected Object eval( var result = executeBody(frame); - if (!shouldRunTypecheck(frame)) { + if (shouldRunTypeCheck(frame)) { var property = getProperty(owner.getVmClass()); var typeAnnNode = property.getTypeNode(); if (typeAnnNode != null) { diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java index e95261bc7..1a139da70 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java @@ -45,7 +45,7 @@ public TypedPropertyNode( @Override public Object execute(VirtualFrame frame) { var propertyValue = executeBody(frame); - if (!shouldRunTypecheck(frame)) { + if (shouldRunTypeCheck(frame)) { typeCheckCallNode.call(VmUtils.getReceiver(frame), VmUtils.getOwner(frame), propertyValue); } return propertyValue; diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java index a367198a6..ff1c35620 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java @@ -62,7 +62,7 @@ import org.pkl.core.util.Nullable; public final class VmUtils { - /** See {@link MemberNode#shouldRunTypecheck(VirtualFrame)}. */ + /** See {@link MemberNode#shouldRunTypeCheck(VirtualFrame)}. */ @SuppressWarnings("JavadocReference") public static final Object SKIP_TYPECHECK_MARKER = new Object(); From 87421114eb5bcfa6bf3951780fa17783605f0e10 Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:22:30 -0700 Subject: [PATCH 3/8] Introduce VmObjectBuilder Introduce VmObjectBuilder, a uniform way to build `VmObject`s whose `ObjectMember`s are determined at run time. Replace some manual object building code with VmObjectBuilder. Add some assertions to fix IntelliJ warnings. --- .../java/org/pkl/core/runtime/VmList.java | 14 ++++ .../main/java/org/pkl/core/runtime/VmMap.java | 26 +++++++ .../org/pkl/core/runtime/VmObjectBuilder.java | 75 +++++++++++++++++++ .../main/java/org/pkl/core/runtime/VmSet.java | 14 ++++ .../org/pkl/core/stdlib/base/ListNodes.java | 26 +------ .../org/pkl/core/stdlib/base/MapNodes.java | 14 +--- .../org/pkl/core/stdlib/base/SetNodes.java | 28 +------ 7 files changed, 134 insertions(+), 63 deletions(-) create mode 100644 pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmList.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmList.java index 8c397798b..9cfbe2c44 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmList.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmList.java @@ -382,6 +382,20 @@ public VmSet toSet() { return VmSet.create(rrbt); } + @TruffleBoundary + public VmListing toListing() { + var builder = new VmObjectBuilder(rrbt.size()); + for (var elem : rrbt) builder.addElement(elem); + return builder.toListing(); + } + + @TruffleBoundary + public VmDynamic toDynamic() { + var builder = new VmObjectBuilder(rrbt.size()); + for (var elem : rrbt) builder.addElement(elem); + return builder.toDynamic(); + } + @Override @TruffleBoundary public void force(boolean allowUndefinedValues) { diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java index e2fa42e77..98d816ca9 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java @@ -225,6 +225,32 @@ public void force(boolean allowUndefinedValues) { } } + @TruffleBoundary + public VmMapping toMapping() { + var builder = new VmObjectBuilder(keyOrder.size()); + for (var key : keyOrder) { + var value = map.get(key); + assert value != null; + builder.addEntry(key, value); + } + return builder.toMapping(); + } + + @TruffleBoundary + public VmDynamic toDynamic() { + var builder = new VmObjectBuilder(keyOrder.size()); + for (var key : keyOrder) { + var value = map.get(key); + assert value != null; + if (key instanceof String) { + builder.addProperty(Identifier.get((String) key), value); + } else { + builder.addEntry(key, value); + } + } + return builder.toDynamic(); + } + @Override @TruffleBoundary public Map export() { diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java new file mode 100644 index 000000000..5ef798bb4 --- /dev/null +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java @@ -0,0 +1,75 @@ +/** + * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * + * 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 org.pkl.core.runtime; + +import org.graalvm.collections.EconomicMap; +import org.pkl.core.ast.member.ObjectMember; +import org.pkl.core.util.EconomicMaps; + +/** + * A builder for {@link VmObject}s whose {@link ObjectMember}s are determined at run time. + */ +public final class VmObjectBuilder { + private final EconomicMap members; + private int elementCount = 0; + + public VmObjectBuilder() { + members = EconomicMaps.create(); + } + + public VmObjectBuilder(int initialSize) { + members = EconomicMaps.create(initialSize); + } + + public VmObjectBuilder addProperty(Identifier name, Object value) { + EconomicMaps.put(members, name, VmUtils.createSyntheticObjectProperty(name, "", value)); + return this; + } + + public VmObjectBuilder addElement(Object value) { + EconomicMaps.put(members, (long) elementCount++, VmUtils.createSyntheticObjectElement("", value)); + return this; + } + + public VmObjectBuilder addEntry(Object key, Object value) { + EconomicMaps.put(members, key, VmUtils.createSyntheticObjectEntry("", value)); + return this; + } + + public VmListing toListing() { + return new VmListing( + VmUtils.createEmptyMaterializedFrame(), + BaseModule.getListingClass().getPrototype(), + members, + elementCount + ); + } + + public VmMapping toMapping() { + return new VmMapping( + VmUtils.createEmptyMaterializedFrame(), + BaseModule.getMappingClass().getPrototype(), + members); + } + + public VmDynamic toDynamic() { + return new VmDynamic( + VmUtils.createEmptyMaterializedFrame(), + BaseModule.getDynamicClass().getPrototype(), + members, + elementCount); + } +} diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmSet.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmSet.java index e00b7b32e..89d793730 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmSet.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmSet.java @@ -282,6 +282,20 @@ public VmSet toSet() { return this; } + @TruffleBoundary + public VmListing toListing() { + var builder = new VmObjectBuilder(elementOrder.size()); + for (var elem : elementOrder) builder.addElement(elem); + return builder.toListing(); + } + + @TruffleBoundary + public VmDynamic toDynamic() { + var builder = new VmObjectBuilder(elementOrder.size()); + for (var elem : elementOrder) builder.addElement(elem); + return builder.toDynamic(); + } + @Override @TruffleBoundary public void force(boolean allowUndefinedValues) { diff --git a/pkl-core/src/main/java/org/pkl/core/stdlib/base/ListNodes.java b/pkl-core/src/main/java/org/pkl/core/stdlib/base/ListNodes.java index 11b1a199b..1eab8dddd 100644 --- a/pkl-core/src/main/java/org/pkl/core/stdlib/base/ListNodes.java +++ b/pkl-core/src/main/java/org/pkl/core/stdlib/base/ListNodes.java @@ -19,18 +19,15 @@ import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.nodes.LoopNode; -import org.graalvm.collections.EconomicMap; import org.pkl.core.ast.expression.binary.*; import org.pkl.core.ast.internal.IsInstanceOfNode; import org.pkl.core.ast.internal.IsInstanceOfNodeGen; import org.pkl.core.ast.lambda.*; -import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.runtime.*; import org.pkl.core.stdlib.*; import org.pkl.core.stdlib.base.CollectionNodes.CompareByNode; import org.pkl.core.stdlib.base.CollectionNodes.CompareNode; import org.pkl.core.stdlib.base.CollectionNodes.CompareWithNode; -import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.EconomicSets; // duplication between ListNodes and SetNodes is "intentional" @@ -1295,11 +1292,7 @@ public abstract static class toListing extends ExternalMethod0Node { @Specialization @TruffleBoundary protected VmListing eval(VmList self) { - return new VmListing( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getListingClass().getPrototype(), - toObjectMembers(self), - self.getLength()); + return self.toListing(); } } @@ -1307,22 +1300,7 @@ public abstract static class toDynamic extends ExternalMethod0Node { @Specialization @TruffleBoundary protected VmDynamic eval(VmList self) { - return new VmDynamic( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getDynamicClass().getPrototype(), - toObjectMembers(self), - self.getLength()); + return self.toDynamic(); } } - - private static EconomicMap toObjectMembers(VmList self) { - var result = EconomicMaps.create(self.getLength()); - - for (long idx = 0; idx < self.getLength(); idx++) { - EconomicMaps.put( - result, idx, VmUtils.createSyntheticObjectElement(String.valueOf(idx), self.get(idx))); - } - - return result; - } } diff --git a/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java b/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java index 5bbb129eb..76cd11251 100644 --- a/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java +++ b/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java @@ -287,19 +287,7 @@ protected VmTyped eval(VmMap self, VmClass clazz) { public abstract static class toMapping extends ExternalMethod0Node { @Specialization protected VmMapping eval(VmMap self) { - var members = EconomicMaps.create(self.getLength()); - - for (var entry : self) { - EconomicMaps.put( - members, - VmUtils.getKey(entry), - VmUtils.createSyntheticObjectEntry("", VmUtils.getValue(entry))); - } - - return new VmMapping( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getMappingClass().getPrototype(), - members); + return self.toMapping(); } } } diff --git a/pkl-core/src/main/java/org/pkl/core/stdlib/base/SetNodes.java b/pkl-core/src/main/java/org/pkl/core/stdlib/base/SetNodes.java index 95d3d86bf..0be7f1c19 100644 --- a/pkl-core/src/main/java/org/pkl/core/stdlib/base/SetNodes.java +++ b/pkl-core/src/main/java/org/pkl/core/stdlib/base/SetNodes.java @@ -19,7 +19,6 @@ import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.nodes.LoopNode; -import org.graalvm.collections.EconomicMap; import org.pkl.core.ast.expression.binary.GreaterThanNode; import org.pkl.core.ast.expression.binary.GreaterThanNodeGen; import org.pkl.core.ast.expression.binary.LessThanNode; @@ -27,13 +26,11 @@ import org.pkl.core.ast.internal.IsInstanceOfNode; import org.pkl.core.ast.internal.IsInstanceOfNodeGen; import org.pkl.core.ast.lambda.*; -import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.runtime.*; import org.pkl.core.stdlib.*; import org.pkl.core.stdlib.base.CollectionNodes.CompareByNode; import org.pkl.core.stdlib.base.CollectionNodes.CompareNode; import org.pkl.core.stdlib.base.CollectionNodes.CompareWithNode; -import org.pkl.core.util.EconomicMaps; // duplication between ListNodes and SetNodes is "intentional" // (sharing nodes between VmCollection subtypes results in @@ -1052,11 +1049,7 @@ public abstract static class toListing extends ExternalMethod0Node { @Specialization @TruffleBoundary protected VmListing eval(VmSet self) { - return new VmListing( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getListingClass().getPrototype(), - toObjectMembers(self), - self.getLength()); + return self.toListing(); } } @@ -1064,11 +1057,7 @@ public abstract static class toDynamic extends ExternalMethod0Node { @Specialization @TruffleBoundary protected VmDynamic eval(VmSet self) { - return new VmDynamic( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getDynamicClass().getPrototype(), - toObjectMembers(self), - self.getLength()); + return self.toDynamic(); } } @@ -1110,17 +1099,4 @@ protected VmSet eval(VmSet self, VmSet other) { return builder.build(); } } - - private static EconomicMap toObjectMembers(VmSet self) { - var result = EconomicMaps.create(self.getLength()); - - long idx = 0; - for (var element : self) { - EconomicMaps.put( - result, idx, VmUtils.createSyntheticObjectElement(String.valueOf(idx), element)); - idx += 1; - } - - return result; - } } From a8b7047e828ace5199627909c757430c7e53142d Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:09:55 -0700 Subject: [PATCH 4/8] Improve implementation of globbed read/import nodes - Leverage SharedMemberNode to have a single Truffle root node per globbed read/import expression instead of one root node per resolved glob element. - Remove caching in ReadGlobNode because it only works correctly if glob pattern is a string *constant*. - Remove caching in StaticReadNode (now ReadGlobElementNode/ImportGlobElementNode) because it seems unnecessary and the implementation doesn't look quite right. --- .../unary/ImportGlobElementNode.java | 66 ++++++++++++++ .../ast/expression/unary/ImportGlobNode.java | 64 +++++-------- .../expression/unary/ReadGlobElementNode.java | 52 +++++++++++ .../ast/expression/unary/ReadGlobNode.java | 90 +++++++------------ .../ast/expression/unary/StaticReadNode.java | 50 ----------- .../org/pkl/core/runtime/ModuleCache.java | 7 +- .../org/pkl/core/runtime/ResourceManager.java | 16 ++-- .../java/org/pkl/core/runtime/VmLanguage.java | 3 +- .../org/pkl/core/runtime/VmObjectBuilder.java | 45 ++++++---- .../java/org/pkl/core/util/GlobResolver.java | 18 ++-- .../input/basic/readGlob.pkl | 7 ++ .../output/basic/readGlob.pcf | 40 +++++++++ 12 files changed, 266 insertions(+), 192 deletions(-) create mode 100644 pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java create mode 100644 pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java delete mode 100644 pkl-core/src/main/java/org/pkl/core/ast/expression/unary/StaticReadNode.java diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java new file mode 100644 index 000000000..8873ac775 --- /dev/null +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java @@ -0,0 +1,66 @@ +/** + * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * + * 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 org.pkl.core.ast.expression.unary; + +import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.source.SourceSection; +import java.util.Map; +import org.pkl.core.SecurityManagerException; +import org.pkl.core.ast.ExpressionNode; +import org.pkl.core.http.HttpClientInitException; +import org.pkl.core.module.ResolvedModuleKey; +import org.pkl.core.packages.PackageLoadError; +import org.pkl.core.runtime.VmContext; +import org.pkl.core.runtime.VmLanguage; +import org.pkl.core.runtime.VmObjectLike; +import org.pkl.core.runtime.VmTyped; +import org.pkl.core.runtime.VmUtils; +import org.pkl.core.util.GlobResolver.ResolvedGlobElement; + +/** Used by {@link ReadGlobNode}. */ +public final class ImportGlobElementNode extends ExpressionNode { + private final VmLanguage language; + private final ResolvedModuleKey currentModule; + + public ImportGlobElementNode(SourceSection sourceSection, VmLanguage language, ResolvedModuleKey currentModule) { + super(sourceSection); + this.language = language; + this.currentModule = currentModule; + } + + @Override + public Object executeGeneric(VirtualFrame frame) { + var mapping = VmUtils.getObjectReceiver(frame); + var path = (String) VmUtils.getMemberKey(frame); + return importModule(mapping, path); + } + + @TruffleBoundary + private VmTyped importModule(VmObjectLike mapping, String path) { + @SuppressWarnings("unchecked") + var globElements = (Map) mapping.getExtraStorage(); + var importUri = globElements.get(path).getUri(); + var context = VmContext.get(this); + try { + context.getSecurityManager().checkImportModule(currentModule.getUri(), importUri); + var moduleToImport = context.getModuleResolver().resolve(importUri, this); + return language.loadModule(moduleToImport, this); + } catch (SecurityManagerException | PackageLoadError | HttpClientInitException e) { + throw exceptionBuilder().withCause(e).build(); + } + } +} diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java index 3d4cd2b6d..2893e14b3 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java @@ -17,42 +17,32 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; -import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.source.SourceSection; import java.io.IOException; import java.net.URI; -import java.util.List; -import org.graalvm.collections.EconomicMap; import org.pkl.core.SecurityManagerException; -import org.pkl.core.ast.VmModifier; -import org.pkl.core.ast.member.ObjectMember; -import org.pkl.core.ast.member.UntypedObjectMemberNode; +import org.pkl.core.ast.member.SharedMemberNode; import org.pkl.core.http.HttpClientInitException; import org.pkl.core.module.ResolvedModuleKey; import org.pkl.core.packages.PackageLoadError; -import org.pkl.core.runtime.BaseModule; import org.pkl.core.runtime.VmContext; import org.pkl.core.runtime.VmLanguage; import org.pkl.core.runtime.VmMapping; -import org.pkl.core.runtime.VmUtils; -import org.pkl.core.util.EconomicMaps; +import org.pkl.core.runtime.VmObjectBuilder; import org.pkl.core.util.GlobResolver; import org.pkl.core.util.GlobResolver.InvalidGlobPatternException; -import org.pkl.core.util.GlobResolver.ResolvedGlobElement; import org.pkl.core.util.LateInit; @NodeInfo(shortName = "import*") public class ImportGlobNode extends AbstractImportNode { - private final VmLanguage language; - private final ResolvedModuleKey currentModule; - private final String globPattern; + private final SharedMemberNode importGlobElementNode; - @CompilationFinal @LateInit private VmMapping importedMapping; + @CompilationFinal @LateInit private VmMapping importsMapping; public ImportGlobNode( VmLanguage language, @@ -61,36 +51,21 @@ public ImportGlobNode( URI importUri, String globPattern) { super(sourceSection, importUri); - this.language = language; this.currentModule = currentModule; this.globPattern = globPattern; - } - - @TruffleBoundary - private EconomicMap buildMembers( - FrameDescriptor frameDescriptor, List uris) { - var members = EconomicMaps.create(); - for (var entry : uris) { - var readNode = - new ImportNode( - language, VmUtils.unavailableSourceSection(), currentModule, entry.getUri()); - var member = - new ObjectMember( - VmUtils.unavailableSourceSection(), - VmUtils.unavailableSourceSection(), - VmModifier.ENTRY, - null, - ""); - var memberNode = new UntypedObjectMemberNode(language, frameDescriptor, member, readNode); - member.initMemberNode(memberNode); - EconomicMaps.put(members, entry.getPath(), member); - } - return members; + importGlobElementNode = + new SharedMemberNode( + sourceSection, + sourceSection, + "", + language, + new FrameDescriptor(), + new ImportGlobElementNode(sourceSection, language, currentModule)); } @Override public Object executeGeneric(VirtualFrame frame) { - if (importedMapping == null) { + if (importsMapping == null) { CompilerDirectives.transferToInterpreterAndInvalidate(); var context = VmContext.get(this); try { @@ -101,17 +76,18 @@ public Object executeGeneric(VirtualFrame frame) { .evalError("cannotGlobUri", importUri, importUri.getScheme()) .build(); } - var uris = + var resolvedElements = GlobResolver.resolveGlob( securityManager, moduleKey, currentModule.getOriginal(), currentModule.getUri(), globPattern); - var members = buildMembers(frame.getFrameDescriptor(), uris); - importedMapping = - new VmMapping( - frame.materialize(), BaseModule.getMappingClass().getPrototype(), members); + var builder = new VmObjectBuilder(); + for (var entry : resolvedElements.entrySet()) { + builder.addEntry(entry.getKey(), importGlobElementNode); + } + importsMapping = builder.toMapping(resolvedElements); } catch (IOException e) { throw exceptionBuilder().evalError("ioErrorResolvingGlob", importUri).withCause(e).build(); } catch (SecurityManagerException | HttpClientInitException e) { @@ -125,6 +101,6 @@ public Object executeGeneric(VirtualFrame frame) { .build(); } } - return importedMapping; + return importsMapping; } } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java new file mode 100644 index 000000000..d4e394c1f --- /dev/null +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java @@ -0,0 +1,52 @@ +/** + * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * + * 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 org.pkl.core.ast.expression.unary; + +import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.source.SourceSection; +import java.util.Map; +import org.pkl.core.ast.ExpressionNode; +import org.pkl.core.runtime.VmContext; +import org.pkl.core.runtime.VmObjectLike; +import org.pkl.core.runtime.VmUtils; +import org.pkl.core.util.GlobResolver.ResolvedGlobElement; + +/** Used by {@link ReadGlobNode}. */ +public class ReadGlobElementNode extends ExpressionNode { + public ReadGlobElementNode(SourceSection sourceSection) { + super(sourceSection); + } + + @Override + public Object executeGeneric(VirtualFrame frame) { + var mapping = VmUtils.getObjectReceiver(frame); + var path = (String) VmUtils.getMemberKey(frame); + return readResource(mapping, path); + } + + @TruffleBoundary + private Object readResource(VmObjectLike mapping, String path) { + @SuppressWarnings("unchecked") + var globElements = (Map) mapping.getExtraStorage(); + var resourceUri = globElements.get(path).getUri(); + var resource = VmContext.get(this).getResourceManager().read(resourceUri, this).orElse(null); + if (resource == null) { + throw exceptionBuilder().evalError("cannotFindResource", resourceUri).build(); + } + return resource; + } +} diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java index 9161083a7..eac2ed8e3 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java @@ -15,97 +15,67 @@ */ package org.pkl.core.ast.expression.unary; -import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.FrameDescriptor; -import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.source.SourceSection; import java.net.URI; import java.net.URISyntaxException; -import java.util.List; -import org.graalvm.collections.EconomicMap; -import org.pkl.core.ast.VmModifier; -import org.pkl.core.ast.member.ObjectMember; -import org.pkl.core.ast.member.UntypedObjectMemberNode; +import org.pkl.core.ast.member.SharedMemberNode; import org.pkl.core.module.ModuleKey; -import org.pkl.core.runtime.BaseModule; import org.pkl.core.runtime.VmContext; import org.pkl.core.runtime.VmLanguage; -import org.pkl.core.runtime.VmMapping; -import org.pkl.core.runtime.VmUtils; -import org.pkl.core.util.EconomicMaps; -import org.pkl.core.util.GlobResolver.ResolvedGlobElement; +import org.pkl.core.runtime.VmObjectBuilder; import org.pkl.core.util.IoUtils; -import org.pkl.core.util.LateInit; @NodeInfo(shortName = "read*") public abstract class ReadGlobNode extends UnaryExpressionNode { - private final VmLanguage language; private final ModuleKey currentModule; - - @CompilationFinal @LateInit VmMapping readResult; + private final SharedMemberNode readGlobElementNode; protected ReadGlobNode( VmLanguage language, SourceSection sourceSection, ModuleKey currentModule) { super(sourceSection); this.currentModule = currentModule; - this.language = language; + readGlobElementNode = + new SharedMemberNode( + sourceSection, + sourceSection, + "", + language, + new FrameDescriptor(), + new ReadGlobElementNode(sourceSection)); } + @Specialization @TruffleBoundary - private URI doResolveUri(String globExpression) { + public Object read(String globPattern) { + var context = VmContext.get(this); + var globUri = toUri(globPattern); + var resolvedElements = + context + .getResourceManager() + .resolveGlob(globUri, currentModule.getUri(), currentModule, this, globPattern); + var builder = new VmObjectBuilder(); + for (var entry : resolvedElements.entrySet()) { + builder.addEntry(entry.getKey(), readGlobElementNode); + } + return builder.toMapping(resolvedElements); + } + + private URI toUri(String globPattern) { try { - var globUri = IoUtils.toUri(globExpression); - var tripleDotImport = IoUtils.parseTripleDotPath(globUri); - if (tripleDotImport != null) { + var globUri = IoUtils.toUri(globPattern); + if (IoUtils.parseTripleDotPath(globUri) != null) { throw exceptionBuilder().evalError("cannotGlobTripleDots").build(); } return globUri; } catch (URISyntaxException e) { throw exceptionBuilder() - .evalError("invalidResourceUri", globExpression) + .evalError("invalidResourceUri", globPattern) .withHint(e.getReason()) .build(); } } - - @TruffleBoundary - private EconomicMap buildMembers( - FrameDescriptor frameDescriptor, List uris) { - var members = EconomicMaps.create(); - for (var entry : uris) { - var readNode = new StaticReadNode(entry.getUri()); - var member = - new ObjectMember( - VmUtils.unavailableSourceSection(), - VmUtils.unavailableSourceSection(), - VmModifier.ENTRY, - null, - ""); - var memberNode = new UntypedObjectMemberNode(language, frameDescriptor, member, readNode); - member.initMemberNode(memberNode); - EconomicMaps.put(members, entry.getPath(), member); - } - return members; - } - - @Specialization - public Object read(VirtualFrame frame, String globPattern) { - if (readResult == null) { - CompilerDirectives.transferToInterpreterAndInvalidate(); - var context = VmContext.get(this); - var resolvedUri = doResolveUri(globPattern); - var uris = - context - .getResourceManager() - .resolveGlob(resolvedUri, currentModule.getUri(), currentModule, this, globPattern); - var members = buildMembers(frame.getFrameDescriptor(), uris); - readResult = - new VmMapping(frame.materialize(), BaseModule.getMappingClass().getPrototype(), members); - } - return readResult; - } } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/StaticReadNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/StaticReadNode.java deleted file mode 100644 index eb98934d7..000000000 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/StaticReadNode.java +++ /dev/null @@ -1,50 +0,0 @@ -/** - * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. - * - * 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 org.pkl.core.ast.expression.unary; - -import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; -import com.oracle.truffle.api.frame.VirtualFrame; -import java.net.URI; -import org.pkl.core.runtime.VmContext; -import org.pkl.core.runtime.VmUtils; -import org.pkl.core.util.LateInit; - -/** Used by {@link ReadGlobNode}. */ -public class StaticReadNode extends UnaryExpressionNode { - private final URI resourceUri; - - @CompilationFinal @LateInit private Object readResult; - - public StaticReadNode(URI resourceUri) { - super(VmUtils.unavailableSourceSection()); - assert resourceUri.isAbsolute(); - this.resourceUri = resourceUri; - } - - @Override - public Object executeGeneric(VirtualFrame frame) { - if (readResult == null) { - CompilerDirectives.transferToInterpreterAndInvalidate(); - var context = VmContext.get(this); - readResult = context.getResourceManager().read(resourceUri, this).orElse(null); - if (readResult == null) { - throw exceptionBuilder().evalError("cannotFindResource", resourceUri).build(); - } - } - return readResult; - } -} diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/ModuleCache.java b/pkl-core/src/main/java/org/pkl/core/runtime/ModuleCache.java index f01f350ff..afb6964b9 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/ModuleCache.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/ModuleCache.java @@ -30,7 +30,6 @@ import org.pkl.core.Release; import org.pkl.core.SecurityManager; import org.pkl.core.SecurityManagerException; -import org.pkl.core.ast.expression.unary.ImportNode; import org.pkl.core.module.ModuleKey; import org.pkl.core.module.ModuleKeys; import org.pkl.core.module.ResolvedModuleKey; @@ -77,7 +76,7 @@ public synchronized VmTyped getOrLoad( ModuleResolver moduleResolver, Supplier moduleInstantiator, ModuleInitializer moduleInitializer, - @Nullable ImportNode importNode) { + @Nullable Node importNode) { if (ModuleKeys.isStdLibModule(moduleKey)) { var moduleName = moduleKey.getUri().getSchemeSpecificPart(); @@ -162,7 +161,7 @@ private VmTyped doLoad( ModuleResolver moduleResolver, Supplier moduleInstantiator, ModuleInitializer moduleInitializer, - @Nullable ImportNode importNode) { + @Nullable Node importNode) { VmTyped module = moduleInstantiator.get(); @@ -189,7 +188,7 @@ private VmTyped doLoad( } private ResolvedModuleKey resolve( - ModuleKey module, SecurityManager securityManager, @Nullable ImportNode importNode) { + ModuleKey module, SecurityManager securityManager, @Nullable Node importNode) { try { return module.resolve(securityManager); } catch (SecurityManagerException | PackageLoadError e) { diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java b/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java index 62b7c262b..4cdb73b2c 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java @@ -22,7 +22,6 @@ import java.net.URISyntaxException; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; import org.pkl.core.SecurityManager; @@ -45,7 +44,7 @@ public final class ResourceManager { // cache resources indefinitely to make resource reads deterministic private final Map> resources = new HashMap<>(); - private final Map> globExpressions = new HashMap<>(); + private final Map> resolvedGlobs = new HashMap<>(); public ResourceManager(SecurityManager securityManager, Collection readers) { this.securityManager = securityManager; @@ -67,13 +66,13 @@ public ResourceManager(SecurityManager securityManager, CollectionThe glob URI must be absolute. For example: {@code "file:///foo/bar/*.pkl"}. */ @TruffleBoundary - public List resolveGlob( + public Map resolveGlob( URI globUri, URI enclosingUri, ModuleKey enclosingModuleKey, Node readNode, - String globExpression) { - return globExpressions.computeIfAbsent( + String globPattern) { + return resolvedGlobs.computeIfAbsent( globUri.normalize(), uri -> { var scheme = uri.getScheme(); @@ -97,12 +96,11 @@ public List resolveGlob( .withLocation(readNode) .build(); } - var securityManager = VmContext.get(readNode).getSecurityManager(); return GlobResolver.resolveGlob( - securityManager, reader, enclosingModuleKey, enclosingUri, globExpression); + securityManager, reader, enclosingModuleKey, enclosingUri, globPattern); } catch (InvalidGlobPatternException e) { throw new VmExceptionBuilder() - .evalError("invalidGlobPattern", globExpression) + .evalError("invalidGlobPattern", globPattern) .withHint(e.getMessage()) .withLocation(readNode) .build(); @@ -110,7 +108,7 @@ public List resolveGlob( throw new VmExceptionBuilder().withCause(e).withLocation(readNode).build(); } catch (IOException e) { throw new VmExceptionBuilder() - .evalError("ioErrorResolvingGlob", globExpression) + .evalError("ioErrorResolvingGlob", globPattern) .withCause(e) .withLocation(readNode) .build(); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmLanguage.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmLanguage.java index 1a1ce9a25..bbf21b146 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmLanguage.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmLanguage.java @@ -22,7 +22,6 @@ import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.source.Source; import org.pkl.core.ast.builder.AstBuilder; -import org.pkl.core.ast.expression.unary.ImportNode; import org.pkl.core.module.ModuleKey; import org.pkl.core.module.ResolvedModuleKey; import org.pkl.core.parser.LexParseException; @@ -73,7 +72,7 @@ public VmTyped loadModule(ModuleKey moduleKey) { } @TruffleBoundary - public VmTyped loadModule(ModuleKey moduleKey, ImportNode importNode) { + public VmTyped loadModule(ModuleKey moduleKey, @Nullable Node importNode) { var context = VmContext.get(null); return context diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java index 5ef798bb4..5a833b1e1 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java @@ -16,12 +16,12 @@ package org.pkl.core.runtime; import org.graalvm.collections.EconomicMap; +import org.pkl.core.ast.VmModifier; import org.pkl.core.ast.member.ObjectMember; +import org.pkl.core.ast.member.SharedMemberNode; import org.pkl.core.util.EconomicMaps; -/** - * A builder for {@link VmObject}s whose {@link ObjectMember}s are determined at run time. - */ +/** A builder for {@link VmObject}s whose {@link ObjectMember}s are determined at run time. */ public final class VmObjectBuilder { private final EconomicMap members; private int elementCount = 0; @@ -29,7 +29,7 @@ public final class VmObjectBuilder { public VmObjectBuilder() { members = EconomicMaps.create(); } - + public VmObjectBuilder(int initialSize) { members = EconomicMaps.create(initialSize); } @@ -38,9 +38,10 @@ public VmObjectBuilder addProperty(Identifier name, Object value) { EconomicMaps.put(members, name, VmUtils.createSyntheticObjectProperty(name, "", value)); return this; } - + public VmObjectBuilder addElement(Object value) { - EconomicMaps.put(members, (long) elementCount++, VmUtils.createSyntheticObjectElement("", value)); + EconomicMaps.put( + members, (long) elementCount++, VmUtils.createSyntheticObjectElement("", value)); return this; } @@ -49,13 +50,21 @@ public VmObjectBuilder addEntry(Object key, Object value) { return this; } + public VmObjectBuilder addEntry(Object key, SharedMemberNode valueNode) { + var entry = + new ObjectMember( + valueNode.getSourceSection(), valueNode.getHeaderSection(), VmModifier.ENTRY, null, ""); + entry.initMemberNode(valueNode); + EconomicMaps.put(members, key, entry); + return this; + } + public VmListing toListing() { return new VmListing( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getListingClass().getPrototype(), - members, - elementCount - ); + VmUtils.createEmptyMaterializedFrame(), + BaseModule.getListingClass().getPrototype(), + members, + elementCount); } public VmMapping toMapping() { @@ -65,11 +74,17 @@ public VmMapping toMapping() { members); } + public VmMapping toMapping(Object extraStorage) { + var result = toMapping(); + result.setExtraStorage(extraStorage); + return result; + } + public VmDynamic toDynamic() { return new VmDynamic( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getDynamicClass().getPrototype(), - members, - elementCount); + VmUtils.createEmptyMaterializedFrame(), + BaseModule.getDynamicClass().getPrototype(), + members, + elementCount); } } diff --git a/pkl-core/src/main/java/org/pkl/core/util/GlobResolver.java b/pkl-core/src/main/java/org/pkl/core/util/GlobResolver.java index 52b9b4b02..d64712b9a 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/GlobResolver.java +++ b/pkl-core/src/main/java/org/pkl/core/util/GlobResolver.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -250,7 +251,7 @@ private static void resolveOpaqueGlob( ReaderBase reader, URI globUri, Pattern pattern, - ArrayList result) + Map result) throws IOException, SecurityManagerException { var elements = reader.listElements(securityManager, globUri); for (var elem : sorted(elements)) { @@ -261,7 +262,8 @@ private static void resolveOpaqueGlob( throw new IllegalArgumentException(e.getMessage(), e); } if (pattern.matcher(resolvedUri.toString()).matches()) { - result.add(new ResolvedGlobElement(resolvedUri.toString(), resolvedUri, false)); + var path = resolvedUri.toString(); + result.put(path, new ResolvedGlobElement(path, resolvedUri, false)); } } } @@ -372,7 +374,7 @@ private static void resolveHierarchicalGlob( URI baseUri, String expandedGlobPatternSoFar, boolean hasAbsoluteGlob, - List result, + Map result, MutableLong listElementCallCount) throws IOException, SecurityManagerException, InvalidGlobPatternException { var isLeaf = idx == globPatternParts.length - 1; @@ -384,7 +386,7 @@ private static void resolveHierarchicalGlob( if (reader.hasElement(securityManager, newBaseUri)) { // Note: isDirectory is not meaningful here. Setting it to false is a way to skip setting // it. - result.add(new ResolvedGlobElement(newPath, newBaseUri, false)); + result.put(newPath, new ResolvedGlobElement(newPath, newBaseUri, false)); } } else { var newBaseUri = IoUtils.resolve(reader, baseUri, patternPart + "/"); @@ -414,7 +416,7 @@ private static void resolveHierarchicalGlob( listElementCallCount); for (var element : matchedElements) { if (isLeaf) { - result.add(element); + result.put(element.getPath(), element); } else if (element.isDirectory()) { resolveHierarchicalGlob( securityManager, @@ -465,7 +467,7 @@ private static Pair splitGlobPatternIntoBaseAndWildcards( *

Each pair is the expanded form of the glob pattern, paired with its resolved absolute URI. */ @TruffleBoundary - public static List resolveGlob( + public static Map resolveGlob( SecurityManager securityManager, ReaderBase reader, ModuleKey enclosingModuleKey, @@ -473,7 +475,7 @@ public static List resolveGlob( String globPattern) throws IOException, SecurityManagerException, InvalidGlobPatternException { - var result = new ArrayList(); + var result = new LinkedHashMap(); var hasAbsoluteGlob = globPattern.matches("\\w+:.*"); if (reader.hasHierarchicalUris()) { @@ -484,7 +486,7 @@ public static List resolveGlob( if (globParts.length == 0) { var resolvedUri = IoUtils.resolve(reader, enclosingUri, globPattern); if (reader.hasElement(securityManager, resolvedUri)) { - result.add(new ResolvedGlobElement(globPattern, resolvedUri, true)); + result.put(globPattern, new ResolvedGlobElement(globPattern, resolvedUri, true)); } return result; } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl index a7b2b18ed..238abc2f9 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl @@ -30,4 +30,11 @@ examples { ["package:"] { read*("package://localhost:0/birds@0.5.0#/**.pkl") } + + ["non-constant glob pattern"] { + local function readGlob(pattern) = read*(pattern) + readGlob("globtest/file*.txt") + readGlob("globtest/file1.txt") + readGlob("globtest/file2.txt") + } } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf index 3df2be340..c29ffd1eb 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf @@ -176,4 +176,44 @@ examples { } } } + ["non-constant glob pattern"] { + new { + ["globtest/file1.txt"] { + uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" + text = """ + file1 + + """ + base64 = "ZmlsZTEK" + } + ["globtest/file2.txt"] { + uri = "file:///$snippetsDir/input/basic/globtest/file2.txt" + text = """ + file2 + + """ + base64 = "ZmlsZTIK" + } + } + new { + ["globtest/file1.txt"] { + uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" + text = """ + file1 + + """ + base64 = "ZmlsZTEK" + } + } + new { + ["globtest/file2.txt"] { + uri = "file:///$snippetsDir/input/basic/globtest/file2.txt" + text = """ + file2 + + """ + base64 = "ZmlsZTIK" + } + } + } } From 5b9e27ded08f0ad951801cfb8af96cc8c337ffb1 Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:11:50 -0700 Subject: [PATCH 5/8] Simplify code --- .../src/main/java/org/pkl/core/runtime/ModuleInfo.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/ModuleInfo.java b/pkl-core/src/main/java/org/pkl/core/runtime/ModuleInfo.java index f452e03e5..ce1c4be9d 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/ModuleInfo.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/ModuleInfo.java @@ -22,8 +22,7 @@ import org.pkl.core.PClass; import org.pkl.core.TypeAlias; import org.pkl.core.ast.MemberNode; -import org.pkl.core.ast.expression.unary.ImportGlobNode; -import org.pkl.core.ast.expression.unary.ImportNode; +import org.pkl.core.ast.expression.unary.AbstractImportNode; import org.pkl.core.module.ModuleKey; import org.pkl.core.module.ResolvedModuleKey; import org.pkl.core.util.EconomicMaps; @@ -132,11 +131,8 @@ public ModuleSchema getModuleSchema(VmTyped module) { if (propertyDef.isImport()) { MemberNode memberNode = propertyDef.getMemberNode(); assert memberNode != null; // import is never a constant - var importNode = memberNode.getBodyNode(); - var importUri = - importNode instanceof ImportNode - ? ((ImportNode) importNode).getImportUri() - : ((ImportGlobNode) importNode).getImportUri(); + var importNode = (AbstractImportNode) memberNode.getBodyNode(); + var importUri = importNode.getImportUri(); imports.put(propertyDef.getName().toString(), importUri); continue; } From f307a45859a46de6ca376f0a00b408f34b044c2a Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:39:06 -0700 Subject: [PATCH 6/8] Fix ClassCastException when reflecting on globbed import --- .../src/main/java/org/pkl/core/runtime/VmTyped.java | 4 ++-- .../LanguageSnippetTests/input/api/reflect4.pkl | 12 ++++++++++++ .../LanguageSnippetTests/output/api/reflect4.pcf | 5 +++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java index 3f7d046ff..7ccae1c9d 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java @@ -23,7 +23,7 @@ import org.pkl.core.Composite; import org.pkl.core.PModule; import org.pkl.core.PObject; -import org.pkl.core.ast.expression.unary.ImportNode; +import org.pkl.core.ast.expression.unary.AbstractImportNode; import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.LateInit; @@ -105,7 +105,7 @@ public VmMap getImports() { assert memberNode != null; // import is never a constant builder.add( member.getName().toString(), - ((ImportNode) memberNode.getBodyNode()).getImportUri().toString()); + ((AbstractImportNode) memberNode.getBodyNode()).getImportUri().toString()); } } return builder.build(); diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl new file mode 100644 index 000000000..4ac3867b7 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl @@ -0,0 +1,12 @@ +amends ".../snippetTest.pkl" + +import "pkl:reflect" +import* "reflect*.pkl" + +local mod = reflect.Module(this) + +examples { + ["Reflecting globbed import"] { + mod.imports.keys == Set("reflect", "reflect*") + } +} diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf new file mode 100644 index 000000000..68d45e08d --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf @@ -0,0 +1,5 @@ +examples { + ["Reflecting globbed import"] { + true + } +} From 5fba86890096ec1207ec75cf996dede0a51f01f7 Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:14:56 -0700 Subject: [PATCH 7/8] Fix caching of globbed reads Problem: The result of a globbed read depends not only on the glob pattern but also on the current module URI. However, ResourceManager does not take this into account when caching globbed reads, causing wrong results. Changes: - Cache globbed reads per read expression. This is also how globbed imports are cached, except that import URIs are constant. - Make ReadGlobNode and ImportGlobNode code as similar as possible. - Reduce code duplication by inheriting from AbstractReadNode. - Add some tests. --- .../expression/unary/AbstractImportNode.java | 7 +- .../expression/unary/AbstractReadNode.java | 25 ++-- .../unary/ImportGlobElementNode.java | 5 +- .../ast/expression/unary/ImportGlobNode.java | 71 +++++----- .../core/ast/expression/unary/ImportNode.java | 4 +- .../expression/unary/ReadGlobElementNode.java | 2 +- .../ast/expression/unary/ReadGlobNode.java | 70 ++++++---- .../org/pkl/core/runtime/ResourceManager.java | 76 ++-------- .../input-helper/basic/read/child/module2.pkl | 2 + .../basic/read/child/resource.txt | 1 + .../input-helper/basic/read/module1.pkl | 2 + .../input-helper/basic/read/resource.txt | 1 + .../LanguageSnippetTests/input/basic/read.pkl | 22 ++- .../input/basic/readGlob.pkl | 17 ++- .../output/basic/read.pcf | 132 ++++++++---------- .../output/basic/readGlob.pcf | 24 +++- 16 files changed, 231 insertions(+), 230 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/module2.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/resource.txt create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/module1.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/resource.txt diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractImportNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractImportNode.java index 054382b57..ae3fc06f2 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractImportNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractImportNode.java @@ -18,16 +18,19 @@ import com.oracle.truffle.api.source.SourceSection; import java.net.URI; import org.pkl.core.ast.ExpressionNode; +import org.pkl.core.module.ResolvedModuleKey; public abstract class AbstractImportNode extends ExpressionNode { + protected final ResolvedModuleKey currentModule; protected final URI importUri; - AbstractImportNode(SourceSection sourceSection, URI importUri) { + AbstractImportNode(SourceSection sourceSection, ResolvedModuleKey currentModule, URI importUri) { super(sourceSection); + this.currentModule = currentModule; this.importUri = importUri; } - public URI getImportUri() { + public final URI getImportUri() { return importUri; } } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractReadNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractReadNode.java index 0e5c891d2..9d6a1ce49 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractReadNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/AbstractReadNode.java @@ -30,30 +30,33 @@ import org.pkl.core.util.Nullable; public abstract class AbstractReadNode extends UnaryExpressionNode { - private final ModuleKey moduleKey; + protected final ModuleKey currentModule; - protected AbstractReadNode(SourceSection sourceSection, ModuleKey moduleKey) { + protected AbstractReadNode(SourceSection sourceSection, ModuleKey currentModule) { super(sourceSection); - this.moduleKey = moduleKey; + this.currentModule = currentModule; } @TruffleBoundary - protected @Nullable Object doRead(String resourceUri, VmContext context, Node readNode) { - var resolvedUri = resolveResource(moduleKey, resourceUri); - return context.getResourceManager().read(resolvedUri, readNode).orElse(null); - } - - private URI resolveResource(ModuleKey moduleKey, String resourceUri) { - URI parsedUri; + protected final URI parseUri(String resourceUri) { try { - parsedUri = IoUtils.toUri(resourceUri); + return IoUtils.toUri(resourceUri); } catch (URISyntaxException e) { throw exceptionBuilder() .evalError("invalidResourceUri", resourceUri) .withHint(e.getReason()) .build(); } + } + @TruffleBoundary + protected final @Nullable Object doRead(String resourceUri, VmContext context, Node readNode) { + var resolvedUri = resolveResource(currentModule, resourceUri); + return context.getResourceManager().read(resolvedUri, readNode).orElse(null); + } + + private URI resolveResource(ModuleKey moduleKey, String resourceUri) { + var parsedUri = parseUri(resourceUri); var context = VmContext.get(this); URI resolvedUri; try { diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java index 8873ac775..93cc3de15 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java @@ -36,7 +36,8 @@ public final class ImportGlobElementNode extends ExpressionNode { private final VmLanguage language; private final ResolvedModuleKey currentModule; - public ImportGlobElementNode(SourceSection sourceSection, VmLanguage language, ResolvedModuleKey currentModule) { + public ImportGlobElementNode( + SourceSection sourceSection, VmLanguage language, ResolvedModuleKey currentModule) { super(sourceSection); this.language = language; this.currentModule = currentModule; @@ -48,7 +49,7 @@ public Object executeGeneric(VirtualFrame frame) { var path = (String) VmUtils.getMemberKey(frame); return importModule(mapping, path); } - + @TruffleBoundary private VmTyped importModule(VmObjectLike mapping, String path) { @SuppressWarnings("unchecked") diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java index 2893e14b3..f9fba61d9 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java @@ -38,11 +38,10 @@ @NodeInfo(shortName = "import*") public class ImportGlobNode extends AbstractImportNode { - private final ResolvedModuleKey currentModule; private final String globPattern; private final SharedMemberNode importGlobElementNode; - @CompilationFinal @LateInit private VmMapping importsMapping; + @CompilationFinal @LateInit private VmMapping cachedResult; public ImportGlobNode( VmLanguage language, @@ -50,8 +49,7 @@ public ImportGlobNode( ResolvedModuleKey currentModule, URI importUri, String globPattern) { - super(sourceSection, importUri); - this.currentModule = currentModule; + super(sourceSection, currentModule, importUri); this.globPattern = globPattern; importGlobElementNode = new SharedMemberNode( @@ -65,42 +63,41 @@ public ImportGlobNode( @Override public Object executeGeneric(VirtualFrame frame) { - if (importsMapping == null) { - CompilerDirectives.transferToInterpreterAndInvalidate(); - var context = VmContext.get(this); - try { - var moduleKey = context.getModuleResolver().resolve(importUri); - var securityManager = VmContext.get(this).getSecurityManager(); - if (!moduleKey.isGlobbable()) { - throw exceptionBuilder() - .evalError("cannotGlobUri", importUri, importUri.getScheme()) - .build(); - } - var resolvedElements = - GlobResolver.resolveGlob( - securityManager, - moduleKey, - currentModule.getOriginal(), - currentModule.getUri(), - globPattern); - var builder = new VmObjectBuilder(); - for (var entry : resolvedElements.entrySet()) { - builder.addEntry(entry.getKey(), importGlobElementNode); - } - importsMapping = builder.toMapping(resolvedElements); - } catch (IOException e) { - throw exceptionBuilder().evalError("ioErrorResolvingGlob", importUri).withCause(e).build(); - } catch (SecurityManagerException | HttpClientInitException e) { - throw exceptionBuilder().withCause(e).build(); - } catch (PackageLoadError e) { - throw exceptionBuilder().adhocEvalError(e.getMessage()).build(); - } catch (InvalidGlobPatternException e) { + if (cachedResult != null) return cachedResult; + + CompilerDirectives.transferToInterpreterAndInvalidate(); + var context = VmContext.get(this); + try { + var moduleKey = context.getModuleResolver().resolve(importUri); + if (!moduleKey.isGlobbable()) { throw exceptionBuilder() - .evalError("invalidGlobPattern", globPattern) - .withHint(e.getMessage()) + .evalError("cannotGlobUri", importUri, importUri.getScheme()) .build(); } + var resolvedElements = + GlobResolver.resolveGlob( + context.getSecurityManager(), + moduleKey, + currentModule.getOriginal(), + currentModule.getUri(), + globPattern); + var builder = new VmObjectBuilder(); + for (var entry : resolvedElements.entrySet()) { + builder.addEntry(entry.getKey(), importGlobElementNode); + } + cachedResult = builder.toMapping(resolvedElements); + return cachedResult; + } catch (IOException e) { + throw exceptionBuilder().evalError("ioErrorResolvingGlob", importUri).withCause(e).build(); + } catch (SecurityManagerException | HttpClientInitException e) { + throw exceptionBuilder().withCause(e).build(); + } catch (PackageLoadError e) { + throw exceptionBuilder().adhocEvalError(e.getMessage()).build(); + } catch (InvalidGlobPatternException e) { + throw exceptionBuilder() + .evalError("invalidGlobPattern", globPattern) + .withHint(e.getMessage()) + .build(); } - return importsMapping; } } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportNode.java index f8fdd4a8c..94ce45b0a 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportNode.java @@ -33,7 +33,6 @@ @NodeInfo(shortName = "import") public final class ImportNode extends AbstractImportNode { private final VmLanguage language; - private final ResolvedModuleKey currentModule; @CompilationFinal @LateInit private VmTyped importedModule; @@ -42,9 +41,8 @@ public ImportNode( SourceSection sourceSection, ResolvedModuleKey currentModule, URI importUri) { - super(sourceSection, importUri); + super(sourceSection, currentModule, importUri); this.language = language; - this.currentModule = currentModule; assert importUri.isAbsolute(); } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java index d4e394c1f..fb782b854 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java @@ -37,7 +37,7 @@ public Object executeGeneric(VirtualFrame frame) { var path = (String) VmUtils.getMemberKey(frame); return readResource(mapping, path); } - + @TruffleBoundary private Object readResource(VmObjectLike mapping, String path) { @SuppressWarnings("unchecked") diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java index eac2ed8e3..070c1a3ea 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java @@ -20,24 +20,27 @@ import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.source.SourceSection; -import java.net.URI; -import java.net.URISyntaxException; +import java.io.IOException; +import org.graalvm.collections.EconomicMap; +import org.pkl.core.SecurityManagerException; import org.pkl.core.ast.member.SharedMemberNode; +import org.pkl.core.http.HttpClientInitException; import org.pkl.core.module.ModuleKey; import org.pkl.core.runtime.VmContext; import org.pkl.core.runtime.VmLanguage; +import org.pkl.core.runtime.VmMapping; import org.pkl.core.runtime.VmObjectBuilder; -import org.pkl.core.util.IoUtils; +import org.pkl.core.util.GlobResolver; +import org.pkl.core.util.GlobResolver.InvalidGlobPatternException; @NodeInfo(shortName = "read*") -public abstract class ReadGlobNode extends UnaryExpressionNode { - private final ModuleKey currentModule; +public abstract class ReadGlobNode extends AbstractReadNode { private final SharedMemberNode readGlobElementNode; + private final EconomicMap cachedResults = EconomicMap.create(); protected ReadGlobNode( VmLanguage language, SourceSection sourceSection, ModuleKey currentModule) { - super(sourceSection); - this.currentModule = currentModule; + super(sourceSection, currentModule); readGlobElementNode = new SharedMemberNode( sourceSection, @@ -51,30 +54,43 @@ protected ReadGlobNode( @Specialization @TruffleBoundary public Object read(String globPattern) { - var context = VmContext.get(this); - var globUri = toUri(globPattern); - var resolvedElements = - context - .getResourceManager() - .resolveGlob(globUri, currentModule.getUri(), currentModule, this, globPattern); - var builder = new VmObjectBuilder(); - for (var entry : resolvedElements.entrySet()) { - builder.addEntry(entry.getKey(), readGlobElementNode); - } - return builder.toMapping(resolvedElements); - } + var cachedResult = cachedResults.get(globPattern); + if (cachedResult != null) return cachedResult; - private URI toUri(String globPattern) { + // use same check as for globbed imports (see AstBuilder) + if (globPattern.startsWith("...")) { + throw exceptionBuilder().evalError("cannotGlobTripleDots").build(); + } + var globUri = parseUri(globPattern); + var context = VmContext.get(this); try { - var globUri = IoUtils.toUri(globPattern); - if (IoUtils.parseTripleDotPath(globUri) != null) { - throw exceptionBuilder().evalError("cannotGlobTripleDots").build(); + var resolvedUri = currentModule.resolveUri(globUri); + var reader = context.getResourceManager().getReader(resolvedUri, this); + if (!reader.isGlobbable()) { + throw exceptionBuilder().evalError("cannotGlobUri", globUri, globUri.getScheme()).build(); + } + var resolvedElements = + GlobResolver.resolveGlob( + context.getSecurityManager(), + reader, + currentModule, + currentModule.getUri(), + globPattern); + var builder = new VmObjectBuilder(); + for (var entry : resolvedElements.entrySet()) { + builder.addEntry(entry.getKey(), readGlobElementNode); } - return globUri; - } catch (URISyntaxException e) { + cachedResult = builder.toMapping(resolvedElements); + cachedResults.put(globPattern, cachedResult); + return cachedResult; + } catch (IOException e) { + throw exceptionBuilder().evalError("ioErrorResolvingGlob", globPattern).withCause(e).build(); + } catch (SecurityManagerException | HttpClientInitException e) { + throw exceptionBuilder().withCause(e).build(); + } catch (InvalidGlobPatternException e) { throw exceptionBuilder() - .evalError("invalidResourceUri", globPattern) - .withHint(e.getReason()) + .evalError("invalidGlobPattern", globPattern) + .withHint(e.getMessage()) .build(); } } diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java b/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java index 4cdb73b2c..7c5e64b84 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java @@ -27,14 +27,10 @@ import org.pkl.core.SecurityManager; import org.pkl.core.SecurityManagerException; import org.pkl.core.http.HttpClientInitException; -import org.pkl.core.module.ModuleKey; import org.pkl.core.packages.PackageLoadError; import org.pkl.core.resource.Resource; import org.pkl.core.resource.ResourceReader; import org.pkl.core.stdlib.VmObjectFactory; -import org.pkl.core.util.GlobResolver; -import org.pkl.core.util.GlobResolver.InvalidGlobPatternException; -import org.pkl.core.util.GlobResolver.ResolvedGlobElement; public final class ResourceManager { private final Map resourceReaders = new HashMap<>(); @@ -44,8 +40,6 @@ public final class ResourceManager { // cache resources indefinitely to make resource reads deterministic private final Map> resources = new HashMap<>(); - private final Map> resolvedGlobs = new HashMap<>(); - public ResourceManager(SecurityManager securityManager, Collection readers) { this.securityManager = securityManager; @@ -60,60 +54,16 @@ public ResourceManager(SecurityManager securityManager, CollectionThe glob URI must be absolute. For example: {@code "file:///foo/bar/*.pkl"}. - */ @TruffleBoundary - public Map resolveGlob( - URI globUri, - URI enclosingUri, - ModuleKey enclosingModuleKey, - Node readNode, - String globPattern) { - return resolvedGlobs.computeIfAbsent( - globUri.normalize(), - uri -> { - var scheme = uri.getScheme(); - URI resolvedUri; - try { - resolvedUri = enclosingModuleKey.resolveUri(globUri); - } catch (SecurityManagerException | IOException e) { - throw new VmExceptionBuilder().withLocation(readNode).withCause(e).build(); - } - try { - var reader = resourceReaders.get(resolvedUri.getScheme()); - if (reader == null) { - throw new VmExceptionBuilder() - .withLocation(readNode) - .evalError("noResourceReaderRegistered", scheme) - .build(); - } - if (!reader.isGlobbable()) { - throw new VmExceptionBuilder() - .evalError("cannotGlobUri", uri, scheme) - .withLocation(readNode) - .build(); - } - return GlobResolver.resolveGlob( - securityManager, reader, enclosingModuleKey, enclosingUri, globPattern); - } catch (InvalidGlobPatternException e) { - throw new VmExceptionBuilder() - .evalError("invalidGlobPattern", globPattern) - .withHint(e.getMessage()) - .withLocation(readNode) - .build(); - } catch (SecurityManagerException | HttpClientInitException e) { - throw new VmExceptionBuilder().withCause(e).withLocation(readNode).build(); - } catch (IOException e) { - throw new VmExceptionBuilder() - .evalError("ioErrorResolvingGlob", globPattern) - .withCause(e) - .withLocation(readNode) - .build(); - } - }); + public ResourceReader getReader(URI resourceUri, Node readNode) { + var reader = resourceReaders.get(resourceUri.getScheme()); + if (reader == null) { + throw new VmExceptionBuilder() + .withLocation(readNode) + .evalError("noResourceReaderRegistered", resourceUri.getScheme()) + .build(); + } + return reader; } @TruffleBoundary @@ -127,13 +77,7 @@ public Optional read(URI resourceUri, Node readNode) { throw new VmExceptionBuilder().withCause(e).withLocation(readNode).build(); } - var reader = resourceReaders.get(uri.getScheme()); - if (reader == null) { - throw new VmExceptionBuilder() - .withLocation(readNode) - .evalError("noResourceReaderRegistered", resourceUri.getScheme()) - .build(); - } + var reader = getReader(resourceUri, readNode); Optional resource; try { diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/module2.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/module2.pkl new file mode 100644 index 000000000..141cb9532 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/module2.pkl @@ -0,0 +1,2 @@ +normalRead = read("resource.txt") +globbedRead = read*("*.txt") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/resource.txt b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/resource.txt new file mode 100644 index 000000000..cdf568206 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/child/resource.txt @@ -0,0 +1 @@ +child resource diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/module1.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/module1.pkl new file mode 100644 index 000000000..141cb9532 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/module1.pkl @@ -0,0 +1,2 @@ +normalRead = read("resource.txt") +globbedRead = read*("*.txt") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/resource.txt b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/resource.txt new file mode 100644 index 000000000..91e75c679 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/basic/read/resource.txt @@ -0,0 +1 @@ +resource diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/read.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/read.pkl index dd0da33fc..8a6918138 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/read.pkl +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/read.pkl @@ -22,14 +22,32 @@ examples { } ["read file"] { - read("read.pkl") + read("globtest/file1.txt") module.catch(() -> read("other.txt")) - read?("read.pkl") + read?("globtest/file1.txt") read?("other.txt") } + + ["read triple-dot file"] { + read(".../input-helper/basic/read/resource.txt") + read?(".../input-helper/basic/read/resource.txt") + } ["read non-allowed resource"] { module.catch(() -> read("forbidden:resource")) module.catch(() -> read?("forbidden:resource")) } + + ["use read expression with non-constant resource URI"] { + local function doRead(uri) = read(uri) + doRead("globtest/file1.txt") + doRead("globtest/file2.txt") + } + + ["read different resources with same relative resource URI"] { + local module1 = import(".../input-helper/basic/read/module1.pkl") + local module2 = import(".../input-helper/basic/read/child/module2.pkl") + module1.normalRead + module2.normalRead + } } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl index 238abc2f9..8f7b8adb4 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/readGlob.pkl @@ -31,10 +31,17 @@ examples { read*("package://localhost:0/birds@0.5.0#/**.pkl") } - ["non-constant glob pattern"] { - local function readGlob(pattern) = read*(pattern) - readGlob("globtest/file*.txt") - readGlob("globtest/file1.txt") - readGlob("globtest/file2.txt") + ["use read expression with non-constant glob pattern"] { + local function doRead(pattern) = read*(pattern) + doRead("globtest/file*.txt") + doRead("globtest/file1.txt") + doRead("globtest/file2.txt") + } + + ["read different resources with same glob pattern"] { + local module1 = import(".../input-helper/basic/read/module1.pkl") + local module2 = import(".../input-helper/basic/read/child/module2.pkl") + module1.globbedRead + module2.globbedRead } } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/read.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/read.pcf index 590d34bf9..733f2e77e 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/read.pcf +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/read.pcf @@ -17,94 +17,80 @@ examples { } ["read file"] { new { - uri = "file:///$snippetsDir/input/basic/read.pkl" + uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" text = """ - amends "../snippetTest.pkl" - - examples { - ["read env variable"] { - read("env:NAME1") - read("env:NAME2") - module.catch(() -> read("env:OTHER")) - - read?("env:NAME1") - read?("env:NAME2") - read?("env:OTHER") - } - - ["read external property"] { - read("prop:name1") - read("prop:name2") - module.catch(() -> read("prop:other")) - - read?("prop:name1") - read?("prop:name2") - read?("prop:other") - } - - ["read file"] { - read("read.pkl") - module.catch(() -> read("other.txt")) - read?("read.pkl") - read?("other.txt") - } - - ["read non-allowed resource"] { - module.catch(() -> read("forbidden:resource")) - module.catch(() -> read?("forbidden:resource")) - } - } + file1 """ - base64 = "YW1lbmRzICIuLi9zbmlwcGV0VGVzdC5wa2wiCgpleGFtcGxlcyB7CiAgWyJyZWFkIGVudiB2YXJpYWJsZSJdIHsKICAgIHJlYWQoImVudjpOQU1FMSIpCiAgICByZWFkKCJlbnY6TkFNRTIiKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoImVudjpPVEhFUiIpKQoKICAgIHJlYWQ/KCJlbnY6TkFNRTEiKQogICAgcmVhZD8oImVudjpOQU1FMiIpCiAgICByZWFkPygiZW52Ok9USEVSIikKICB9CgogIFsicmVhZCBleHRlcm5hbCBwcm9wZXJ0eSJdIHsKICAgIHJlYWQoInByb3A6bmFtZTEiKQogICAgcmVhZCgicHJvcDpuYW1lMiIpCiAgICBtb2R1bGUuY2F0Y2goKCkgLT4gcmVhZCgicHJvcDpvdGhlciIpKQoKICAgIHJlYWQ/KCJwcm9wOm5hbWUxIikKICAgIHJlYWQ/KCJwcm9wOm5hbWUyIikKICAgIHJlYWQ/KCJwcm9wOm90aGVyIikKICB9CgogIFsicmVhZCBmaWxlIl0gewogICAgcmVhZCgicmVhZC5wa2wiKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoIm90aGVyLnR4dCIpKQogICAgcmVhZD8oInJlYWQucGtsIikKICAgIHJlYWQ/KCJvdGhlci50eHQiKQogIH0KCiAgWyJyZWFkIG5vbi1hbGxvd2VkIHJlc291cmNlIl0gewogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoImZvcmJpZGRlbjpyZXNvdXJjZSIpKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQ/KCJmb3JiaWRkZW46cmVzb3VyY2UiKSkKICB9Cn0K" + base64 = "ZmlsZTEK" } "Cannot find resource `other.txt`." new { - uri = "file:///$snippetsDir/input/basic/read.pkl" + uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" text = """ - amends "../snippetTest.pkl" + file1 - examples { - ["read env variable"] { - read("env:NAME1") - read("env:NAME2") - module.catch(() -> read("env:OTHER")) - - read?("env:NAME1") - read?("env:NAME2") - read?("env:OTHER") - } - - ["read external property"] { - read("prop:name1") - read("prop:name2") - module.catch(() -> read("prop:other")) - - read?("prop:name1") - read?("prop:name2") - read?("prop:other") - } - - ["read file"] { - read("read.pkl") - module.catch(() -> read("other.txt")) - read?("read.pkl") - read?("other.txt") - } + """ + base64 = "ZmlsZTEK" + } + null + } + ["read triple-dot file"] { + new { + uri = "file:///$snippetsDir/input-helper/basic/read/resource.txt" + text = """ + resource - ["read non-allowed resource"] { - module.catch(() -> read("forbidden:resource")) - module.catch(() -> read?("forbidden:resource")) - } - } + """ + base64 = "cmVzb3VyY2UK" + } + new { + uri = "file:///$snippetsDir/input-helper/basic/read/resource.txt" + text = """ + resource """ - base64 = "YW1lbmRzICIuLi9zbmlwcGV0VGVzdC5wa2wiCgpleGFtcGxlcyB7CiAgWyJyZWFkIGVudiB2YXJpYWJsZSJdIHsKICAgIHJlYWQoImVudjpOQU1FMSIpCiAgICByZWFkKCJlbnY6TkFNRTIiKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoImVudjpPVEhFUiIpKQoKICAgIHJlYWQ/KCJlbnY6TkFNRTEiKQogICAgcmVhZD8oImVudjpOQU1FMiIpCiAgICByZWFkPygiZW52Ok9USEVSIikKICB9CgogIFsicmVhZCBleHRlcm5hbCBwcm9wZXJ0eSJdIHsKICAgIHJlYWQoInByb3A6bmFtZTEiKQogICAgcmVhZCgicHJvcDpuYW1lMiIpCiAgICBtb2R1bGUuY2F0Y2goKCkgLT4gcmVhZCgicHJvcDpvdGhlciIpKQoKICAgIHJlYWQ/KCJwcm9wOm5hbWUxIikKICAgIHJlYWQ/KCJwcm9wOm5hbWUyIikKICAgIHJlYWQ/KCJwcm9wOm90aGVyIikKICB9CgogIFsicmVhZCBmaWxlIl0gewogICAgcmVhZCgicmVhZC5wa2wiKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoIm90aGVyLnR4dCIpKQogICAgcmVhZD8oInJlYWQucGtsIikKICAgIHJlYWQ/KCJvdGhlci50eHQiKQogIH0KCiAgWyJyZWFkIG5vbi1hbGxvd2VkIHJlc291cmNlIl0gewogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQoImZvcmJpZGRlbjpyZXNvdXJjZSIpKQogICAgbW9kdWxlLmNhdGNoKCgpIC0+IHJlYWQ/KCJmb3JiaWRkZW46cmVzb3VyY2UiKSkKICB9Cn0K" + base64 = "cmVzb3VyY2UK" } - null } ["read non-allowed resource"] { "Refusing to read resource `forbidden:resource` because it does not match any entry in the resource allowlist (`--allowed-resources`)." "Refusing to read resource `forbidden:resource` because it does not match any entry in the resource allowlist (`--allowed-resources`)." } + ["use read expression with non-constant resource URI"] { + new { + uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" + text = """ + file1 + + """ + base64 = "ZmlsZTEK" + } + new { + uri = "file:///$snippetsDir/input/basic/globtest/file2.txt" + text = """ + file2 + + """ + base64 = "ZmlsZTIK" + } + } + ["read different resources with same relative resource URI"] { + new { + uri = "file:///$snippetsDir/input-helper/basic/read/resource.txt" + text = """ + resource + + """ + base64 = "cmVzb3VyY2UK" + } + new { + uri = "file:///$snippetsDir/input-helper/basic/read/child/resource.txt" + text = """ + child resource + + """ + base64 = "Y2hpbGQgcmVzb3VyY2UK" + } + } } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf index c29ffd1eb..6fedc7045 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/basic/readGlob.pcf @@ -176,7 +176,7 @@ examples { } } } - ["non-constant glob pattern"] { + ["use read expression with non-constant glob pattern"] { new { ["globtest/file1.txt"] { uri = "file:///$snippetsDir/input/basic/globtest/file1.txt" @@ -216,4 +216,26 @@ examples { } } } + ["read different resources with same glob pattern"] { + new { + ["resource.txt"] { + uri = "file:///$snippetsDir/input-helper/basic/read/resource.txt" + text = """ + resource + + """ + base64 = "cmVzb3VyY2UK" + } + } + new { + ["resource.txt"] { + uri = "file:///$snippetsDir/input-helper/basic/read/child/resource.txt" + text = """ + child resource + + """ + base64 = "Y2hpbGQgcmVzb3VyY2UK" + } + } + } } From e2a46043205597b355f8164c7a657ef77385c86b Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Tue, 28 May 2024 13:16:39 -0700 Subject: [PATCH 8/8] Address comments per review * Defer initialization of shared member node, add @Child annotations * Reduce footprint of truffle boundaries * Revert bugfix when reflecting upon modules with globbed imports --- .../org/pkl/core/ast/builder/AstBuilder.java | 6 ++-- ...ode.java => ImportGlobMemberBodyNode.java} | 4 +-- .../ast/expression/unary/ImportGlobNode.java | 32 ++++++++++------- ...tNode.java => ReadGlobMemberBodyNode.java} | 10 +++--- .../ast/expression/unary/ReadGlobNode.java | 35 ++++++++++++------- .../main/java/org/pkl/core/runtime/VmMap.java | 18 ++++------ .../org/pkl/core/runtime/VmObjectBuilder.java | 4 --- .../java/org/pkl/core/runtime/VmTyped.java | 4 +-- .../java/org/pkl/core/runtime/VmUtils.java | 5 +++ .../org/pkl/core/stdlib/base/MapNodes.java | 25 +------------ .../input/api/reflect4.pkl | 12 ------- .../output/api/reflect4.pcf | 5 --- 12 files changed, 65 insertions(+), 95 deletions(-) rename pkl-core/src/main/java/org/pkl/core/ast/expression/unary/{ImportGlobElementNode.java => ImportGlobMemberBodyNode.java} (96%) rename pkl-core/src/main/java/org/pkl/core/ast/expression/unary/{ReadGlobElementNode.java => ReadGlobMemberBodyNode.java} (85%) delete mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl delete mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf diff --git a/pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java b/pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java index c4d15a7e4..4e4124434 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java @@ -2211,8 +2211,7 @@ public Object visitReadExpr(ReadExprContext ctx) { return ReadOrNullNodeGen.create(createSourceSection(ctx), moduleKey, visitExpr(exprCtx)); } assert tokenType == PklLexer.READ_GLOB; - return ReadGlobNodeGen.create( - language, createSourceSection(ctx), moduleKey, visitExpr(exprCtx)); + return ReadGlobNodeGen.create(createSourceSection(ctx), moduleKey, visitExpr(exprCtx)); } @Override @@ -2653,8 +2652,7 @@ private AbstractImportNode doVisitImport( } var resolvedUri = resolveImport(importUri, importUriCtx); if (isGlobImport) { - return new ImportGlobNode( - language, section, moduleInfo.getResolvedModuleKey(), resolvedUri, importUri); + return new ImportGlobNode(section, moduleInfo.getResolvedModuleKey(), resolvedUri, importUri); } return new ImportNode(language, section, moduleInfo.getResolvedModuleKey(), resolvedUri); } diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobMemberBodyNode.java similarity index 96% rename from pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java rename to pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobMemberBodyNode.java index 93cc3de15..fde8c3297 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobMemberBodyNode.java @@ -32,11 +32,11 @@ import org.pkl.core.util.GlobResolver.ResolvedGlobElement; /** Used by {@link ReadGlobNode}. */ -public final class ImportGlobElementNode extends ExpressionNode { +public final class ImportGlobMemberBodyNode extends ExpressionNode { private final VmLanguage language; private final ResolvedModuleKey currentModule; - public ImportGlobElementNode( + public ImportGlobMemberBodyNode( SourceSection sourceSection, VmLanguage language, ResolvedModuleKey currentModule) { super(sourceSection); this.language = language; diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java index f9fba61d9..b22b32aa9 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java @@ -39,26 +39,32 @@ @NodeInfo(shortName = "import*") public class ImportGlobNode extends AbstractImportNode { private final String globPattern; - private final SharedMemberNode importGlobElementNode; - + @Child @LateInit private SharedMemberNode memberNode; @CompilationFinal @LateInit private VmMapping cachedResult; public ImportGlobNode( - VmLanguage language, SourceSection sourceSection, ResolvedModuleKey currentModule, URI importUri, String globPattern) { super(sourceSection, currentModule, importUri); this.globPattern = globPattern; - importGlobElementNode = - new SharedMemberNode( - sourceSection, - sourceSection, - "", - language, - new FrameDescriptor(), - new ImportGlobElementNode(sourceSection, language, currentModule)); + } + + private SharedMemberNode getMemberNode() { + if (memberNode == null) { + CompilerDirectives.transferToInterpreterAndInvalidate(); + var language = VmLanguage.get(this); + memberNode = + new SharedMemberNode( + sourceSection, + sourceSection, + "", + language, + new FrameDescriptor(), + new ImportGlobMemberBodyNode(sourceSection, language, currentModule)); + } + return memberNode; } @Override @@ -81,9 +87,9 @@ public Object executeGeneric(VirtualFrame frame) { currentModule.getOriginal(), currentModule.getUri(), globPattern); - var builder = new VmObjectBuilder(); + var builder = new VmObjectBuilder(resolvedElements.size()); for (var entry : resolvedElements.entrySet()) { - builder.addEntry(entry.getKey(), importGlobElementNode); + builder.addEntry(entry.getKey(), getMemberNode()); } cachedResult = builder.toMapping(resolvedElements); return cachedResult; diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobMemberBodyNode.java similarity index 85% rename from pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java rename to pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobMemberBodyNode.java index fb782b854..838122ca4 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobMemberBodyNode.java @@ -15,7 +15,7 @@ */ package org.pkl.core.ast.expression.unary; -import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.source.SourceSection; import java.util.Map; @@ -26,8 +26,8 @@ import org.pkl.core.util.GlobResolver.ResolvedGlobElement; /** Used by {@link ReadGlobNode}. */ -public class ReadGlobElementNode extends ExpressionNode { - public ReadGlobElementNode(SourceSection sourceSection) { +public class ReadGlobMemberBodyNode extends ExpressionNode { + public ReadGlobMemberBodyNode(SourceSection sourceSection) { super(sourceSection); } @@ -38,13 +38,13 @@ public Object executeGeneric(VirtualFrame frame) { return readResource(mapping, path); } - @TruffleBoundary private Object readResource(VmObjectLike mapping, String path) { @SuppressWarnings("unchecked") var globElements = (Map) mapping.getExtraStorage(); - var resourceUri = globElements.get(path).getUri(); + var resourceUri = VmUtils.getMapValue(globElements, path).getUri(); var resource = VmContext.get(this).getResourceManager().read(resourceUri, this).orElse(null); if (resource == null) { + CompilerDirectives.transferToInterpreter(); throw exceptionBuilder().evalError("cannotFindResource", resourceUri).build(); } return resource; diff --git a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java index 070c1a3ea..1a58ed829 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java @@ -15,6 +15,7 @@ */ package org.pkl.core.ast.expression.unary; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.FrameDescriptor; @@ -32,23 +33,31 @@ import org.pkl.core.runtime.VmObjectBuilder; import org.pkl.core.util.GlobResolver; import org.pkl.core.util.GlobResolver.InvalidGlobPatternException; +import org.pkl.core.util.LateInit; @NodeInfo(shortName = "read*") public abstract class ReadGlobNode extends AbstractReadNode { - private final SharedMemberNode readGlobElementNode; private final EconomicMap cachedResults = EconomicMap.create(); + @Child @LateInit private SharedMemberNode memberNode; - protected ReadGlobNode( - VmLanguage language, SourceSection sourceSection, ModuleKey currentModule) { + protected ReadGlobNode(SourceSection sourceSection, ModuleKey currentModule) { super(sourceSection, currentModule); - readGlobElementNode = - new SharedMemberNode( - sourceSection, - sourceSection, - "", - language, - new FrameDescriptor(), - new ReadGlobElementNode(sourceSection)); + } + + private SharedMemberNode getMemberNode() { + if (memberNode == null) { + CompilerDirectives.transferToInterpreterAndInvalidate(); + var language = VmLanguage.get(this); + memberNode = + new SharedMemberNode( + sourceSection, + sourceSection, + "", + language, + new FrameDescriptor(), + new ReadGlobMemberBodyNode(sourceSection)); + } + return memberNode; } @Specialization @@ -76,9 +85,9 @@ public Object read(String globPattern) { currentModule, currentModule.getUri(), globPattern); - var builder = new VmObjectBuilder(); + var builder = new VmObjectBuilder(resolvedElements.size()); for (var entry : resolvedElements.entrySet()) { - builder.addEntry(entry.getKey(), readGlobElementNode); + builder.addEntry(entry.getKey(), getMemberNode()); } cachedResult = builder.toMapping(resolvedElements); cachedResults.put(globPattern, cachedResult); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java index 98d816ca9..dfe3abfd6 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java @@ -225,23 +225,19 @@ public void force(boolean allowUndefinedValues) { } } - @TruffleBoundary public VmMapping toMapping() { - var builder = new VmObjectBuilder(keyOrder.size()); - for (var key : keyOrder) { - var value = map.get(key); - assert value != null; - builder.addEntry(key, value); + var builder = new VmObjectBuilder(getLength()); + for (var entry : this) { + builder.addEntry(VmUtils.getKey(entry), VmUtils.getValue(entry)); } return builder.toMapping(); } - @TruffleBoundary public VmDynamic toDynamic() { - var builder = new VmObjectBuilder(keyOrder.size()); - for (var key : keyOrder) { - var value = map.get(key); - assert value != null; + var builder = new VmObjectBuilder(getLength()); + for (var entry : this) { + var key = VmUtils.getKey(entry); + var value = VmUtils.getValue(entry); if (key instanceof String) { builder.addProperty(Identifier.get((String) key), value); } else { diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java index 5a833b1e1..867d33fdd 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java @@ -26,10 +26,6 @@ public final class VmObjectBuilder { private final EconomicMap members; private int elementCount = 0; - public VmObjectBuilder() { - members = EconomicMaps.create(); - } - public VmObjectBuilder(int initialSize) { members = EconomicMaps.create(initialSize); } diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java index 7ccae1c9d..3f7d046ff 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java @@ -23,7 +23,7 @@ import org.pkl.core.Composite; import org.pkl.core.PModule; import org.pkl.core.PObject; -import org.pkl.core.ast.expression.unary.AbstractImportNode; +import org.pkl.core.ast.expression.unary.ImportNode; import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.LateInit; @@ -105,7 +105,7 @@ public VmMap getImports() { assert memberNode != null; // import is never a constant builder.add( member.getName().toString(), - ((AbstractImportNode) memberNode.getBodyNode()).getImportUri().toString()); + ((ImportNode) memberNode.getBodyNode()).getImportUri().toString()); } } return builder.build(); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java index ff1c35620..6fcbc305d 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java @@ -844,4 +844,9 @@ public static int findSlot(VirtualFrame frame, Object identifier) { public static int findAuxiliarySlot(VirtualFrame frame, Object identifier) { return frame.getFrameDescriptor().getAuxiliarySlots().getOrDefault(identifier, -1); } + + @TruffleBoundary + public static V getMapValue(Map map, K key) { + return map.get(key); + } } diff --git a/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java b/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java index 76cd11251..f722a77b9 100644 --- a/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java +++ b/pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java @@ -21,13 +21,11 @@ import com.oracle.truffle.api.nodes.LoopNode; import java.util.Map; import org.pkl.core.ast.lambda.*; -import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.runtime.*; import org.pkl.core.stdlib.ExternalMethod0Node; import org.pkl.core.stdlib.ExternalMethod1Node; import org.pkl.core.stdlib.ExternalMethod2Node; import org.pkl.core.stdlib.ExternalPropertyNode; -import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.MutableReference; public final class MapNodes { @@ -238,28 +236,7 @@ protected VmMap eval(VmMap self) { public abstract static class toDynamic extends ExternalMethod0Node { @Specialization protected VmDynamic eval(VmMap self) { - var members = EconomicMaps.create(self.getLength()); - - for (var entry : self) { - var key = VmUtils.getKey(entry); - - if (key instanceof String string) { - var name = Identifier.get(string); - EconomicMaps.put( - members, - name, - VmUtils.createSyntheticObjectProperty(name, "", VmUtils.getValue(entry))); - } else { - EconomicMaps.put( - members, key, VmUtils.createSyntheticObjectEntry("", VmUtils.getValue(entry))); - } - } - - return new VmDynamic( - VmUtils.createEmptyMaterializedFrame(), - BaseModule.getDynamicClass().getPrototype(), - members, - 0); + return self.toDynamic(); } } diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl deleted file mode 100644 index 4ac3867b7..000000000 --- a/pkl-core/src/test/files/LanguageSnippetTests/input/api/reflect4.pkl +++ /dev/null @@ -1,12 +0,0 @@ -amends ".../snippetTest.pkl" - -import "pkl:reflect" -import* "reflect*.pkl" - -local mod = reflect.Module(this) - -examples { - ["Reflecting globbed import"] { - mod.imports.keys == Set("reflect", "reflect*") - } -} diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf deleted file mode 100644 index 68d45e08d..000000000 --- a/pkl-core/src/test/files/LanguageSnippetTests/output/api/reflect4.pcf +++ /dev/null @@ -1,5 +0,0 @@ -examples { - ["Reflecting globbed import"] { - true - } -}