Skip to content

Commit

Permalink
Cleanup dataflow error propagation
Browse files Browse the repository at this point in the history
Rather than going through instance checks, introduced a separate node
that uses specializations for that.
In the process, discovered that ComparatorNode was never used or covered
on our test suite. If it did, we would notice that we were comparing
Atoms with AtomConstructors, meaning that we would always throw a panic
when sort is given a comparator. It made more sense to remove that
specialization.
  • Loading branch information
hubertp committed Nov 16, 2022
1 parent bfb914e commit 0da8b95
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 75 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.enso.interpreter.node.expression.builtin.mutable;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.nodes.NodeInfo;
import org.enso.interpreter.runtime.Context;
import org.enso.interpreter.runtime.error.DataflowError;
import org.enso.interpreter.runtime.error.PanicException;

@NodeInfo(
shortName = "invalidComparison",
description = "Propagates the result of the invalid comparison via a Panic.")
public abstract class InvalidComparisonNode extends Node {

public abstract int execute(Object result);

public static InvalidComparisonNode build() {
return InvalidComparisonNodeGen.create();
}

@Specialization
int doDataFlowError(DataflowError dataflowError) {
System.out.println("DoDataflowc " + dataflowError.getPayload());
CompilerDirectives.transferToInterpreter();
throw new PanicException(dataflowError.getPayload(), dataflowError.getLocation());
}

@Specialization
int doPanic(Object result) {
CompilerDirectives.transferToInterpreter();
var ordering = Context.get(this).getBuiltins().ordering().getType();
throw new PanicException(
Context.get(this).getBuiltins().error().makeTypeError(ordering, result, "result"), this);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.enso.interpreter.node.expression.builtin.mutable;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
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.LoopNode;
Expand All @@ -15,38 +17,39 @@
import org.enso.interpreter.runtime.callable.atom.Atom;
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.error.DataflowError;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.state.State;

@BuiltinMethod(type = "Array", name = "sort", description = "Sorts a mutable array in place.")
public abstract class SortNode extends Node {
private @Child ComparatorNode comparatorNode = ComparatorNode.build();
private @Child CallOptimiserNode callOptimiserNode = SimpleCallOptimiserNode.build();
private @Child InvalidComparisonNode invalidComparisonNode = InvalidComparisonNode.build();
private final BranchProfile resultProfile = BranchProfile.create();

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

static SortNode build() {
return SortNodeGen.create();
}

@Specialization
Object doSortFunction(VirtualFrame frame, State state, Array self, Function comparator) {
Object doSortFunction(State state, Array self, Function comparator) {
Context context = Context.get(this);
Comparator<Object> compare = getComparator(comparator, context, state);
return runSort(compare, self, context);
}

@Specialization
Object doSortCallable(VirtualFrame frame, State state, Array self, Object comparator) {
Comparator<Object> compare = (l, r) -> comparatorNode.execute(frame, comparator, state, l, r);
return runSort(compare, self, Context.get(this));
Object doAtomThis(State state, Atom self, Object that) {
return Context.get(this).getBuiltins().nothing();
}

@Specialization
Object doAtomThis(VirtualFrame frame, State state, Atom self, Object that) {
return Context.get(this).getBuiltins().nothing();
@Fallback
Object doOther(State state, Object self, Object comparator) {
CompilerDirectives.transferToInterpreter();
var fun = Context.get(this).getBuiltins().function();
throw new PanicException(
Context.get(this).getBuiltins().error().makeTypeError(fun, comparator, "comparator"), this);
}

Object runSort(Comparator<Object> compare, Array self, Context context) {
Expand Down Expand Up @@ -98,13 +101,7 @@ private int convertResult(Object res) {
return 1;
} else {
resultProfile.enter();
if (res instanceof DataflowError error) {
throw new PanicException(error.getPayload(), error.getLocation());
}

var ordering = context.getBuiltins().ordering().getType();
throw new PanicException(
context.getBuiltins().error().makeTypeError(ordering, res, "result"), outerThis);
return invalidComparisonNode.execute(res);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test/Tests/src/Data/Vector_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ spec = Test.group "Vectors" <|
[2, Date.new 1999].sort . should_fail_with Incomparable_Values_Error
[Date_Time.new 1999 1 1 12 30, Date.new 1999].sort . should_fail_with Incomparable_Values_Error
[Date_Time.new 1999 1 1 12 30, Time_Of_Day.new 12 30].sort . should_fail_with Incomparable_Values_Error
Test.expect_panic_with ([3,2,1].to_array.sort 42) Type_Error_Data

Test.specify "should leave the original vector unchanged" <|
non_empty_vec = [2, 4, 2, 3, 2, 3]
Expand Down

0 comments on commit 0da8b95

Please sign in to comment.