From 64ad116a94be48ee95d7cf6e40bdc3fcf6878e08 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 4 Jan 2023 18:31:32 +0100 Subject: [PATCH] Make ArrayOverBuffer behave like an Array Most of the problems with accessing `ArrayOverBuffer` have been resolved by using `CoerceArrayNode` (https://github.com/enso-org/enso/pull/3817). In `Array.sort` we still however specialized on Array which wasn't compatible with `ArrayOverBuffer`. Added a specialization to `Array.sort` to deal with that case. Because one cannot use a custom comparator with primitive (Java) arrays there is a conversion needed. It's a necessary penalty if we want to keep `ArrayOverBuffer` around. Also fixed an example in `Array.enso` by providing a default argument. --- .../Base/0.0.0-dev/src/Data/Array.enso | 14 +++-- .../expression/builtin/mutable/SortNode.java | 54 +++++++++++++--- .../runtime/data/ArrayOverBuffer.java | 61 ++++++++++++++++++- test/Tests/src/Data/Array_Spec.enso | 18 ++++++ 4 files changed, 131 insertions(+), 16 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso index 1eac6aff34ebe..6a058c388b474 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso @@ -134,19 +134,21 @@ type Array length : Integer length self = @Builtin_Method "Array.length" - ## Sorts the this array in place. + ## ADVANCED - Arguments: - - comparator: A comparison function that takes two elements and returns - an Ordering that describes how the first element is ordered with - respect to the second. + Sorts this array in place. + + Arguments: + - comparator: A comparison function that takes two elements and returns + an Ordering that describes how the first element is ordered with + respect to the second. > Example Sorting an array of numbers. [1,2,3].to_array.sort sort : (Any -> Any -> Ordering) -> Nothing - sort self comparator = @Builtin_Method "Array.sort" + sort self comparator=(_.compare_to _) = self.sort_builtin comparator ## Identity. diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/SortNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/SortNode.java index 8d801b9195fe1..c339f5bdeddfd 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/SortNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/SortNode.java @@ -9,20 +9,28 @@ import com.oracle.truffle.api.profiles.BranchProfile; import java.util.Arrays; import java.util.Comparator; + +import org.apache.commons.lang3.ArrayUtils; import org.enso.interpreter.dsl.BuiltinMethod; +import org.enso.interpreter.epb.node.CoercePrimitiveNode; import org.enso.interpreter.node.callable.dispatch.CallOptimiserNode; import org.enso.interpreter.node.callable.dispatch.SimpleCallOptimiserNode; import org.enso.interpreter.runtime.EnsoContext; 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.data.ArrayOverBuffer; 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.") +@BuiltinMethod( + type = "Array", + name = "sort_builtin", + description = "Sorts a mutable array in place.") public abstract class SortNode extends Node { private @Child CallOptimiserNode callOptimiserNode = SimpleCallOptimiserNode.build(); private @Child InvalidComparisonNode invalidComparisonNode = InvalidComparisonNode.build(); + private @Child CoercePrimitiveNode coercePrimitiveNode = CoercePrimitiveNode.build(); private final BranchProfile resultProfile = BranchProfile.create(); abstract Object execute(State state, Object self, Object comparator); @@ -34,8 +42,8 @@ static SortNode build() { @Specialization Object doSortFunction(State state, Array self, Function comparator) { EnsoContext context = EnsoContext.get(this); - Comparator compare = getComparator(comparator, context, state); - return runSort(compare, self, context); + Comparator compare = getComparator(comparator, context, state, false); + return runSort(compare, self.getItems(), context); } @Specialization @@ -43,6 +51,13 @@ Object doAtomThis(State state, Atom self, Object that) { return EnsoContext.get(this).getBuiltins().nothing(); } + @Specialization + Object doArrayOverBuffer(State state, ArrayOverBuffer self, Function comparator) { + EnsoContext context = EnsoContext.get(this); + Comparator compare = getComparator(comparator, context, state, true); + return runSort(compare, self.backingArray(), context); + } + @Fallback Object doOther(State state, Object self, Object comparator) { CompilerDirectives.transferToInterpreter(); @@ -52,19 +67,30 @@ Object doOther(State state, Object self, Object comparator) { this); } - Object runSort(Comparator compare, Array self, EnsoContext context) { - doSort(self.getItems(), compare); - LoopNode.reportLoopCount(this, (int) self.length()); + Object runSort(Comparator compare, Object[] self, EnsoContext context) { + doSort(self, compare); + LoopNode.reportLoopCount(this, self.length); return context.getBuiltins().nothing(); } + Object runSort(Comparator compare, byte[] self, EnsoContext context) { + Byte[] tmpArray = ArrayUtils.toObject(self); + Object result = runSort(compare, tmpArray, context); + byte[] primitiveTmpArray = ArrayUtils.toPrimitive(tmpArray); + System.arraycopy(primitiveTmpArray, 0, self, 0, self.length); + return result; + } + @TruffleBoundary void doSort(Object[] items, Comparator compare) { Arrays.sort(items, compare); } - private SortComparator getComparator(Function comp, EnsoContext context, State state) { - return new SortComparator(comp, context, this, state); + private SortComparator getComparator( + Function comp, EnsoContext context, State state, boolean coerce) { + return coerce + ? new CoerceSortComparator(comp, context, this, state) + : new SortComparator(comp, context, this, state); } private class SortComparator implements Comparator { @@ -105,4 +131,16 @@ private int convertResult(Object res) { } } } + + private class CoerceSortComparator extends SortComparator { + + CoerceSortComparator(Function compFn, EnsoContext context, SortNode outerThis, State state) { + super(compFn, context, outerThis, state); + } + + @Override + public int compare(Object o1, Object o2) { + return super.compare(coercePrimitiveNode.execute(o1), coercePrimitiveNode.execute(o2)); + } + } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ArrayOverBuffer.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ArrayOverBuffer.java index 04468e59c3b80..13d6e96f55b27 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ArrayOverBuffer.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ArrayOverBuffer.java @@ -1,10 +1,18 @@ package org.enso.interpreter.runtime.data; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.InvalidArrayIndexException; import com.oracle.truffle.api.interop.TruffleObject; +import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.library.ExportLibrary; import com.oracle.truffle.api.library.ExportMessage; +import org.enso.interpreter.node.expression.builtin.error.InvalidArrayIndex; +import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; +import org.enso.interpreter.runtime.error.WarningsLibrary; +import java.io.PrintWriter; +import java.io.StringWriter; import java.nio.ByteBuffer; @ExportLibrary(InteropLibrary.class) @@ -16,8 +24,12 @@ private ArrayOverBuffer(ByteBuffer buffer) { } @ExportMessage - Object readArrayElement(long index) { - return (long) buffer.get(buffer.position() + Math.toIntExact(index)); + Object readArrayElement(long index) throws InvalidArrayIndexException { + try { + return (long) buffer.get(buffer.position() + Math.toIntExact(index)); + } catch (IndexOutOfBoundsException e) { + throw InvalidArrayIndexException.create(index); + } } @ExportMessage @@ -38,4 +50,49 @@ long getArraySize() { public static ArrayOverBuffer wrapBuffer(ByteBuffer buffer) { return new ArrayOverBuffer(buffer); } + + @ExportMessage + String toDisplayString(boolean allowSideEffects) { + final InteropLibrary iop = InteropLibrary.getUncached(); + StringBuilder sb = new StringBuilder(); + try { + sb.append('['); + String sep = ""; + long len = getArraySize(); + for (long i = 0; i < len; i++) { + sb.append(sep); + + Object at = readArrayElement(i); + Object str = showObject(iop, allowSideEffects, at); + if (iop.isString(str)) { + sb.append(iop.asString(str)); + } else { + sb.append("_"); + } + sep = ", "; + } + sb.append(']'); + } catch (InvalidArrayIndexException | UnsupportedMessageException ex) { + StringWriter w = new StringWriter(); + ex.printStackTrace(new PrintWriter(w)); + sb.append("...\n").append(w.toString()); + } + return sb.toString(); + } + + @CompilerDirectives.TruffleBoundary + private Object showObject(InteropLibrary iop, boolean allowSideEffects, Object child) + throws UnsupportedMessageException { + if (child == null) { + return "null"; + } else if (child instanceof Boolean) { + return (boolean) child ? "True" : "False"; + } else { + return iop.toDisplayString(child, allowSideEffects); + } + } + + public byte[] backingArray() { + return buffer.array(); + } } diff --git a/test/Tests/src/Data/Array_Spec.enso b/test/Tests/src/Data/Array_Spec.enso index 180103c0e1b02..2d875ad758bd7 100644 --- a/test/Tests/src/Data/Array_Spec.enso +++ b/test/Tests/src/Data/Array_Spec.enso @@ -43,4 +43,22 @@ spec = res = Array.new err res . should_fail_with Illegal_State.Error + Test.specify "should sort in place" <| + arr = make_enso_array [3, 1, 2] + arr.sort + arr . should_equal [1, 2, 3] + + Test.group "ArrayOverBuffer" <| + Test.specify "should behave like an Array" <| + array_over_buffer = (File.new (enso_project.data / "sample.txt") . read_last_bytes 10).to_array + + case array_over_buffer of + _ : Array -> Nothing + _ -> Test.fail "Expected ArrayOverBuffer to match on Array type" + + array_over_buffer.to_text . should_equal "[32, 106, 117, 106, 117, 98, 101, 115, 46, 10]" + array_over_buffer.sort + array_over_buffer.to_text . should_equal "[10, 32, 46, 98, 101, 106, 106, 115, 117, 117]" + + main = Test_Suite.run_main spec