Skip to content

Commit

Permalink
Fix ClassCastException when builtin is invoked with wrong args (#12033)
Browse files Browse the repository at this point in the history
* Add failing test

* Error.catch_primitive throws type error on wrong self arg

* Move Error.catch_primitive to a private internal module

* error message contains just single string
  • Loading branch information
Akirathan authored Jan 15, 2025
1 parent 48aa65c commit 4324e65
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 12 deletions.
11 changes: 1 addition & 10 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Error.enso
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import project.Panic.Panic
import project.Runtime.Stack_Trace_Element
from project.Data.Boolean import Boolean, False, True
from project.Function import const
from project.Internal.Error_Builtins import all

## A type representing dataflow errors.

Expand Down Expand Up @@ -39,16 +40,6 @@ type Error
throw : Any -> Error
throw payload = @Builtin_Method "Error.throw"

## PRIVATE

Executes the provided handler on a dataflow error, or executes as
identity on a non-error value.

Arguments:
- handler: The function to call on this if it is an error value.
catch_primitive : (Error -> Any) -> Any
catch_primitive self handler = @Builtin_Method "Error.catch_primitive"

## PRIVATE
UNSTABLE

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
private

import project.Any.Any
import project.Error.Error

## PRIVATE

Executes the provided handler on a dataflow error, or executes as
identity on a non-error value.

Arguments:
- handler: The function to call on this if it is an error value.
Error.catch_primitive : (Error -> Any) -> Any
Error.catch_primitive self handler = @Builtin_Method "Error.catch_primitive"
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.enso.interpreter.test.builtins;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;

import org.enso.test.utils.ContextUtils;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.PolyglotException;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;

public class BuiltinsInvocationTest {
private static Context ctx;

@BeforeClass
public static void prepareCtx() {
ctx = ContextUtils.createDefaultContext();
}

@AfterClass
public static void disposeCtx() {
ctx.close();
ctx = null;
}

@Test
public void invokeBuiltinWithWrongArguments_ShouldNotCrash() {
var src =
"""
from Standard.Base import all
main =
(Error.catch_primitive self=(y->y)) (x->x)
""";
try {
ContextUtils.evalModule(ctx, src);
} catch (PolyglotException e) {
var panic = e.getGuestObject();
assertThat("Should be panic", panic.isException());
assertThat("Should have Type error as payload", e.getMessage(), containsString("Type error"));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package org.enso.interpreter.node.expression.builtin.error;

import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.Node;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.node.BaseNode;
import org.enso.interpreter.node.callable.InvokeCallableNode;
import org.enso.interpreter.runtime.EnsoContext;
import org.enso.interpreter.runtime.callable.argument.CallArgumentInfo;
import org.enso.interpreter.runtime.error.DataflowError;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.state.State;

@BuiltinMethod(
Expand All @@ -15,9 +19,15 @@
description =
"If called on an error, executes the provided handler on the error's payload. Otherwise"
+ " acts as identity.")
public class CatchErrorNode extends Node {
public abstract class CatchErrorNode extends Node {
private @Child InvokeCallableNode invokeCallableNode;

public abstract Object execute(VirtualFrame frame, State state, Object self, Object handler);

public static CatchErrorNode build() {
return CatchErrorNodeGen.create();
}

CatchErrorNode() {
this.invokeCallableNode =
InvokeCallableNode.build(
Expand All @@ -27,7 +37,15 @@ public class CatchErrorNode extends Node {
this.invokeCallableNode.setTailStatus(BaseNode.TailStatus.TAIL_DIRECT);
}

Object execute(VirtualFrame frame, State state, DataflowError self, Object handler) {
@Specialization
Object doDataflowError(VirtualFrame frame, State state, DataflowError self, Object handler) {
return invokeCallableNode.execute(handler, frame, state, new Object[] {self.getPayload()});
}

@Fallback
Object doOther(VirtualFrame frame, State state, Object self, Object handler) {
var builtins = EnsoContext.get(this).getBuiltins();
var typeErr = builtins.error().makeTypeError("Dataflow_Error", self, "self");
throw new PanicException(typeErr, this);
}
}

0 comments on commit 4324e65

Please sign in to comment.