Fix semantic issue according to Stefan's review comments and add test cases

Author: ackratos

benchmarks/time/src/main/scala/strawman/collection/immutable/ImmutableArrayBenchmark.scala

@@ -10,7 +10,7 @@ import scala.Predef.intWrapper
 
 @BenchmarkMode(scala.Array(Mode.AverageTime))
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
-@Fork(2)
+@Fork(1)
 @Warmup(iterations = 8)
 @Measurement(iterations = 8)
 @State(Scope.Benchmark)

collections/src/main/scala/strawman/collection/immutable/ImmutableArray.scala

@@ -186,7 +186,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[T] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange[T](unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofRef(Arrays.copyOfRange[T](unsafeArray, lo, hi))
     }
   }
 
@@ -201,7 +202,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Byte] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofByte(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -216,7 +218,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Short] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofShort(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -231,7 +234,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Char] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofChar(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -246,7 +250,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Int] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofInt(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -261,7 +266,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Long] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofLong(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -276,7 +282,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Float] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofFloat(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -291,7 +298,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Double] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofDouble(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -306,7 +314,8 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
     }
     override def slice(from: Int, until: Int): ImmutableArray[Boolean] = {
       val lo = scala.math.max(from, 0)
-      ImmutableArray.unsafeWrapArray(Arrays.copyOfRange(unsafeArray, lo, until))
+      val hi = scala.math.min(until, length)
+      new ofBoolean(Arrays.copyOfRange(unsafeArray, lo, hi))
     }
   }
 
@@ -324,8 +333,9 @@ object ImmutableArray extends StrictOptimizedSeqFactory[ImmutableArray] {
       // new ofUnit(util.Arrays.copyOfRange[Unit](array, from, until)) - Unit is special and doesnt compile
       // cant use util.Arrays.copyOfRange[Unit](repr, from, until) - Unit is special and doesnt compile
       val lo = scala.math.max(from, 0)
-      val res = new Array[Unit](until-lo)
-      System.arraycopy(unsafeArray, lo, res, 0, until-lo)
+      val hi = scala.math.min(until, length)
+      val slicedLenght = hi - lo
+      val res = new Array[Unit](slicedLenght)
       new ofUnit(res)
     }
   }

test/junit/src/test/scala/strawman/collection/immutable/ImmutableArrayTest.scala

@@ -0,0 +1,60 @@
+package strawman.collection.immutable
+
+import org.junit.Assert
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(classOf[JUnit4])
+class ImmutableArrayTest {
+  @Test
+  def slice(): Unit = {
+    import scala.language.implicitConversions
+
+    implicit def array2ImmutableArray[T](array: Array[T]): ImmutableArray[T] =
+      ImmutableArray.unsafeWrapArray(array)
+
+    val booleanArray = Array(true, false, true, false)
+    check(booleanArray, Array(true, false), Array(false, true))
+
+    val shortArray = Array(1.toShort, 2.toShort, 3.toShort, 4.toShort)
+    check(shortArray, Array(1.toShort, 2.toShort), Array(2.toShort, 3.toShort))
+
+    val intArray = Array(1, 2, 3, 4)
+    check(intArray, Array(1, 2), Array(2, 3))
+
+    val longArray = Array(1L, 2L, 3L, 4L)
+    check(longArray, Array(1L, 2L), Array(2L, 3L))
+
+    val byteArray = Array(1.toByte, 2.toByte, 3.toByte, 4.toByte)
+    check(byteArray, Array(1.toByte, 2.toByte), Array(2.toByte, 3.toByte))
+
+    val charArray = Array('1', '2', '3', '4')
+    check(charArray, Array('1', '2'), Array('2', '3'))
+
+    val doubleArray = Array(1.0, 2.0, 3.0, 4.0)
+    check(doubleArray, Array(1.0, 2.0), Array(2.0, 3.0))
+
+    val floatArray = Array(1.0f, 2.0f, 3.0f, 4.0f)
+    check(floatArray, Array(1.0f, 2.0f), Array(2.0f, 3.0f))
+
+    val refArray = Array("1", "2", "3", "4")
+    check[String](refArray, Array("1", "2"), Array("2", "3"))
+
+    def unit1(): Unit = {}
+    def unit2(): Unit = {}
+    Assert.assertEquals(unit1, unit2)
+    // unitArray is actually an instance of Immutable[BoxedUnit], the check to which is actually checked slice
+    // implementation of ofRef
+    val unitArray: ImmutableArray[Unit] = Array(unit1, unit2, unit1, unit2)
+    check(unitArray, Array(unit1, unit1), Array(unit1, unit1))
+  }
+
+  private def check[T](array: ImmutableArray[T], expectedSliceResult1: ImmutableArray[T], expectedSliceResult2: ImmutableArray[T]) {
+    Assert.assertEquals(array, array.slice(-1, 4))
+    Assert.assertEquals(array, array.slice(0, 5))
+    Assert.assertEquals(array, array.slice(-1, 5))
+    Assert.assertEquals(expectedSliceResult1, array.slice(0, 2))
+    Assert.assertEquals(expectedSliceResult2, array.slice(1, 3))
+  }
+}