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

Promote broken values instead of ignoring them #11777

Merged
merged 30 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1c2d007
Currently a DataflowError may get lost in a middle of block statements
JaroslavTulach Dec 5, 2024
49c628e
Propagate Error ASAP instead of ignoring it
JaroslavTulach Dec 5, 2024
91f7497
check for dataflow error in should_equal rhs and report it - this is …
radeusgd Dec 5, 2024
1ed8505
add error check in remaining should_equal overrides
radeusgd Dec 5, 2024
7373304
align behaviour of find across range/vector
radeusgd Dec 6, 2024
00516f5
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach Dec 9, 2024
b0f71f8
Fix Decimal test and `Array_Like_Helpers.find` (#11883)
GregoryTravis Dec 17, 2024
ee7c81e
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach Dec 17, 2024
d703496
Space before dot
JaroslavTulach Dec 17, 2024
7901a25
fix mistake in should_equal check
radeusgd Dec 17, 2024
ba16c47
fix frame offsets
radeusgd Dec 17, 2024
6e0928a
fixing more offsets
radeusgd Dec 17, 2024
bed6609
fix invalid tests
radeusgd Dec 17, 2024
1647c53
remove unnecessary Not_Found type (clashing with another existing one)
radeusgd Dec 17, 2024
ef80339
remove check that was actually too strict
radeusgd Dec 17, 2024
a8571c4
fix cloud test cleanup
radeusgd Dec 17, 2024
6551756
fix a test which was propagating error and crashing suite builder
radeusgd Dec 17, 2024
09fb17f
fixing other tests that break Table Tests progress
radeusgd Dec 17, 2024
3b8c089
fixing more issues in Table tests
radeusgd Dec 17, 2024
434ba8a
enable Context to fix lazy init issue
radeusgd Dec 17, 2024
777c38b
Merging with latest develop
JaroslavTulach Dec 18, 2024
8748349
Speculate on statements returning Nothing
JaroslavTulach Dec 18, 2024
b7430cb
Code style issues found in CHANGELOG.md file
JaroslavTulach Dec 18, 2024
fe5b9e1
duplicate rhs_error_check and make it imperative to avoid nesting
radeusgd Dec 18, 2024
3ce19c7
fix offsets and finally add tests for this
radeusgd Dec 18, 2024
afedc71
Documenting propagation of _broken values_
JaroslavTulach Dec 19, 2024
bda76cf
Renaming to Errors & Panics
JaroslavTulach Dec 19, 2024
0130601
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach Dec 19, 2024
5916204
Reorganizing changelog
JaroslavTulach Dec 19, 2024
5bfe082
Merge branch 'develop' into wip/jtulach/PropagateDataflowError5430
mergify[bot] Dec 19, 2024
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# Next Next Release

#### Enso Language & Runtime

- [Propagate Error ASAP instead of ignoring it][11777].

[11777]: https://github.com/enso-org/enso/pull/11777

# Next Release

#### Enso IDE
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.enso.interpreter.test.semantic;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

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

public class DataflowErrorPropagationTest {
private static Context ctx;
private static Value suppressError;
private static Value suppressErrorWithAssign;

@BeforeClass
public static void prepareCtx() {
ctx = ContextUtils.createDefaultContext();
var code =
"""
from Standard.Base import all

private yield_error yes:Boolean -> Text =
if yes then Error.throw "Yielding an error" else
"OK"

suppress_error yes:Boolean value =
yield_error yes
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
value

suppress_error_with_assign yes:Boolean value =
_ = yield_error yes
value
""";
suppressError =
ctx.eval("enso", code).invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error");
suppressErrorWithAssign =
ctx.eval("enso", code)
.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error_with_assign");
}

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

@Test
public void noErrorReturnValue() {
var value = suppressError.execute(false, 42);
assertTrue("It is a number", value.isNumber());
assertEquals(42, value.asInt());
}

@Test
public void propagateErrorImmediatelly() {
var value = suppressError.execute(true, 42);
assertFalse("It is not a number", value.isNumber());
assertTrue("It is an error", value.isException());
try {
throw value.throwException();
} catch (PolyglotException ex) {
assertEquals("Yielding an error", ex.getMessage());
}
}

@Test
public void noErrorReturnValueWithAssign() {
var value = suppressErrorWithAssign.execute(false, 42);
assertTrue("It is a number", value.isNumber());
assertEquals(42, value.asInt());
}

@Test
public void errorIsAssignedAndThatIsEnoughReturnValue() {
var value = suppressErrorWithAssign.execute(true, 42);
assertTrue("It is a number", value.isNumber());
assertFalse("Not an error", value.isException());
assertEquals(42, value.asInt());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import com.oracle.truffle.api.source.SourceSection;
import java.util.Set;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.EnsoContext;
import org.enso.interpreter.runtime.error.DataflowError;

/**
* This node defines the body of a function for execution, as well as the protocol for executing the
Expand Down Expand Up @@ -55,8 +57,15 @@ public static BlockNode buildSilent(ExpressionNode[] expressions, ExpressionNode
@Override
@ExplodeLoop
public Object executeGeneric(VirtualFrame frame) {
var ctx = EnsoContext.get(this);
var nothing = ctx.getBuiltins().nothing();
for (ExpressionNode statement : statements) {
statement.executeGeneric(frame);
var result = statement.executeGeneric(frame);
if (result != nothing) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not get rid of this if statement and use simply just if (result instanceof DataflowError err). If it is an instance of DataflowError, it cannot be nothing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this seems redundant, can we remove it, or is there some reason that we don't see here?

Copy link
Member Author

@JaroslavTulach JaroslavTulach Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is the other part of #5430:

We don't need to solve it in this PR. Just my 2 Kč explanation of the != Nothing check. In any case, that's for another PR.

if (result instanceof DataflowError err) {
return err;
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return returnExpr.executeGeneric(frame);
}
Expand Down
Loading