Improve error message for aliased references

pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java

@@ -27,7 +27,9 @@
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.LoopNode;
+import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.source.SourceSection;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -38,6 +40,7 @@
 import org.pkl.core.PType;
 import org.pkl.core.PType.StringLiteral;
 import org.pkl.core.PklBugException;
+import org.pkl.core.StackFrame;
 import org.pkl.core.TypeParameter;
 import org.pkl.core.ast.*;
 import org.pkl.core.ast.builder.SymbolTable.CustomThisScope;
@@ -54,6 +57,7 @@
 import org.pkl.core.util.EconomicSets;
 import org.pkl.core.util.LateInit;
 import org.pkl.core.util.MutableBoolean;
+import org.pkl.core.util.MutableReference;
 
 public abstract class TypeNode extends PklNode {
 
@@ -2138,7 +2142,17 @@ public ReferenceTypeNode(
       this.domainTypeNode = domainTypeNode;
       this.referentTypeNode = referentTypeNode;
       this.getModuleNode = new GetModuleNode(sourceSection);
-      validateTypeArguments(sourceSection);
+      // A type constraint anywhere in the referent is forbidden, including one reached through a
+      // type alias used in the referent.
+      var constraint = findReferentConstraint();
+      if (constraint != null) {
+        CompilerDirectives.transferToInterpreter();
+        throw exceptionBuilder()
+            .evalError("invalidReferenceTypeAnnotationWithConstraint")
+            .withLeadingStackFrames(
+                buildReferentConstraintFrames(constraint, getSourceSection(), null))
+            .build();
+      }
     }
 
     @Specialization
@@ -2173,26 +2187,67 @@ private Object doEval(VmReference value, VmTyped module) {
           sourceSection, value, TypeNode.export(domainTypeNode), referentType);
     }
 
-    public void validateTypeArguments(@Nullable SourceSection aliasSourceSection) {
-      // constraints may not be used in Reference type annotation referents
-      // walk the type and throw if any part of the referent is constrained
-
-      // TODO improve error message when this type node and/or referent constraint are behind type
-      // aliases
+    /**
+     * Type constraints may not appear anywhere in a {@code Reference}'s referent type argument.
+     * Walks the referent type and returns the first offending {@link ConstrainedTypeNode} , or
+     * {@code null} if the referent is constraint-free.
+     */
+    public @Nullable ConstrainedTypeNode findReferentConstraint() {
+      var found = new MutableReference<@Nullable ConstrainedTypeNode>(null);
       referentTypeNode.acceptTypeNode(
           true,
           (typeNode) -> {
-            if (typeNode instanceof ConstrainedTypeNode) {
-              CompilerDirectives.transferToInterpreter();
-              var err =
-                  exceptionBuilder().evalError("invalidReferenceTypeAnnotationWithConstraint");
-              if (aliasSourceSection != null) {
-                err.withSourceSection(aliasSourceSection);
-              }
-              throw err.build();
+            if (typeNode instanceof ConstrainedTypeNode constrainedTypeNode) {
+              found.set(constrainedTypeNode);
+              return false;
             }
             return true;
           });
+      return found.getOrNull();
+    }
+
+    /** Builds the frames to show ahead of an "invalid referent constraint" error. */
+    public static List<StackFrame> buildReferentConstraintFrames(
+        ConstrainedTypeNode constraintNode,
+        SourceSection usageSection,
+        @Nullable VmTypeAlias outermostAlias) {
+      var frames = new ArrayList<StackFrame>();
+      for (Node node = constraintNode; node != null; node = node.getParent()) {
+        if (!(node instanceof ConstrainedTypeNode
+            || node instanceof TypeAliasTypeNode
+            || node instanceof ReferenceTypeNode)) {
+          continue;
+        }
+        var section = node.getSourceSection();
+        if (section == null || !section.isAvailable() || isWithin(usageSection, section)) {
+          continue;
+        }
+        var owner = ownerAlias(node, outermostAlias);
+        if (owner != null) {
+          frames.add(VmUtils.createStackFrame(section, owner.getQualifiedName()));
+        }
+      }
+      return frames;
+    }
+
+    /**
+     * The type alias whose body contains {@code node}: the nearest enclosing alias, else the
+     * outermost alias being instantiated (which is {@code null} for a directly-used Reference).
+     */
+    private static @Nullable VmTypeAlias ownerAlias(
+        Node node, @Nullable VmTypeAlias outermostAlias) {
+      for (var parent = node.getParent(); parent != null; parent = parent.getParent()) {
+        if (parent instanceof TypeAliasTypeNode aliasNode) {
+          return aliasNode.getTypeAlias();
+        }
+      }
+      return outermostAlias;
+    }
+
+    private static boolean isWithin(SourceSection outer, SourceSection inner) {
+      return inner.getSource().equals(outer.getSource())
+          && inner.getCharIndex() >= outer.getCharIndex()
+          && inner.getCharEndIndex() <= outer.getCharEndIndex();
     }
 
     @Fallback
@@ -2703,13 +2758,42 @@ public TypeAliasTypeNode(
 
       this.typeAlias = typeAlias;
       this.typeArgumentNodes = typeArgumentNodes;
-      aliasedTypeNode = typeAlias.instantiate(typeArgumentNodes, sourceSection);
+      aliasedTypeNode = typeAlias.instantiate(typeArgumentNodes);
+      checkReferentConstraints(typeAlias);
+    }
+
+    /**
+     * Reports a forbidden type constraint that a type argument introduced into a {@code
+     * Reference}'s referent through this (generic) alias. The error is reported at this usage type
+     * expression, with leading frames for the constraint and every alias layer it passed through.
+     */
+    private void checkReferentConstraints(VmTypeAlias outermostAlias) {
+      aliasedTypeNode.accept(
+          node -> {
+            if (node instanceof ReferenceTypeNode referenceTypeNode) {
+              var constraint = referenceTypeNode.findReferentConstraint();
+              if (constraint != null) {
+                CompilerDirectives.transferToInterpreter();
+                throw exceptionBuilder()
+                    .evalError("invalidReferenceTypeAnnotationWithConstraint")
+                    .withLeadingStackFrames(
+                        ReferenceTypeNode.buildReferentConstraintFrames(
+                            constraint, getSourceSection(), outermostAlias))
+                    .build();
+              }
+            }
+            return true;
+          });
     }
 
     public TypeNode getAliasedTypeNode() {
       return aliasedTypeNode;
     }
 
+    public VmTypeAlias getTypeAlias() {
+      return typeAlias;
+    }
+
     @Override
     public FrameSlotKind getFrameSlotKind() {
       return aliasedTypeNode.getFrameSlotKind();

pkl-core/src/main/java/org/pkl/core/runtime/StackTraceGenerator.java

@@ -38,6 +38,10 @@ private StackTraceGenerator(VmException exception) {
   }
 
   private List<StackFrame> capture() {
+    // frames that aren't part of the runtime call stack are
+    // shown ahead of the captured frames.
+    frames.addAll(exception.getLeadingStackFrames());
+
     var truffleElements = TruffleStackTrace.getStackTrace(exception);
     if (truffleElements.isEmpty()) {
       addFrame(exception.getSourceSection(), exception.getMemberName());

pkl-core/src/main/java/org/pkl/core/runtime/VmException.java

@@ -34,6 +34,7 @@ public abstract class VmException extends AbstractTruffleException {
   private final @Nullable SourceSection sourceSection;
   private final @Nullable String memberName;
   private final Map<CallTarget, StackFrame> insertedStackFrames;
+  private List<StackFrame> leadingStackFrames = List.of();
   @Nullable private final BiConsumer<AnsiStringBuilder, Boolean> messageBuilder;
   @Nullable protected BiConsumer<AnsiStringBuilder, Boolean> hintBuilder;
 
@@ -89,6 +90,18 @@ public final Map<CallTarget, StackFrame> getInsertedStackFrames() {
     return insertedStackFrames;
   }
 
+  /**
+   * Stack frames to prepend to the rendered stack trace, ahead of the captured frames. Used to show
+   * source locations that aren't part of the runtime call stack.
+   */
+  public final List<StackFrame> getLeadingStackFrames() {
+    return leadingStackFrames;
+  }
+
+  public final void setLeadingStackFrames(List<StackFrame> leadingStackFrames) {
+    this.leadingStackFrames = leadingStackFrames;
+  }
+
   public @Nullable BiConsumer<AnsiStringBuilder, Boolean> getMessageBuilder() {
     return messageBuilder;
   }

pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionBuilder.java

@@ -99,6 +99,7 @@ public String toString() {
   private @Nullable Node location;
   private @Nullable SourceSection sourceSection;
   private @Nullable String memberName;
+  private List<StackFrame> leadingStackFrames = List.of();
 
   public VmExceptionBuilder typeMismatch(Object value, VmClass expectedType) {
     if (value instanceof VmNull) {
@@ -359,6 +360,15 @@ public VmExceptionBuilder withInsertedStackFrames(
     return this;
   }
 
+  /**
+   * Frames to show ahead of the captured stack trace (see {@link
+   * VmException#getLeadingStackFrames()}).
+   */
+  public VmExceptionBuilder withLeadingStackFrames(List<StackFrame> leadingStackFrames) {
+    this.leadingStackFrames = leadingStackFrames;
+    return this;
+  }
+
   public VmException build() {
     if (message != null && messageBuilder != null) {
       throw new IllegalStateException("Both message and messageBuilder are set");
@@ -374,64 +384,67 @@ public VmException build() {
 
     var effectiveInsertedStackFrames =
         insertedStackFrames == null ? new HashMap<CallTarget, StackFrame>() : insertedStackFrames;
-    return switch (kind) {
-      case EVAL_ERROR ->
-          new VmEvalException(
-              message,
-              cause,
-              isExternalMessage,
-              messageArguments,
-              messageBuilder,
-              programValues,
-              location,
-              sourceSection,
-              memberName,
-              hintBuilder,
-              effectiveInsertedStackFrames);
-      case UNDEFINED_VALUE ->
-          new VmUndefinedValueException(
-              message,
-              cause,
-              isExternalMessage,
-              messageArguments,
-              messageBuilder,
-              programValues,
-              location,
-              sourceSection,
-              memberName,
-              hintBuilder,
-              receiver,
-              effectiveInsertedStackFrames);
-      case BUG ->
-          new VmBugException(
-              message,
-              cause,
-              isExternalMessage,
-              messageArguments,
-              messageBuilder,
-              programValues,
-              location,
-              sourceSection,
-              memberName,
-              hintBuilder,
-              effectiveInsertedStackFrames);
-      case WRAPPED -> {
-        assert wrappedException != null;
-        yield new VmWrappedEvalException(
-            message,
-            cause,
-            isExternalMessage,
-            messageArguments,
-            messageBuilder,
-            programValues,
-            location,
-            sourceSection,
-            memberName,
-            hintBuilder,
-            effectiveInsertedStackFrames,
-            wrappedException);
-      }
-    };
+    var exception =
+        switch (kind) {
+          case EVAL_ERROR ->
+              new VmEvalException(
+                  message,
+                  cause,
+                  isExternalMessage,
+                  messageArguments,
+                  messageBuilder,
+                  programValues,
+                  location,
+                  sourceSection,
+                  memberName,
+                  hintBuilder,
+                  effectiveInsertedStackFrames);
+          case UNDEFINED_VALUE ->
+              new VmUndefinedValueException(
+                  message,
+                  cause,
+                  isExternalMessage,
+                  messageArguments,
+                  messageBuilder,
+                  programValues,
+                  location,
+                  sourceSection,
+                  memberName,
+                  hintBuilder,
+                  receiver,
+                  effectiveInsertedStackFrames);
+          case BUG ->
+              new VmBugException(
+                  message,
+                  cause,
+                  isExternalMessage,
+                  messageArguments,
+                  messageBuilder,
+                  programValues,
+                  location,
+                  sourceSection,
+                  memberName,
+                  hintBuilder,
+                  effectiveInsertedStackFrames);
+          case WRAPPED -> {
+            assert wrappedException != null;
+            yield new VmWrappedEvalException(
+                message,
+                cause,
+                isExternalMessage,
+                messageArguments,
+                messageBuilder,
+                programValues,
+                location,
+                sourceSection,
+                memberName,
+                hintBuilder,
+                effectiveInsertedStackFrames,
+                wrappedException);
+          }
+        };
+    exception.setLeadingStackFrames(leadingStackFrames);
+    return exception;
   }
 
   private List<Identifier> collectPropertyNames(VmObjectLike object, boolean isRead) {

pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java

@@ -32,7 +32,6 @@
 import org.pkl.core.ast.VmModifier;
 import org.pkl.core.ast.type.TypeNode;
 import org.pkl.core.ast.type.TypeNode.ConstrainedTypeNode;
-import org.pkl.core.ast.type.TypeNode.ReferenceTypeNode;
 import org.pkl.core.ast.type.TypeNode.TypeVariableNode;
 import org.pkl.core.ast.type.TypeNode.UnknownTypeNode;
 import org.pkl.core.util.LateInit;
@@ -178,8 +177,7 @@ public Frame getEnclosingFrame() {
   }
 
   @TruffleBoundary
-  public TypeNode instantiate(
-      TypeNode[] typeArgumentNodes, SourceSection typeAliasTypeNodeSourceSection) {
+  public TypeNode instantiate(TypeNode[] typeArgumentNodes) {
     // Cloning the type node means that the entire type check remains within a single root node,
     // which should be good for interpreted and compiled performance alike:
     // * Fewer root nodes to call
@@ -201,13 +199,6 @@ public TypeNode instantiate(
           }
           return true;
         });
-    clone.accept(
-        node -> {
-          if (node instanceof ReferenceTypeNode referenceTypeNode) {
-            referenceTypeNode.validateTypeArguments(typeAliasTypeNodeSourceSection);
-          }
-          return true;
-        });
 
     return clone;
   }

pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference21.pkl

@@ -0,0 +1,7 @@
+import "pkl:ref"
+
+typealias Ref<T> = ref.Reference<ref.Domain, T>
+
+typealias Ref2<T> = Ref<T>
+
+foo: Ref2<String(isEmpty)>