Unify {Vector|Array}.at, Vector.unsafe_at begone

Vector.at and Array.at should both support negative indices and both
return a dataflow error on invalid index.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso

@@ -7,20 +7,23 @@ import Standard.Base.Meta
 ## The type of primitive mutable arrays.
 @Builtin_Type
 type Array
-    ## Gets the element at index in the array this.
+    ## Gets an element from the array at a specified index (0-based).
 
        Arguments:
-       - index: The index to get the element from.
-
-       ? Safety
-         If index < 0 or index >= self.length, then this operation will result
-         in an Invalid_Array_Index_Error exception.
+       - index: The index to get the element from. The index is
+         also allowed be negative, then the elements are indexed from the back
+         of the array, i.e. -1 will correspond to the last element.
 
        > Example
          Get the element at index 1.
 
              [1,2,3].to_array.at 1
-    at : Integer -> Any
+
+       > Example
+         Get the last element of an array.
+
+             [1,2,3].to_array.at -1
+    at : Integer -> Any ! Index_Out_Of_Bounds_Error
     at self index = @Builtin_Method "Array.at"
 
     ## Gets the length of the array this.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso

@@ -146,19 +146,7 @@ type Vector a
 
              [1, 2, 3].at -1 == 3
     at : Integer -> Any ! Index_Out_Of_Bounds_Error
-    at self index =
-        Panic.catch Invalid_Array_Index_Error_Data (self.at_builtin index) _->
-            Error.throw (Index_Out_Of_Bounds_Error_Data index self.length)
-
-    ## ADVANCED
-       UNSTABLE
-
-       An unsafe variant of the `at` operation. It only allows non-negative
-       indices and will panic with a raw Java exception on out-of-bounds access.
-       Thus it should only be used when the access is guaranteed to be within
-       bounds or with additional error handling.
-    unsafe_at : Integer -> Any
-    unsafe_at self index = @Builtin_Method "Vector.unsafe_at"
+    at self index = @Builtin_Method "Vector.at"
 
     ## Combines all the elements of the vector, by iteratively applying the
        passed function with next elements of the vector.
@@ -178,7 +166,7 @@ type Vector a
              [0, 1, 2] . fold 0 (+)
     fold : Any -> (Any -> Any -> Any) -> Any
     fold self init function =
-        f = acc -> ix -> function acc (self.unsafe_at ix)
+        f = acc -> ix -> function acc (self.at ix)
         0.up_to self.length . fold init f
 
     ## Combines all the elements of the vector, by iteratively applying the
@@ -212,9 +200,9 @@ type Vector a
     reduce : (Any -> Any -> Any) -> Any ! Empty_Error
     reduce self function =
         case self.not_empty of
-            True -> if self.length == 1 then self.unsafe_at 0 else
+            True -> if self.length == 1 then self.at 0 else
                 f = acc -> ix -> function acc (self.at ix)
-                1.up_to self.length . fold (self.unsafe_at 0) f
+                1.up_to self.length . fold (self.at 0) f
             False -> Error.throw Empty_Error
 
     ## Computes the sum of the values in the vector.
@@ -247,7 +235,7 @@ type Vector a
              [1, 2, 3, 4, 5].exists (> 3)
     exists : (Any -> Boolean) -> Boolean
     exists self predicate =
-        0.up_to self.length . exists (idx -> (predicate (self.unsafe_at idx)))
+        0.up_to self.length . exists (idx -> (predicate (self.at idx)))
 
     ## Returns the first element of the vector that satisfies the predicate or
        if no elements of the vector satisfy the predicate, it throws nothing.
@@ -266,7 +254,7 @@ type Vector a
         len = self.length
         go idx =
             if (idx >= len) then Error.throw Nothing else
-                elem = self.unsafe_at idx
+                elem = self.at idx
                 if predicate elem then elem else
                     @Tail_Call go idx+1
         go 0
@@ -434,7 +422,7 @@ type Vector a
              [1, 2, 3] . map +1
     map : (Any -> Any) -> Vector Any
     map self function =
-        new self.length i-> function (self.unsafe_at i)
+        new self.length i-> function (self.at i)
 
     ## Applies a function to each element of the vector, returning the vector
        that contains all results concatenated.
@@ -482,7 +470,7 @@ type Vector a
 
              [1, 2, 3].map_with_index (+)
     map_with_index : (Integer -> Any -> Any) -> Vector Any
-    map_with_index self function = new self.length i-> function i (self.unsafe_at i)
+    map_with_index self function = new self.length i-> function i (self.at i)
 
     ## Applies a function to each element of the vector.
 
@@ -499,7 +487,7 @@ type Vector a
     each : (Any -> Any) -> Nothing
     each self f =
         0.up_to self.length . each ix->
-            f (self.unsafe_at ix)
+            f (self.at ix)
 
     ## Applies a function to each element of the vector.
 
@@ -519,7 +507,7 @@ type Vector a
     each_with_index : (Integer -> Any -> Any) -> Nothing
     each_with_index self f =
         0.up_to self.length . each ix->
-            f ix (self.unsafe_at ix)
+            f ix (self.at ix)
 
     ## Reverses the vector, returning a vector with the same elements, but in
        the opposite order.
@@ -529,7 +517,7 @@ type Vector a
 
              [1, 2].reverse
     reverse : Vector Any
-    reverse self = new self.length (i -> self.unsafe_at (self.length - (1 + i)))
+    reverse self = new self.length (i -> self.at (self.length - (1 + i)))
 
     ## Generates a human-readable text representation of the vector.
 
@@ -578,7 +566,7 @@ type Vector a
     == : Vector -> Boolean
     == self that = case that of
         _ : Vector ->
-            eq_at i = self.unsafe_at i == that.unsafe_at i
+            eq_at i = self.at i == that.at i
             if self.length == that.length then 0.up_to self.length . all eq_at else False
         _ : Array ->
             eq_at i = self.at i == that.at i
@@ -642,8 +630,8 @@ type Vector a
     join : Text -> Text -> Text -> Text
     join self separator="" prefix="" suffix="" =
         if self.is_empty then prefix+suffix else
-            if self.length == 1 then prefix + self.unsafe_at 0 + suffix else
-                prefix + self.unsafe_at 0 + (1.up_to self.length . fold "" acc-> i-> acc + separator + self.unsafe_at i) + suffix
+            if self.length == 1 then prefix + self.at 0 + suffix else
+                prefix + self.at 0 + (1.up_to self.length . fold "" acc-> i-> acc + separator + self.at i) + suffix
 
     ## PRIVATE
        Creates a new vector with the skipping elements until `start` and then
@@ -715,7 +703,7 @@ type Vector a
     zip : Vector Any -> (Any -> Any -> Any) -> Vector Any
     zip self that function=[_,_] =
         len = Math.min self.length that.length
-        new len i-> function (self.unsafe_at i) (that.unsafe_at i)
+        new len i-> function (self.at i) (that.at i)
 
     ## Extend `self` vector to the length of `n` appending elements `elem` to
        the end.
@@ -758,7 +746,7 @@ type Vector a
 
              [1, 2, 3, 4].head
     head : Any ! Empty_Error
-    head self = if self.length >= 1 then self.unsafe_at 0 else Error.throw Empty_Error
+    head self = if self.length >= 1 then self.at 0 else Error.throw Empty_Error
 
     ## Get all elements in the vector except the first.
 
@@ -787,7 +775,7 @@ type Vector a
 
              [1, 2, 3, 4].last
     last : Vector ! Empty_Error
-    last self = if self.length >= 1 then self.unsafe_at (self.length-1) else
+    last self = if self.length >= 1 then self.at (self.length-1) else
         Error.throw Empty_Error
 
     ## Get the first element from the vector, or an `Empty_Error` if the vector
@@ -1132,7 +1120,7 @@ slice_ranges vector ranges =
     if ranges.length == 0 then [] else
         if ranges.length != 1 then slice_many_ranges vector ranges else
             case ranges.first of
-                _ : Integer -> [vector.unsafe_at ranges.first]
+                _ : Integer -> [vector.at ranges.first]
                 Range_Data start end step -> case step == 1 of
                     True -> vector.slice start end
                     False -> slice_many_ranges vector ranges
@@ -1146,11 +1134,11 @@ slice_many_ranges vector ranges =
     builder = new_builder new_length
     ranges.each descriptor-> case descriptor of
         _ : Integer ->
-            builder.append (vector.unsafe_at descriptor)
+            builder.append (vector.at descriptor)
         Range_Data start end step -> case step == 1 of
             True ->
                 builder.append_vector_range vector start end
             False ->
                 descriptor.each ix->
-                    builder.append (vector.unsafe_at ix)
+                    builder.append (vector.at ix)
     builder.to_vector

distribution/lib/Standard/Base/0.0.0-dev/src/Error/Common.enso

@@ -218,6 +218,7 @@ from project.Error.Common.Index_Out_Of_Bounds_Error export all
    Arguments:
    - index: The requested index.
    - length: The length of the collection.
+@Builtin_Type
 type Index_Out_Of_Bounds_Error
     Index_Out_Of_Bounds_Error_Data index length
 

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/IndexOutOfBoundsError.java

@@ -0,0 +1,14 @@
+package org.enso.interpreter.node.expression.builtin.error;
+
+import org.enso.interpreter.dsl.BuiltinType;
+import org.enso.interpreter.node.expression.builtin.UniquelyConstructibleBuiltin;
+
+import java.util.List;
+
+@BuiltinType
+public class IndexOutOfBoundsError extends UniquelyConstructibleBuiltin {
+  @Override
+  protected List<String> getConstructorParamNames() {
+    return List.of("index", "length");
+  }
+}

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/immutable/AtVectorNode.java

@@ -1,5 +1,6 @@
 package org.enso.interpreter.node.expression.builtin.immutable;
 
+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;
@@ -8,11 +9,12 @@
 import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
 import org.enso.interpreter.runtime.Context;
 import org.enso.interpreter.runtime.data.Vector;
+import org.enso.interpreter.runtime.error.DataflowError;
 import org.enso.interpreter.runtime.error.PanicException;
 
 @BuiltinMethod(
     type = "Vector",
-    name = "at_builtin",
+    name = "at",
     description = "Returns an element of Vector at the specified index.")
 public class AtVectorNode extends Node {
   private @Child InteropLibrary interop = InteropLibrary.getFactory().createDispatched(3);
@@ -24,9 +26,15 @@ Object execute(Vector self, long index) {
       return self.readArrayElement(actualIndex, interop, toEnso);
     } catch (InvalidArrayIndexException e) {
       Context ctx = Context.get(this);
-      throw new PanicException(
-          ctx.getBuiltins().error().makeInvalidArrayIndexError(self, index), this);
+      try {
+        return DataflowError.withoutTrace(
+            ctx.getBuiltins().error().makeIndexOutOfBoundsError(index, self.length(interop)), this);
+      } catch (UnsupportedMessageException ex) {
+        CompilerDirectives.transferToInterpreter();
+        throw new PanicException(e.getMessage(), this);
+      }
     } catch (UnsupportedMessageException e) {
+      CompilerDirectives.transferToInterpreter();
       throw new PanicException(e.getMessage(), this);
     }
   }

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/ArrayAtNode.java

@@ -9,24 +9,32 @@
 import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
 import org.enso.interpreter.runtime.Context;
 import org.enso.interpreter.runtime.builtin.Builtins;
+import org.enso.interpreter.runtime.error.DataflowError;
 import org.enso.interpreter.runtime.error.PanicException;
 
 @BuiltinMethod(type = "Array", name = "at", description = "Get element of a polyglot array")
 public class ArrayAtNode extends Node {
   @Child InteropLibrary iop = InteropLibrary.getFactory().createDispatched(3);
   @Child HostValueToEnsoNode convert = HostValueToEnsoNode.build();
 
-  Object execute(Object self, long at) {
+  Object execute(Object self, long index) {
     try {
-      var r = iop.readArrayElement(self, at);
+      long actualIndex = index < 0 ? index + iop.getArraySize(self) : index;
+      var r = iop.readArrayElement(self, actualIndex);
       return convert.execute(r);
     } catch (UnsupportedMessageException ex) {
       CompilerDirectives.transferToInterpreter();
       throw new IllegalStateException(ex);
-    } catch (InvalidArrayIndexException ex) {
+    } catch (InvalidArrayIndexException e) {
       Context ctx = Context.get(this);
-      Builtins builtins = ctx.getBuiltins();
-      throw new PanicException(builtins.error().makeInvalidArrayIndexError(self, at), this);
+      try {
+        return DataflowError.withoutTrace(
+            ctx.getBuiltins().error().makeIndexOutOfBoundsError(index, iop.getArraySize(self)),
+            this);
+      } catch (UnsupportedMessageException ex) {
+        CompilerDirectives.transferToInterpreter();
+        throw new PanicException(ex.getMessage(), this);
+      }
     }
   }
 }

engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Error.java

@@ -20,6 +20,7 @@ public class Error {
   private final SyntaxError syntaxError;
   private final TypeError typeError;
   private final CompileError compileError;
+  private final IndexOutOfBoundsError indexOutOfBoundsError;
   private final InexhaustivePatternMatchError inexhaustivePatternMatchError;
   private final UninitializedState uninitializedState;
   private final NoSuchMethodError noSuchMethodError;
@@ -50,6 +51,7 @@ public Error(Builtins builtins, Context context) {
     syntaxError = builtins.getBuiltinType(SyntaxError.class);
     typeError = builtins.getBuiltinType(TypeError.class);
     compileError = builtins.getBuiltinType(CompileError.class);
+    indexOutOfBoundsError = builtins.getBuiltinType(IndexOutOfBoundsError.class);
     inexhaustivePatternMatchError = builtins.getBuiltinType(InexhaustivePatternMatchError.class);
     uninitializedState = builtins.getBuiltinType(UninitializedState.class);
     noSuchMethodError = builtins.getBuiltinType(NoSuchMethodError.class);
@@ -76,6 +78,10 @@ public Atom makeCompileError(Object message) {
     return compileError.newInstance(message);
   }
 
+  public Atom makeIndexOutOfBoundsError(long index, long length) {
+    return indexOutOfBoundsError.newInstance(index, length);
+  }
+
   public Atom makeInexhaustivePatternMatchError(Object message) {
     return inexhaustivePatternMatchError.newInstance(message);
   }

test/Tests/src/Data/Array_Spec.enso

@@ -26,11 +26,12 @@ test_arrays array_from_vector =
         arr = array_from_vector [1, 2, 3]
         arr.at 0 . should_equal 1
         arr.at 2 . should_equal 3
+        arr.at -1 . should_equal 3
 
     Test.specify "should panic on out of bounds access" <|
         arr = array_from_vector [1, 2, 3]
-        Test.expect_panic_with (arr.at -1) Invalid_Array_Index_Error_Data
-        Test.expect_panic_with (arr.at 3) Invalid_Array_Index_Error_Data
+        (arr.at -4) . should_fail_with Index_Out_Of_Bounds_Error_Data
+        (arr.at 3) . should_fail_with Index_Out_Of_Bounds_Error_Data
 
 spec =
     Test.group "Enso Arrays" <|