diff --git a/RELEASES.md b/RELEASES.md index 56528a2798d5..3296cabcc8f9 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -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 diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/IndirectInvokeMethodNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/IndirectInvokeMethodNode.java index 23c109c5e27a..d399413aad1b 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/IndirectInvokeMethodNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/IndirectInvokeMethodNode.java @@ -153,13 +153,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(); } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java index 617b045226ea..abe121ff4563 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java @@ -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; @@ -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; @@ -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; /** @@ -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); @@ -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(); } @@ -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++) { diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/foreign/ForeignMethodCallNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/foreign/ForeignMethodCallNode.java index 786bd74325b4..dc7b6b65f4a4 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/foreign/ForeignMethodCallNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/foreign/ForeignMethodCallNode.java @@ -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(); + } } /** @@ -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); } diff --git a/test/Tests/src/Semantic/Error_Spec.enso b/test/Tests/src/Semantic/Error_Spec.enso index 41d1fd445574..c27c2130ddcc 100644 --- a/test/Tests/src/Semantic/Error_Spec.enso +++ b/test/Tests/src/Semantic/Error_Spec.enso @@ -2,6 +2,8 @@ from Standard.Base import all import Standard.Test +polyglot java import java.util.Random + type My_Type foo spec = @@ -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 @@ -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 @@ -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 diff --git a/test/Tests/src/Semantic/Js_Interop_Spec.enso b/test/Tests/src/Semantic/Js_Interop_Spec.enso index 41281679b4a3..7109c6b3284f 100644 --- a/test/Tests/src/Semantic/Js_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Js_Interop_Spec.enso @@ -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 + diff --git a/test/Tests/src/Semantic/Python_Interop_Spec.enso b/test/Tests/src/Semantic/Python_Interop_Spec.enso index 873d996cf865..8b477c1a3e1c 100644 --- a/test/Tests/src/Semantic/Python_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Python_Interop_Spec.enso @@ -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 + diff --git a/test/Tests/src/Semantic/R_Interop_Spec.enso b/test/Tests/src/Semantic/R_Interop_Spec.enso index 555fdddc3e81..719a7a1003f8 100644 --- a/test/Tests/src/Semantic/R_Interop_Spec.enso +++ b/test/Tests/src/Semantic/R_Interop_Spec.enso @@ -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 +