Fix exception handling of PklRootNode's (#837)

Motivation:
- Perform same exception handling for every implementation of PklRootNode.execute().
- Avoid code duplication.

Changes:
- Change PklRootNode.execute() to be a final method that performs exception handling
  and calls abstract method executeImpl(), which is implemented by subclasses.
- Remove executeBody() methods, which served a similar purpose but were more limited.
- Remove duplicate exception handling code.

Result:
- More reliable exception handling.
  This should fix known problems such as misclassifying stack overflows
  as internal errors and displaying errors without a stack trace.
- Less code duplication.

pkl-core/src/main/java/org/pkl/core/ast/MemberNode.java

@@ -16,7 +16,6 @@
 package org.pkl.core.ast;
 
 import com.oracle.truffle.api.frame.FrameDescriptor;
-import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.source.SourceSection;
 import java.util.function.Function;
 import org.pkl.core.ast.member.DefaultPropertyBodyNode;
@@ -48,10 +47,6 @@ public abstract class MemberNode extends PklRootNode {
     bodyNode = insert(replacer.apply(bodyNode));
   }
 
-  protected final Object executeBody(VirtualFrame frame) {
-    return executeBody(frame, bodyNode);
-  }
-
   protected final VmExceptionBuilder exceptionBuilder() {
     return new VmExceptionBuilder().withSourceSection(getHeaderSection());
   }

pkl-core/src/main/java/org/pkl/core/ast/PklRootNode.java

@@ -22,6 +22,7 @@ import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.NodeInfo;
 import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.source.SourceSection;
+import org.pkl.core.ast.type.VmTypeMismatchException;
 import org.pkl.core.runtime.*;
 import org.pkl.core.util.Nullable;
 
@@ -36,9 +37,16 @@ public abstract class PklRootNode extends RootNode {
 
   public abstract @Nullable String getName();
 
-  protected final Object executeBody(VirtualFrame frame, ExpressionNode bodyNode) {
+  // name must start with `execute` (see Javadoc of @Specialization)
+  protected abstract Object executeImpl(VirtualFrame frame);
+
+  @Override
+  public final Object execute(VirtualFrame frame) {
     try {
-      return bodyNode.executeGeneric(frame);
+      return executeImpl(frame);
+    } catch (VmTypeMismatchException e) {
+      CompilerDirectives.transferToInterpreter();
+      throw e.toVmException();
     } catch (VmException e) {
       CompilerDirectives.transferToInterpreter();
       throw e;

pkl-core/src/main/java/org/pkl/core/ast/SimpleRootNode.java

@@ -49,7 +49,7 @@ public final class SimpleRootNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    return executeBody(frame, bodyNode);
+  protected Object executeImpl(VirtualFrame frame) {
+    return bodyNode.executeGeneric(frame);
   }
 }

pkl-core/src/main/java/org/pkl/core/ast/member/FunctionNode.java

@@ -29,7 +29,6 @@ import org.pkl.core.TypeParameter;
 import org.pkl.core.ast.ExpressionNode;
 import org.pkl.core.ast.VmModifier;
 import org.pkl.core.ast.type.TypeNode;
-import org.pkl.core.ast.type.VmTypeMismatchException;
 import org.pkl.core.runtime.*;
 import org.pkl.core.util.CollectionUtils;
 import org.pkl.core.util.Nullable;
@@ -106,7 +105,7 @@ public final class FunctionNode extends RegularMemberNode {
 
   @Override
   @ExplodeLoop
-  public Object execute(VirtualFrame frame) {
+  protected Object executeImpl(VirtualFrame frame) {
     var totalArgCount = frame.getArguments().length;
     if (totalArgCount != totalParamCount) {
       CompilerDirectives.transferToInterpreter();
@@ -114,37 +113,23 @@ public final class FunctionNode extends RegularMemberNode {
     }
 
     var isInIterable = (boolean) frame.getArguments()[2];
-    try {
-      for (var i = 0; i < parameterTypeNodes.length; i++) {
-        var argument = frame.getArguments()[IMPLICIT_PARAM_COUNT + i];
-        if (isInIterable) {
-          parameterTypeNodes[i].executeEagerlyAndSet(frame, argument);
-        } else {
-          parameterTypeNodes[i].executeAndSet(frame, argument);
-        }
-      }
 
-      var result = bodyNode.executeGeneric(frame);
-
-      if (checkedReturnTypeNode != null) {
-        return checkedReturnTypeNode.execute(frame, result);
-      }
-
-      return result;
-    } catch (VmTypeMismatchException e) {
-      CompilerDirectives.transferToInterpreter();
-      throw e.toVmException();
-    } catch (StackOverflowError e) {
-      CompilerDirectives.transferToInterpreter();
-      throw new VmStackOverflowException(e);
-    } catch (Exception e) {
-      CompilerDirectives.transferToInterpreter();
-      if (e instanceof VmException) {
-        throw e;
+    for (var i = 0; i < parameterTypeNodes.length; i++) {
+      var argument = frame.getArguments()[IMPLICIT_PARAM_COUNT + i];
+      if (isInIterable) {
+        parameterTypeNodes[i].executeEagerlyAndSet(frame, argument);
       } else {
-        throw exceptionBuilder().bug(e.getMessage()).withCause(e).build();
+        parameterTypeNodes[i].executeAndSet(frame, argument);
       }
     }
+
+    var result = bodyNode.executeGeneric(frame);
+
+    if (checkedReturnTypeNode != null) {
+      return checkedReturnTypeNode.execute(frame, result);
+    }
+
+    return result;
   }
 
   public VmMap getParameterMirrors() {

pkl-core/src/main/java/org/pkl/core/ast/member/ListingOrMappingTypeCastNode.java

@@ -15,13 +15,11 @@
  */
 package org.pkl.core.ast.member;
 
-import com.oracle.truffle.api.CompilerDirectives;
 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.PklRootNode;
 import org.pkl.core.ast.type.TypeNode;
-import org.pkl.core.ast.type.VmTypeMismatchException;
 import org.pkl.core.runtime.VmLanguage;
 import org.pkl.core.util.Nullable;
 
@@ -53,12 +51,7 @@ public final class ListingOrMappingTypeCastNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    try {
-      return typeNode.execute(frame, frame.getArguments()[2]);
-    } catch (VmTypeMismatchException e) {
-      CompilerDirectives.transferToInterpreter();
-      throw e.toVmException();
-    }
+  protected Object executeImpl(VirtualFrame frame) {
+    return typeNode.execute(frame, frame.getArguments()[2]);
   }
 }

pkl-core/src/main/java/org/pkl/core/ast/member/LocalTypedPropertyNode.java

@@ -21,8 +21,6 @@ import com.oracle.truffle.api.frame.VirtualFrame;
 import org.pkl.core.ast.ExpressionNode;
 import org.pkl.core.ast.type.TypeNode;
 import org.pkl.core.ast.type.UnresolvedTypeNode;
-import org.pkl.core.ast.type.VmTypeMismatchException;
-import org.pkl.core.runtime.VmException;
 import org.pkl.core.runtime.VmLanguage;
 import org.pkl.core.util.LateInit;
 import org.pkl.core.util.Nullable;
@@ -58,25 +56,13 @@ public final class LocalTypedPropertyNode extends RegularMemberNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    try {
-      if (typeNode == null) {
-        CompilerDirectives.transferToInterpreter();
-        typeNode = insert(unresolvedTypeNode.execute(frame));
-        unresolvedTypeNode = null;
-      }
-      var result = bodyNode.executeGeneric(frame);
-      return typeNode.execute(frame, result);
-    } catch (VmTypeMismatchException e) {
+  protected Object executeImpl(VirtualFrame frame) {
+    if (typeNode == null) {
       CompilerDirectives.transferToInterpreter();
-      throw e.toVmException();
-    } catch (Exception e) {
-      CompilerDirectives.transferToInterpreter();
-      if (e instanceof VmException) {
-        throw e;
-      } else {
-        throw exceptionBuilder().bug(e.getMessage()).withCause(e).build();
-      }
+      typeNode = insert(unresolvedTypeNode.execute(frame));
+      unresolvedTypeNode = null;
     }
+    var result = bodyNode.executeGeneric(frame);
+    return typeNode.execute(frame, result);
   }
 }

pkl-core/src/main/java/org/pkl/core/ast/member/ModuleNode.java

@@ -52,8 +52,8 @@ public final class ModuleNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    var module = executeBody(frame, moduleNode);
+  protected Object executeImpl(VirtualFrame frame) {
+    var module = moduleNode.executeGeneric(frame);
     if (module instanceof VmClass vmClass) {
       return vmClass.getPrototype();
     }

pkl-core/src/main/java/org/pkl/core/ast/member/ObjectMethodNode.java

@@ -59,7 +59,7 @@ public final class ObjectMethodNode extends RegularMemberNode {
   }
 
   @Override
-  public CallTarget execute(VirtualFrame frame) {
+  protected CallTarget executeImpl(VirtualFrame frame) {
     if (functionNode == null) {
       CompilerDirectives.transferToInterpreter();
 

pkl-core/src/main/java/org/pkl/core/ast/member/PropertyTypeNode.java

@@ -15,7 +15,6 @@
  */
 package org.pkl.core.ast.member;
 
-import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.FrameDescriptor;
 import com.oracle.truffle.api.frame.VirtualFrame;
@@ -24,7 +23,6 @@ import org.pkl.core.PType;
 import org.pkl.core.ast.PklRootNode;
 import org.pkl.core.ast.type.TypeNode;
 import org.pkl.core.ast.type.TypeNode.UnknownTypeNode;
-import org.pkl.core.ast.type.VmTypeMismatchException;
 import org.pkl.core.runtime.*;
 import org.pkl.core.util.Nullable;
 
@@ -67,31 +65,19 @@ public final class PropertyTypeNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    try {
-      if (isInIterable(frame)) {
-        // There is currently a bug around resolving variables within the iterable of a for
-        // generator or spread syntax (https://github.com/apple/pkl/issues/741)
-        //
-        // Normally, mappings/listings are type-checked lazily. However, this results in said
-        // bug getting widened, for any object members declared in the iterable.
-        //
-        // As a workaround for now, prevent the bug from being any worse by ensuring that these
-        // object members are eagerly typechecked.
-        return typeNode.executeEagerly(frame, frame.getArguments()[2]);
-      }
-      return typeNode.execute(frame, frame.getArguments()[2]);
-    } catch (VmTypeMismatchException e) {
-      CompilerDirectives.transferToInterpreter();
-      throw e.toVmException();
-    } catch (Exception e) {
-      CompilerDirectives.transferToInterpreter();
-      if (e instanceof VmException) {
-        throw e;
-      } else {
-        throw exceptionBuilder().bug(e.getMessage()).withCause(e).build();
-      }
+  protected Object executeImpl(VirtualFrame frame) {
+    if (isInIterable(frame)) {
+      // There is currently a bug around resolving variables within the iterable of a for
+      // generator or spread syntax (https://github.com/apple/pkl/issues/741)
+      //
+      // Normally, mappings/listings are type-checked lazily. However, this results in said
+      // bug getting widened, for any object members declared in the iterable.
+      //
+      // As a workaround for now, prevent the bug from being any worse by ensuring that these
+      // object members are eagerly typechecked.
+      return typeNode.executeEagerly(frame, frame.getArguments()[2]);
     }
+    return typeNode.execute(frame, frame.getArguments()[2]);
   }
 
   public @Nullable Object getDefaultValue() {

pkl-core/src/main/java/org/pkl/core/ast/member/SharedMemberNode.java

@@ -58,7 +58,7 @@ public class SharedMemberNode extends MemberNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    return executeBody(frame);
+  protected Object executeImpl(VirtualFrame frame) {
+    return bodyNode.executeGeneric(frame);
   }
 }

pkl-core/src/main/java/org/pkl/core/ast/member/TypeCheckedPropertyNode.java

@@ -51,7 +51,7 @@ public abstract class TypeCheckedPropertyNode extends RegularMemberNode {
       @Cached("getProperty(cachedOwnerClass)") ClassProperty property,
       @Cached("createTypeCheckCallNode(property)") @Nullable DirectCallNode callNode) {
 
-    var result = executeBody(frame);
+    var result = bodyNode.executeGeneric(frame);
 
     // TODO: propagate SUPER_CALL_MARKER to disable constraint (but not type) check
     if (callNode != null && VmUtils.shouldRunTypeCheck(frame)) {
@@ -66,7 +66,7 @@ public abstract class TypeCheckedPropertyNode extends RegularMemberNode {
   protected Object eval(
       VirtualFrame frame, VmObjectLike owner, @Cached("create()") IndirectCallNode callNode) {
 
-    var result = executeBody(frame);
+    var result = bodyNode.executeGeneric(frame);
 
     if (VmUtils.shouldRunTypeCheck(frame)) {
       var property = getProperty(owner.getVmClass());
@@ -86,7 +86,7 @@ public abstract class TypeCheckedPropertyNode extends RegularMemberNode {
 
   @Specialization
   protected Object eval(VirtualFrame frame, @SuppressWarnings("unused") VmDynamic owner) {
-    return executeBody(frame);
+    return bodyNode.executeGeneric(frame);
   }
 
   protected ClassProperty getProperty(VmClass ownerClass) {

pkl-core/src/main/java/org/pkl/core/ast/member/TypedPropertyNode.java

@@ -43,8 +43,8 @@ public final class TypedPropertyNode extends RegularMemberNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    var propertyValue = executeBody(frame);
+  protected Object executeImpl(VirtualFrame frame) {
+    var propertyValue = bodyNode.executeGeneric(frame);
     if (VmUtils.shouldRunTypeCheck(frame)) {
       return typeCheckCallNode.call(
           VmUtils.getReceiver(frame),

pkl-core/src/main/java/org/pkl/core/ast/member/UntypedObjectMemberNode.java

@@ -32,7 +32,7 @@ public final class UntypedObjectMemberNode extends RegularMemberNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
-    return executeBody(frame);
+  protected Object executeImpl(VirtualFrame frame) {
+    return bodyNode.executeGeneric(frame);
   }
 }

pkl-core/src/main/java/org/pkl/core/ast/repl/ResolveClassMemberNode.java

@@ -49,7 +49,7 @@ public final class ResolveClassMemberNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
+  protected Object executeImpl(VirtualFrame frame) {
     return unresolvedNode.execute(frame, clazz);
   }
 }

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

@@ -20,7 +20,6 @@ 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.PklRootNode;
-import org.pkl.core.runtime.VmException;
 import org.pkl.core.runtime.VmLanguage;
 import org.pkl.core.util.Nullable;
 
@@ -53,7 +52,7 @@ public final class IdentityMixinNode extends PklRootNode {
   }
 
   @Override
-  public Object execute(VirtualFrame frame) {
+  protected Object executeImpl(VirtualFrame frame) {
     var arguments = frame.getArguments();
     if (arguments.length != 4) {
       CompilerDirectives.transferToInterpreter();
@@ -62,23 +61,10 @@ public final class IdentityMixinNode extends PklRootNode {
           .withSourceSection(sourceSection)
           .build();
     }
-
-    try {
-      var argument = arguments[3];
-      if (argumentTypeNode != null) {
-        return argumentTypeNode.execute(frame, argument);
-      }
-      return argument;
-    } catch (VmTypeMismatchException e) {
-      CompilerDirectives.transferToInterpreter();
-      throw e.toVmException();
-    } catch (Exception e) {
-      CompilerDirectives.transferToInterpreter();
-      if (e instanceof VmException) {
-        throw e;
-      } else {
-        throw exceptionBuilder().bug(e.getMessage()).withCause(e).build();
-      }
+    var argument = arguments[3];
+    if (argumentTypeNode != null) {
+      return argumentTypeNode.execute(frame, argument);
     }
+    return argument;
   }
 }

pkl-core/src/main/java/org/pkl/core/stdlib/benchmark/MicrobenchmarkNodes.java

@@ -17,7 +17,6 @@ package org.pkl.core.stdlib.benchmark;
 
 import static org.pkl.core.stdlib.benchmark.BenchmarkUtils.runBenchmark;
 
-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;
@@ -29,7 +28,6 @@ import org.pkl.core.ast.PklRootNode;
 import org.pkl.core.ast.internal.BlackholeNode;
 import org.pkl.core.ast.internal.BlackholeNodeGen;
 import org.pkl.core.runtime.Identifier;
-import org.pkl.core.runtime.VmException;
 import org.pkl.core.runtime.VmLanguage;
 import org.pkl.core.runtime.VmTyped;
 import org.pkl.core.runtime.VmUtils;
@@ -83,23 +81,14 @@ public final class MicrobenchmarkNodes {
     }
 
     @Override
-    public @Nullable Object execute(VirtualFrame frame) {
-      try {
-        var repetitions = (long) frame.getArguments()[2];
-        for (long i = 0; i < repetitions; i++) {
-          blackholeNode.executeGeneric(frame);
-        }
-        LoopNode.reportLoopCount(
-            this, repetitions > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) repetitions);
-        return null;
-      } catch (Exception e) {
-        CompilerDirectives.transferToInterpreter();
-        if (e instanceof VmException) {
-          throw e;
-        } else {
-          throw exceptionBuilder().bug(e.getMessage()).withCause(e).build();
-        }
+    protected @Nullable Object executeImpl(VirtualFrame frame) {
+      var repetitions = (long) frame.getArguments()[2];
+      for (long i = 0; i < repetitions; i++) {
+        blackholeNode.executeGeneric(frame);
       }
+      LoopNode.reportLoopCount(
+          this, repetitions > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) repetitions);
+      return null;
     }
   }
 }