From 3d1db258648827762f22dfc6e067fa22226cea1d Mon Sep 17 00:00:00 2001 From: Kushal Pisavadia Date: Thu, 15 Feb 2024 21:28:10 +0000 Subject: [PATCH] Fix name resolution in `typealias` with constraint (#144) The body of a typealias gets inlined into wherever the typealias is used. This fixes a bug where the references in the typealias body can resolve to the wrong value. --- .../pkl/core/ast/member/TypeAliasNode.java | 3 ++- .../java/org/pkl/core/ast/type/TypeNode.java | 23 +++++++++++++++++++ .../org/pkl/core/runtime/VmTypeAlias.java | 11 ++++++++- .../java/org/pkl/core/runtime/VmUtils.java | 8 +++++++ .../types/typeAliasConstraint2.pkl | 3 +++ .../input/types/typeAliasConstraint1.pkl | 12 ++++++++++ .../input/types/typeAliasConstraint2.pkl | 8 +++++++ .../output/api/string.pcf | 8 +++---- .../output/types/typeAliasConstraint1.pcf | 2 ++ .../output/types/typeAliasConstraint2.pcf | 3 +++ stdlib/base.pkl | 2 +- 11 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/typeAliasConstraint2.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint1.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint2.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint1.pcf create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint2.pcf diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeAliasNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeAliasNode.java index f3a6614c..6f049160 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/TypeAliasNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/TypeAliasNode.java @@ -82,7 +82,8 @@ public final class TypeAliasNode extends ExpressionNode { simpleName, module, qualifiedName, - typeParameters); + typeParameters, + frame.materialize()); VmUtils.evaluateAnnotations(frame, annotationNodes, annotations); cachedTypeAlias.initTypeCheckNode(typeAnnotationNode.execute(frame)); diff --git a/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java b/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java index cf535162..8670331c 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java @@ -1614,13 +1614,36 @@ public abstract class TypeNode extends PklNode { return getMirrors(typeArgumentNodes); } + /** + * A typealias body is effectively inlined into the type node, and not executed in its own + * frame. + * + *

Before executing the typealias body, use the owner and receiver of the original frame + * where the typealias was declared, so that we preserve its original scope. + */ public void execute(VirtualFrame frame, Object value) { + var prevOwner = VmUtils.getOwner(frame); + var prevReceiver = VmUtils.getReceiver(frame); + VmUtils.setOwner(frame, VmUtils.getOwner(typeAlias.getEnclosingFrame())); + VmUtils.setReceiver(frame, VmUtils.getReceiver(typeAlias.getEnclosingFrame())); + aliasedTypeNode.execute(frame, value); + VmUtils.setOwner(frame, prevOwner); + VmUtils.setReceiver(frame, prevReceiver); } + /** See docstring on {@link TypeAliasTypeNode#execute}. */ @Override public void executeAndSet(VirtualFrame frame, Object value) { + var prevOwner = VmUtils.getOwner(frame); + var prevReceiver = VmUtils.getReceiver(frame); + VmUtils.setOwner(frame, VmUtils.getOwner(typeAlias.getEnclosingFrame())); + VmUtils.setReceiver(frame, VmUtils.getReceiver(typeAlias.getEnclosingFrame())); + aliasedTypeNode.executeAndSet(frame, value); + + VmUtils.setOwner(frame, prevOwner); + VmUtils.setReceiver(frame, prevReceiver); } @Override diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java index 553aa8b9..906ba1a2 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java @@ -16,6 +16,8 @@ package org.pkl.core.runtime; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.frame.Frame; +import com.oracle.truffle.api.frame.MaterializedFrame; import com.oracle.truffle.api.source.SourceSection; import java.util.ArrayList; import java.util.List; @@ -42,6 +44,7 @@ public final class VmTypeAlias extends VmValue { private final VmTyped module; private final String qualifiedName; private final List typeParameters; + private final MaterializedFrame enclosingFrame; @LateInit private TypeNode typeNode; @@ -66,7 +69,8 @@ public final class VmTypeAlias extends VmValue { String simpleName, VmTyped module, String qualifiedName, - List typeParameters) { + List typeParameters, + MaterializedFrame enclosingFrame) { this.sourceSection = sourceSection; this.headerSection = headerSection; this.docComment = docComment; @@ -76,6 +80,7 @@ public final class VmTypeAlias extends VmValue { this.module = module; this.qualifiedName = qualifiedName; this.typeParameters = typeParameters; + this.enclosingFrame = enclosingFrame; } public void initTypeCheckNode(TypeNode typeNode) { @@ -155,6 +160,10 @@ public final class VmTypeAlias extends VmValue { return typeNode; } + public Frame getEnclosingFrame() { + return enclosingFrame; + } + @TruffleBoundary public TypeNode instantiate(TypeNode[] typeArgumentNodes) { // Cloning the type node means that the entire type check remains within a single root node, 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 7c835340..b44b7390 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 @@ -136,6 +136,10 @@ public final class VmUtils { return result; } + public static void setReceiver(Frame frame, Object receiver) { + frame.getArguments()[0] = receiver; + } + public static VmObjectLike getObjectReceiver(Frame frame) { return (VmObjectLike) getReceiver(frame); } @@ -156,6 +160,10 @@ public final class VmUtils { return result; } + public static void setOwner(Frame frame, VmObjectLike owner) { + frame.getArguments()[1] = owner; + } + /** Returns a `ObjectMember`'s key while executing the corresponding `MemberNode`. */ public static Object getMemberKey(Frame frame) { return frame.getArguments()[2]; diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/typeAliasConstraint2.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/typeAliasConstraint2.pkl new file mode 100644 index 00000000..3b681f83 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/typeAliasConstraint2.pkl @@ -0,0 +1,3 @@ +class MyClass + +typealias MyTypeAlias = MyClass(getClass() == MyClass) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint1.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint1.pkl new file mode 100644 index 00000000..31f09373 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint1.pkl @@ -0,0 +1,12 @@ +// length should bind to `String#length` instead of `ClassWithLengthProperty#length` +local typealias OneCharacterString = String(length == 1) + +local class ClassWithLengthProperty { + length: Int = 99 + + // length constraint binds to ClassWithLengthProperty.length here + characters: List = List("a", "b") +} + +res1: OneCharacterString = "a" +res2 = let (c = new ClassWithLengthProperty {}) c.characters diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint2.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint2.pkl new file mode 100644 index 00000000..1e53c785 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/types/typeAliasConstraint2.pkl @@ -0,0 +1,8 @@ +import ".../input-helper/types/typeAliasConstraint2.pkl" as helper + +foo: Int = 1 +bar: helper.MyTypeAlias(foo == 1) = new helper.MyClass {} + +function qux(bar: helper.MyTypeAlias(foo == 1)) = (bar) {} + +qux = qux(new helper.MyTypeAlias {}) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/api/string.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/api/string.pcf index 376fc7e0..a8470a9b 100644 --- a/pkl-core/src/test/files/LanguageSnippetTests/output/api/string.pcf +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/api/string.pcf @@ -341,8 +341,8 @@ examples { "abcdefg" "abcdefg" " abcdefg" - "Type constraint `this.length == 1` violated. Value: \"\"" - "Type constraint `this.length == 1` violated. Value: \"aa\"" + "Type constraint `length == 1` violated. Value: \"\"" + "Type constraint `length == 1` violated. Value: \"aa\"" } ["padEnd()"] { "" @@ -351,8 +351,8 @@ examples { "abcdefg" "abcdefg" "abcdefg " - "Type constraint `this.length == 1` violated. Value: \"\"" - "Type constraint `this.length == 1` violated. Value: \"aa\"" + "Type constraint `length == 1` violated. Value: \"\"" + "Type constraint `length == 1` violated. Value: \"aa\"" } ["toBooleanOrNull()"] { true diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint1.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint1.pcf new file mode 100644 index 00000000..67d93d37 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint1.pcf @@ -0,0 +1,2 @@ +res1 = "a" +res2 = List("a", "b") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint2.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint2.pcf new file mode 100644 index 00000000..768fe5cd --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/types/typeAliasConstraint2.pcf @@ -0,0 +1,3 @@ +foo = 1 +bar {} +qux {} diff --git a/stdlib/base.pkl b/stdlib/base.pkl index e3093588..760d29dc 100644 --- a/stdlib/base.pkl +++ b/stdlib/base.pkl @@ -1058,7 +1058,7 @@ external class Boolean extends Any { } /// A Unicode character (code point). -typealias Char = String(this.length == 1) // `this.` works around rdar://75074742 +typealias Char = String(length == 1) /// A sequence of Unicode characters (code points). ///