diff --git a/CHANGELOG.md b/CHANGELOG.md index 59451bd7d65e..a32065df64bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -498,6 +498,8 @@ - [IGV can jump to JMH sources & more][4008] - [Basic support of VSCode integration][4014] - [Sync language server with file system after VCS restore][4020] +- [`ArrayOverBuffer` behaves like an `Array` and `Array.sort` no longer sorts in + place][4022] - [Introducing Meta.atom_with_hole][4023] - [Report failures in name resolution in type signatures][4030] @@ -581,6 +583,7 @@ [4008]: https://github.com/enso-org/enso/pull/4008 [4014]: https://github.com/enso-org/enso/pull/4014 [4020]: https://github.com/enso-org/enso/pull/4020 +[4022]: https://github.com/enso-org/enso/pull/4022 [4023]: https://github.com/enso-org/enso/pull/4023 [4030]: https://github.com/enso-org/enso/pull/4030 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 1eac6aff34eb..387cb58a4e37 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,20 @@ type Array length : Integer length self = @Builtin_Method "Array.length" - ## Sorts the this array in place. + ## Sorts the Array. - 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. + 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. + Getting a sorted array of numbers. - [1,2,3].to_array.sort - sort : (Any -> Any -> Ordering) -> Nothing - sort self comparator = @Builtin_Method "Array.sort" + [3,2,1].to_array.sort + sort : (Any -> Any -> Ordering) -> Array + sort self comparator=(_.compare_to _) = + self.sort_builtin comparator ## Identity. diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso index 9270b9e52107..652c929b159a 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso @@ -871,12 +871,6 @@ type Vector a [Pair 1 2, Pair -1 8].sort (_.first) (order = Sort_Direction.Descending) sort : (Any -> Any) -> (Any -> Any -> Ordering) -> Sort_Direction -> Vector Any ! Incomparable_Values sort self (on = x -> x) (by = (_.compare_to _)) (order = Sort_Direction.Ascending) = - ## Prepare the destination array that will underlie the vector. We do - not want to sort in place on the original vector, as `sort` is not - intended to be mutable. - new_vec_arr = Array.new self.length - Array.copy self.to_array 0 new_vec_arr 0 self.length - ## As we want to account for both custom projections and custom comparisons we need to construct a comparator for internal use that does both. @@ -886,7 +880,7 @@ type Vector a comp_descending Incomparable_Values.handle_errors <| - new_vec_arr.sort compare + new_vec_arr = self.to_array.sort compare Vector.from_polyglot_array new_vec_arr ## UNSTABLE 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 8d801b9195fe..6131a5b8566e 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 @@ -2,16 +2,24 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Fallback; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.InvalidArrayIndexException; +import com.oracle.truffle.api.interop.UnsupportedMessageException; +import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.LoopNode; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.profiles.BranchProfile; + import java.util.Arrays; import java.util.Comparator; + import org.enso.interpreter.dsl.BuiltinMethod; import org.enso.interpreter.node.callable.dispatch.CallOptimiserNode; import org.enso.interpreter.node.callable.dispatch.SimpleCallOptimiserNode; +import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.callable.atom.Atom; import org.enso.interpreter.runtime.callable.function.Function; @@ -19,7 +27,7 @@ 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 = "Returns a sorted array.") public abstract class SortNode extends Node { private @Child CallOptimiserNode callOptimiserNode = SimpleCallOptimiserNode.build(); private @Child InvalidComparisonNode invalidComparisonNode = InvalidComparisonNode.build(); @@ -32,15 +40,32 @@ static SortNode build() { } @Specialization - Object doSortFunction(State state, Array self, Function comparator) { + Object doArray(State state, Array self, Function comparator) { EnsoContext context = EnsoContext.get(this); - Comparator compare = getComparator(comparator, context, state); - return runSort(compare, self, context); + int size = self.getItems().length; + Object[] newArr = new Object[size]; + System.arraycopy(self.getItems(), 0, newArr, 0, size); + + return getComparatorAndSort(state, newArr, comparator, context); } - @Specialization - Object doAtomThis(State state, Atom self, Object that) { - return EnsoContext.get(this).getBuiltins().nothing(); + @Specialization(guards = "arrays.hasArrayElements(self)") + Object doPolyglotArray( + State state, + Object self, + Function comparator, + @CachedLibrary(limit = "3") InteropLibrary arrays, + @Cached HostValueToEnsoNode hostValueToEnsoNode) { + try { + long size = arrays.getArraySize(self); + Object[] newArray = new Object[(int) size]; + for (int i = 0; i < size; i++) { + newArray[i] = hostValueToEnsoNode.execute(arrays.readArrayElement(self, i)); + } + return getComparatorAndSort(state, newArray, comparator, EnsoContext.get(this)); + } catch (UnsupportedMessageException | InvalidArrayIndexException e) { + throw new IllegalStateException(e); + } } @Fallback @@ -52,10 +77,17 @@ 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()); - return context.getBuiltins().nothing(); + private Object getComparatorAndSort( + State state, Object[] rawItems, Function comparator, EnsoContext context) { + Comparator compare = new SortComparator(comparator, context, this, state); + runSort(compare, rawItems); + return new Array(rawItems); + } + + Object[] runSort(Comparator compare, Object[] self) { + doSort(self, compare); + LoopNode.reportLoopCount(this, self.length); + return self; } @TruffleBoundary @@ -63,10 +95,6 @@ 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 class SortComparator implements Comparator { private final Function compFn; private final EnsoContext context; 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 04468e59c3b8..96c7dfb2d4d4 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,6 +1,8 @@ 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.library.ExportLibrary; import com.oracle.truffle.api.library.ExportMessage; @@ -16,8 +18,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 +44,10 @@ long getArraySize() { public static ArrayOverBuffer wrapBuffer(ByteBuffer buffer) { return new ArrayOverBuffer(buffer); } + + @ExportMessage + String toDisplayString(boolean allowSideEffects) { + final InteropLibrary iop = InteropLibrary.getUncached(); + return DisplayArrayUtils.toDisplayString(this, allowSideEffects, iop); + } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/DisplayArrayUtils.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/DisplayArrayUtils.java new file mode 100644 index 000000000000..4763b60468d7 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/DisplayArrayUtils.java @@ -0,0 +1,52 @@ +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.UnsupportedMessageException; + +import java.io.PrintWriter; +import java.io.StringWriter; + +public class DisplayArrayUtils { + + @CompilerDirectives.TruffleBoundary + public static String toDisplayString( + Object arrayLike, boolean allowSideEffects, InteropLibrary iop) { + StringBuilder sb = new StringBuilder(); + try { + sb.append('['); + String sep = ""; + long len = iop.getArraySize(arrayLike); + for (long i = 0; i < len; i++) { + sb.append(sep); + + Object at = iop.readArrayElement(arrayLike, 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); + } + return sb.toString(); + } + + private static 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); + } + } +} diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Vector.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Vector.java index 3bc1b28e21a9..8b20f4b8bcf3 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Vector.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Vector.java @@ -11,8 +11,6 @@ import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.library.ExportLibrary; import com.oracle.truffle.api.library.ExportMessage; -import java.io.PrintWriter; -import java.io.StringWriter; import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.Builtin; @@ -188,32 +186,7 @@ final void removeArrayElement(long index) throws UnsupportedMessageException { @CompilerDirectives.TruffleBoundary String toDisplayString(boolean allowSideEffects) { final InteropLibrary iop = InteropLibrary.getUncached(); - StringBuilder sb = new StringBuilder(); - try { - sb.append('['); - String sep = ""; - long len = length(iop); - for (long i = 0; i < len; i++) { - sb.append(sep); - - Object at = - readArrayElement( - i, iop, WarningsLibrary.getUncached(), HostValueToEnsoNode.getUncached()); - 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(); + return DisplayArrayUtils.toDisplayString(this, allowSideEffects, iop); } @ExportMessage @@ -271,16 +244,4 @@ public String toString() { private static E raise(Class clazz, Throwable t) throws E { throw (E) t; } - - @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); - } - } } diff --git a/test/Tests/src/Data/Array_Spec.enso b/test/Tests/src/Data/Array_Spec.enso index 180103c0e1b0..4d18d5cd7720 100644 --- a/test/Tests/src/Data/Array_Spec.enso +++ b/test/Tests/src/Data/Array_Spec.enso @@ -43,4 +43,28 @@ spec = res = Array.new err res . should_fail_with Illegal_State.Error + Test.specify "should not sort in place" <| + arr = make_enso_array [3, 1, 2] + new_arr = arr.sort + arr . should_equal [3, 1, 2] + new_arr . should_equal [1, 2, 3] + + Test.group "ArrayOverBuffer" <| + location_pending = case Platform.os of + Platform.OS.Windows -> "This test is disabled on Windows." + _ -> Nothing + + Test.specify "should behave like an Array" pending=location_pending <| + 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]" + sorted = array_over_buffer.sort + array_over_buffer.to_text . should_equal "[32, 106, 117, 106, 117, 98, 101, 115, 46, 10]" + sorted.to_text . should_equal "[10, 32, 46, 98, 101, 106, 106, 115, 117, 117]" + + main = Test_Suite.run_main spec diff --git a/test/Tests/src/Semantic/Js_Interop_Spec.enso b/test/Tests/src/Semantic/Js_Interop_Spec.enso index ce9ada504a79..798e05b28f89 100644 --- a/test/Tests/src/Semantic/Js_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Js_Interop_Spec.enso @@ -47,7 +47,7 @@ foreign js make_array = """ return [{ x: 10}, {x: 20}, {x: 30}]; foreign js make_simple_array = """ - return [10, 20, 30]; + return [30, 10, 20]; foreign js make_str str = """ return "foo " + str + " bar" @@ -100,7 +100,16 @@ spec = Test.group "Polyglot JS" <| vec = Vector.from_polyglot_array make_array vec.map .x . should_equal [10, 20, 30] vec2 = Vector.from_polyglot_array make_simple_array - vec2.to_array.at 0 . should_equal 10 + vec2.to_array.at 0 . should_equal 30 + arr = make_simple_array + new_arr = arr.sort + arr . should_equal [30, 10, 20] + new_arr . should_equal [10, 20, 30] + + arr_2 = make_simple_array + sorted_2 = arr_2.sort + arr_2 . should_equal [30, 10, 20] + sorted_2 . should_equal [10, 20, 30] Test.specify "should correctly marshall strings" <| str = make_str "x" + " baz" diff --git a/test/Tests/src/Semantic/Python_Interop_Spec.enso b/test/Tests/src/Semantic/Python_Interop_Spec.enso index 9f0cf7c214e5..d3ad1b1570c5 100644 --- a/test/Tests/src/Semantic/Python_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Python_Interop_Spec.enso @@ -45,7 +45,10 @@ foreign python make_array = """ self.x = x def compare(self, guess): return self.x < guess - return [My(10), My(20), My(30)] + return [My(30), My(10), My(20)] + +foreign python make_num_array = """ + return [30, 10, 20] foreign python make_str str = """ return ("foo " + str + " bar") @@ -87,7 +90,18 @@ spec = Test.specify "should expose array interfaces for Python arrays" <| vec = Vector.from_polyglot_array make_array - vec.map .x . should_equal [10, 20, 30] + vec.map .x . should_equal [30, 10, 20] + + arr = vec.map .x . to_array + sorted = arr.sort + arr . should_equal [30, 10, 20] + sorted . should_equal [10, 20, 30] + + arr_2 = make_num_array + sorted_2 = arr_2.sort + arr_2 . should_equal [30, 10, 20] + sorted_2 . should_equal [10, 20, 30] + Test.specify "should correctly marshall strings" <| str = make_str "x" + " baz" diff --git a/test/Tests/src/Semantic/R_Interop_Spec.enso b/test/Tests/src/Semantic/R_Interop_Spec.enso index 90bc2807af60..28d721d2214c 100644 --- a/test/Tests/src/Semantic/R_Interop_Spec.enso +++ b/test/Tests/src/Semantic/R_Interop_Spec.enso @@ -31,7 +31,10 @@ foreign r make_object = """ list(x=x, y=FALSE, compare=function(guess) x < guess) foreign r make_array = """ - list(list(x=10), list(x=20), list(x=30)) + list(list(x=30), list(x=10), list(x=20)) + +foreign r make_num_array = """ + list(30, 10, 20) foreign r make_str str = """ paste("foo", str, "bar", sep=" ") @@ -73,7 +76,17 @@ spec = Test.specify "should expose array interfaces for R arrays" <| vec = Vector.from_polyglot_array make_array - vec.map .x . should_equal [10, 20, 30] + vec.map .x . should_equal [30, 10, 20] + + arr = vec.map .x . to_array + sorted = arr.sort + arr . should_equal [30, 10, 20] + sorted . should_equal [10, 20, 30] + + arr_2 = make_num_array + sorted_2 = arr_2.sort + arr_2 . should_equal [30, 10, 20] + sorted_2 . should_equal [10, 20, 30] Test.specify "should correctly marshall strings" <| str = make_str "x" + " baz"