Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate Dataflow Errors in Host and Polyglot #1941

Merged
merged 6 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

- Fixed a bug where visualizations would sometimes randomly fail to compute, due
to thread interrupts ([#1939](https://github.com/enso-org/enso/pull/1939)).
- Fixed an issue where both host and polyglot interop would not properly
propagate error information
([#1941](https://github.com/enso-org/enso/pull/1941)).

## Tooling

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.oracle.truffle.api.library.CachedLibrary;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.profiles.BranchProfile;
iamrecursion marked this conversation as resolved.
Show resolved Hide resolved
import com.oracle.truffle.api.profiles.ConditionProfile;
import org.enso.interpreter.Language;
import org.enso.interpreter.node.BaseNode;
Expand Down Expand Up @@ -153,13 +154,14 @@ Stateful doPolyglot(
@Bind("getPolyglotCallType(_this, symbol.getName(), interop)")
HostMethodCallNode.PolyglotCallType polyglotCallType,
@Cached ThunkExecutorNode argExecutor,
@Cached AnyResolverNode anyResolverNode,
@Cached HostMethodCallNode hostMethodCallNode,
@Cached IndirectInvokeFunctionNode invokeFunctionNode) {

Object[] args = new Object[arguments.length - 1];
for (int i = 0; i < arguments.length - 1; i++) {
Stateful r = argExecutor.executeThunk(arguments[i + 1], state, BaseNode.TailStatus.NOT_TAIL);
if (r.getValue() instanceof DataflowError) {
return r;
}
state = r.getState();
args[i] = r.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.enso.interpreter.node.callable;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.TruffleLanguage;
import com.oracle.truffle.api.dsl.*;
import com.oracle.truffle.api.frame.VirtualFrame;
Expand All @@ -9,6 +8,7 @@
import com.oracle.truffle.api.library.CachedLibrary;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.profiles.BranchProfile;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.source.SourceSection;
import java.util.UUID;
Expand All @@ -31,7 +31,8 @@
@ImportStatic({HostMethodCallNode.PolyglotCallType.class, HostMethodCallNode.class})
public abstract class InvokeMethodNode extends BaseNode {
private @Child InvokeFunctionNode invokeFunctionNode;
private final ConditionProfile errorProfile = ConditionProfile.createCountingProfile();
private final ConditionProfile errorReceiverProfile = ConditionProfile.createCountingProfile();
private final BranchProfile polyglotArgumentErrorProfile = BranchProfile.create();
private final int argumentCount;

/**
Expand Down Expand Up @@ -94,7 +95,7 @@ Stateful doDataflowError(
Object[] arguments,
@Cached DataflowErrorResolverNode dataflowErrorResolverNode) {
Function function = dataflowErrorResolverNode.execute(symbol, _this);
if (errorProfile.profile(function == null)) {
if (errorReceiverProfile.profile(function == null)) {
return new Stateful(state, _this);
} else {
return invokeFunctionNode.execute(function, frame, state, arguments);
Expand Down Expand Up @@ -129,12 +130,16 @@ Stateful doPolyglot(
@CachedLibrary(limit = "10") InteropLibrary interop,
@Bind("getPolyglotCallType(_this, symbol.getName(), interop)")
HostMethodCallNode.PolyglotCallType polyglotCallType,
@Cached("buildExecutors()") ThunkExecutorNode[] argExecutors,
@Cached AnyResolverNode anyResolverNode,
@Cached(value = "buildExecutors()") ThunkExecutorNode[] argExecutors,
@Cached(value = "buildProfiles()", dimensions = 1) BranchProfile[] profiles,
@Cached HostMethodCallNode hostMethodCallNode) {
Object[] args = new Object[argExecutors.length];
for (int i = 0; i < argExecutors.length; i++) {
Stateful r = argExecutors[i].executeThunk(arguments[i + 1], state, TailStatus.NOT_TAIL);
if (r.getValue() instanceof DataflowError) {
profiles[i].enter();
return r;
}
state = r.getState();
args[i] = r.getValue();
}
Expand Down Expand Up @@ -197,6 +202,14 @@ public SourceSection getSourceSection() {
return parent == null ? null : parent.getSourceSection();
}

BranchProfile[] buildProfiles() {
BranchProfile[] result = new BranchProfile[argumentCount - 1];
for (int i = 0; i < argumentCount - 1; i++) {
result[i] = BranchProfile.create();
}
return result;
}

ThunkExecutorNode[] buildExecutors() {
ThunkExecutorNode[] result = new ThunkExecutorNode[argumentCount - 1];
for (int i = 0; i < argumentCount - 1; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.DirectCallNode;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.profiles.BranchProfile;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.error.DataflowError;

/** Performs a call into a given foreign call target. */
public class ForeignMethodCallNode extends ExpressionNode {
private @Children ExpressionNode[] arguments;
private @Child DirectCallNode callNode;
private final BranchProfile[] errorProfiles;

ForeignMethodCallNode(ExpressionNode[] arguments, CallTarget foreignCt) {
this.arguments = arguments;
this.callNode = DirectCallNode.create(foreignCt);

this.errorProfiles = new BranchProfile[arguments.length];
for (int i = 0; i < arguments.length; i++) {
this.errorProfiles[i] = BranchProfile.create();
}
}

/**
Expand All @@ -33,6 +41,10 @@ public Object executeGeneric(VirtualFrame frame) {
Object[] args = new Object[arguments.length];
for (int i = 0; i < arguments.length; i++) {
args[i] = arguments[i].executeGeneric(frame);
if (args[i] instanceof DataflowError) {
errorProfiles[i].enter();
return args[i];
}
}
return callNode.call(args);
}
Expand Down
10 changes: 10 additions & 0 deletions test/Tests/src/Semantic/Error_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ from Standard.Base import all

import Standard.Test

polyglot java import java.util.Random

type My_Type foo

spec =
Expand All @@ -19,7 +21,9 @@ spec =

err_3.target.to_text.should_equal "(My_Type False)"
err_3.method_name.should_equal "nope"

Test.group "Dataflow Errors" <|

Test.specify "should be able to be shown in the default visualization" <|
json = (Error.throw <| My_Type "aaa").to_default_visualization_data
json . should_equal <| (Json.from_pairs [["foo", "aaa"], ["type", "My_Type"]]).to_text
Expand All @@ -38,8 +42,10 @@ spec =
}
]
visualization_text.should_equal expected_json.to_text

Test.specify "should implement to_display_text" <|
Error.throw Nothing . to_display_text . should_equal "Error: Nothing"

Test.specify "should be able to be mapped" <|
error = Error.throw 42
regular = 10
Expand All @@ -53,3 +59,7 @@ spec =

error.is_error . should_equal True
regular.is_error . should_equal False

Test.specify "should short-circuit polyglot evaluation" <|
error = Error.throw 42
Random.new error . should_fail_with Integer
4 changes: 4 additions & 0 deletions test/Tests/src/Semantic/Js_Interop_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,7 @@ spec = Test.group "Polyglot JS" <|
(enso_num + js_num) . should_equal 20
(js_num - enso_num) . should_equal 0

Test.specify "should propagate dataflow errors" <|
error = Error.throw 42
here.my_method error 0 . should_fail_with Integer

4 changes: 4 additions & 0 deletions test/Tests/src/Semantic/Python_Interop_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,7 @@ spec =
(enso_num + py_num) . should_equal 20
(py_num - enso_num) . should_equal 0

Test.specify "should propagate dataflow errors" <|
error = Error.throw 42
here.my_method error 0 . should_fail_with Integer

4 changes: 4 additions & 0 deletions test/Tests/src/Semantic/R_Interop_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,7 @@ spec =
(enso_num + r_num) . should_equal 20
(r_num - enso_num) . should_equal 0

Test.specify "should propagate dataflow errors" <|
error = Error.throw 42
here.my_method error 0 . should_fail_with Integer