Merge pull request #761 from ScorexFoundation/v5.0-finalize-part1

Finalize v5.0 (part1)

common/src/main/scala/scalan/util/CollectionUtil.scala

@@ -12,10 +12,11 @@ import scala.reflect.ClassTag
 
 object CollectionUtil {
 
+  /** @deprecated shouldn't be used other than for backwards compatibility with v3.x, v4.x. */
   def concatArrays[T](xs: Array[T], ys: Array[T]): Array[T] = {
     val len = xs.length + ys.length
     val result = (xs match {
-      case arr: Array[AnyRef] => new Array[AnyRef](len)
+      case arr: Array[AnyRef] => new Array[AnyRef](len) // creates an array with invalid type descriptor (i.e. when T == Tuple2)
       case arr: Array[Byte] => new Array[Byte](len)
       case arr: Array[Short] => new Array[Short](len)
       case arr: Array[Int] => new Array[Int](len)
@@ -30,6 +31,21 @@ object CollectionUtil {
     result
   }
 
+  /** Concatenates two arrays into a new resulting array.
+    * All items of both arrays are copied to the result using System.arraycopy.
+    * This method takes ClassTag to create proper resulting array.
+    * Can be used in v5.0 and above.
+    */
+  def concatArrays_v5[T:ClassTag](arr1: Array[T], arr2: Array[T]): Array[T] = {
+    val l1 = arr1.length
+    val l2 = arr2.length
+    val length: Int = l1 + l2
+    val result: Array[T] = new Array[T](length)
+    System.arraycopy(arr1, 0, result, 0, l1)
+    System.arraycopy(arr2, 0, result, l1, l2)
+    result
+  }
+
   def deepHashCode[T](arr: Array[T]): Int = arr match {
     case arr: Array[AnyRef] => util.Arrays.deepHashCode(arr)
     case arr: Array[Byte] => util.Arrays.hashCode(arr)
@@ -50,6 +66,10 @@ object CollectionUtil {
       }
     }
 
+  /** Group the given sequence of pairs by first values as keys.
+    * @param kvs sequence of values which is traversed once
+    * @return a multimap with ArrayBuffer of values for each key.
+    */
   def createMultiMap[K,V](kvs: GenIterable[(K,V)]): Map[K, ArrayBuffer[V]] = {
     val res = HashMap.empty[K, ArrayBuffer[V]]
     kvs.foreach { case (k, v) =>
@@ -62,12 +82,17 @@ object CollectionUtil {
     res.toMap
   }
 
-  // TODO optimize: using cfor and avoiding allocations
+  /** Perform relational inner join of two sequences using the given key projections. */
   def joinSeqs[O, I, K](outer: GenIterable[O], inner: GenIterable[I])(outKey: O=>K, inKey: I=>K): GenIterable[(O,I)] = {
     val kvs = createMultiMap(inner.map(i => (inKey(i), i)))
     val res = outer.flatMap(o => {
       val ko = outKey(o)
-      kvs(ko).map(i => (o,i))
+      kvs.get(ko) match {
+        case Some(inners) =>
+          inners.map(i => (o,i))
+        case None =>
+          Nil
+      }
     })
     res
   }

common/src/test/scala/scalan/util/CollectionUtilTests.scala

@@ -25,10 +25,15 @@ class CollectionUtilTests extends BaseTests {
     val zs = concatArrays(xs, ys)
     assertResult(Array[Byte](1, 2, 3, 4, 5, 6))(zs)
 
-//    val jxs = Array[JByte](new JByte(1), new JByte(2), new JByte(3))
-//    val jys = Array[JByte](new JByte(4), new JByte(5), new JByte(6))
-//    val jzs = concatArrays(jxs, jys)
-//    assertResult(Array[Byte](1, 2, 3, 4, 5, 6))(jzs)
+    val pairs = xs.zip(ys)
+    // this reproduces the problem which takes place in v3.x, v4.x (ErgoTree v0, v1)
+    an[ClassCastException] should be thrownBy(concatArrays(pairs, pairs))
+
+    // and this is the fix in v5.0
+    concatArrays_v5(pairs, pairs) shouldBe Array((1, 4), (2, 5), (3, 6), (1, 4), (2, 5), (3, 6))
+
+    val xOpts = xs.map(Option(_))
+    concatArrays_v5(xOpts, xOpts) shouldBe Array(Some(1), Some(2), Some(3), Some(1), Some(2), Some(3))
   }
 
   def join(l: Map[Int,Int], r: Map[Int,Int]) =
@@ -38,6 +43,30 @@ class CollectionUtilTests extends BaseTests {
   def joinPairs(l: Seq[(String,Int)], r: Seq[(String,Int)]) =
     outerJoinSeqs(l, r)(l => l._1, r => r._1)((_,l) => l._2, (_,r) => r._2, (k,l,r) => l._2 + r._2)
 
+  test("joinSeqs") {
+    def key(p : (Int, String)): Int = p._1
+
+    {
+      val res = CollectionUtil.joinSeqs(
+        outer = Seq(1 -> "o1", 1 -> "o1"),
+        inner = Seq(1 -> "i1", 2 -> "i2"))(key, key)
+      res shouldBe Seq(
+        (1 -> "o1") -> (1 -> "i1"),
+        (1 -> "o1") -> (1 -> "i1")
+      )
+    }
+
+    { // same as above, but swapping inner and outer
+      val res = CollectionUtil.joinSeqs(
+        outer = Seq(1 -> "o1", 2 -> "o2"),
+        inner = Seq(1 -> "i1", 1 -> "i1"))(key, key)
+      res shouldBe Seq(
+        (1 -> "o1") -> (1 -> "i1"),
+        (1 -> "o1") -> (1 -> "i1")
+      )
+    }
+  }
+
   test("outerJoin maps") {
     val left = Map(1 -> 1, 2 -> 2, 3 -> 3)
     val right = Map(2 -> 2, 3 -> 3, 4 -> 4)

core/src/main/scala/scalan/TypeDescs.scala

@@ -55,7 +55,7 @@ abstract class TypeDescs extends Base { self: Scalan =>
   case class RMethodDesc(method: Method) extends MethodDesc
   case class WMethodDesc(wrapSpec: WrapSpec, method: Method) extends MethodDesc
 
-// TODO benchmark this version agains the version below
+// TODO optimize: benchmark this version agains the version below
 //  def getSourceValues(dataEnv: DataEnv, forWrapper: Boolean, stagedValue: AnyRef, out: DBuffer[AnyRef]): Unit = {
 //    import OverloadHack._
 //    stagedValue match {
@@ -154,7 +154,7 @@ abstract class TypeDescs extends Base { self: Scalan =>
     protected def collectMethods: Map[Method, MethodDesc] = Map() // TODO optimize: all implementations
     protected lazy val methods: Map[Method, MethodDesc] = collectMethods
 
-    // TODO benchamrk against the version below it
+    // TODO optimize: benchamrk against the version below it
     //    def invokeUnlifted(mc: MethodCall, dataEnv: DataEnv): AnyRef = {
     //      val srcArgs = DBuffer.ofSize[AnyRef](mc.args.length + 10)  // with some spare space to have only single allocation
     //      val res = methods.get(mc.method) match {
@@ -244,7 +244,6 @@ abstract class TypeDescs extends Base { self: Scalan =>
         m.getName
     }
 
-    // TODO optimize
     /** Build a mapping between methods of staged class and the corresponding methods of source class.
       * The methods are related using names.
       * The computed mapping can be used to project MethodCalls IR nodes back to the corresponding

library-impl/src/main/scala/special/collection/CollsOverArrays.scala

@@ -211,7 +211,7 @@ class CollOverArrayBuilder extends CollBuilder {
   override def Monoids: MonoidBuilder = new MonoidBuilderInst
 
   @inline override def pairColl[@specialized A, @specialized B](as: Coll[A], bs: Coll[B]): PairColl[A, B] = {
-    // TODO HF (2h): use minimal length and slice longer collection
+    // TODO v6.0 (2h): use minimal length and slice longer collection
     // The current implementation doesn't check the case when `as` and `bs` have different lengths.
     // in which case the implementation of `PairOfCols` has inconsistent semantics of `map`, `exists` etc methods.
     // To fix the problem, the longer collection have to be truncated (which is consistent

library/src/test/scala/special/collections/CollsTests.scala

@@ -5,11 +5,14 @@ import special.collection.{Coll, PairOfCols, CollOverArray, CReplColl}
 import org.scalacheck.Gen
 import org.scalatest.{PropSpec, Matchers}
 import org.scalatest.prop.PropertyChecks
+import scalan.RType
 
 class CollsTests extends PropSpec with PropertyChecks with Matchers with CollGens { testSuite =>
   import Gen._
   import special.collection.ExtensionMethods._
 
+  def squared[A](f: A => A): ((A, A)) => (A, A) = (p: (A, A)) => (f(p._1), f(p._2))
+
   property("Coll.indices") {
     val minSuccess = MinSuccessful(30)
     forAll(collGen, collGen, minSuccess) { (col1: Coll[Int], col2: Coll[Int]) =>
@@ -20,8 +23,41 @@ class CollsTests extends PropSpec with PropertyChecks with Matchers with CollGen
     }
   }
 
-  // TODO col1.zip(col2).length shouldBe col1.arr.zip(col2.arr).length
-  property("Coll.zip") {
+  // TODO v6.0: make sure forall: T, col: Coll[T] => col.length shouldBe col.toArray.length
+  //  The above equality should hold for all possible collection instances
+  property("Coll.length") {
+    def equalLength[A: RType](xs: Coll[A]) = {
+      val arr = xs.toArray
+      xs.length shouldBe arr.length
+      xs.zip(xs).length shouldBe arr.zip(arr).length
+      xs.zip(xs.append(xs)).length shouldBe arr.zip(arr ++ arr).length
+      xs.append(xs).zip(xs).length shouldBe (arr ++ arr).zip(arr).length
+    }
+
+    def equalLengthMapped[A: RType](xs: Coll[A], f: A => A) = {
+      val arr = xs.toArray
+      val ys = xs.map(f)
+      ys.length shouldBe xs.length
+      ys.length shouldBe arr.map(f).length
+
+      equalLength(ys)
+      equalLength(xs.append(ys))
+      equalLength(ys.append(xs))
+    }
+
+    forAll(MinSuccessful(100)) { xs: Coll[Int] =>
+      val arr = xs.toArray
+      equalLength(xs)
+      equalLengthMapped(xs, inc)
+
+      equalLength(xs.append(xs))
+      equalLengthMapped(xs.append(xs), inc)
+
+      val pairs = xs.zip(xs)
+      equalLength(pairs)
+
+      equalLength(pairs.append(pairs))
+    }
   }
 
   property("Coll.flatMap") {
@@ -198,6 +234,15 @@ class CollsTests extends PropSpec with PropertyChecks with Matchers with CollGen
   }
 
   property("Coll.append") {
+
+    { // this shows how the problem with CollectionUtil.concatArrays manifests itself in Coll.append
+      val xs = builder.fromItems(1, 2)
+      val pairs = xs.zip(xs)
+      val ys = pairs.map(squared(inc)) // this map transforms PairOfCols to CollOverArray
+      // due to the problem with concatArrays
+      an[ClassCastException] should be thrownBy (ys.append(ys))
+    }
+
     forAll(collGen, collGen, valGen, MinSuccessful(50)) { (col1, col2, v) =>
 
       {
@@ -212,13 +257,18 @@ class CollsTests extends PropSpec with PropertyChecks with Matchers with CollGen
       {
         val repl1 = builder.replicate(col1.length, v)
         val repl2 = builder.replicate(col2.length, v)
+        assert(repl1.isInstanceOf[CReplColl[Int]])
+
         val arepl = repl1.append(repl2)
         assert(arepl.isInstanceOf[CReplColl[Int]])
         arepl.toArray shouldBe (repl1.toArray ++ repl2.toArray)
 
         val pairs1 = repl1.zip(repl1)
+        assert(pairs1.isInstanceOf[PairOfCols[Int, Int]])
+
         val pairs2 = repl2.zip(repl2)
         val apairs = pairs1.append(pairs2)
+        assert(apairs.isInstanceOf[PairOfCols[Int, Int]])
         apairs.toArray shouldBe (pairs1.toArray ++ pairs2.toArray)
 
         apairs match {
@@ -266,10 +316,6 @@ class CollsTests extends PropSpec with PropertyChecks with Matchers with CollGen
       res.toArray shouldBe col.toArray.reverse
       val pairs = col.zip(col)
       pairs.reverse.toArray shouldBe pairs.toArray.reverse
-// TODO should work
-//      val c1 = col.asInstanceOf[Coll[Any]]
-//      val appended = c1.append(c1)
-//      appended.toArray shouldBe (c1.toArray ++ c1.toArray)
     }
   }
 

sigma-impl/src/main/scala/special/sigma/SigmaDslOverArrays.scala

@@ -40,7 +40,7 @@ class TestSigmaDslBuilder extends SigmaDslBuilder {
 
   @NeverInline
   override def xorOf(conditions: Coll[Boolean]): Boolean = {
-    // TODO HF (2h): see https://github.com/ScorexFoundation/sigmastate-interpreter/issues/640
+    // This is buggy version used in v4.x interpreter (for ErgoTrees v0, v1)
     conditions.toArray.distinct.length == 2
   }
 

sigmastate/src/main/scala/org/ergoplatform/ErgoLikeContext.scala

@@ -165,6 +165,8 @@ class ErgoLikeContext(val lastBlockUtxoRoot: AvlTreeData,
     }
     val vars = contextVars(varMap ++ extensions)
     val avlTree = CAvlTree(lastBlockUtxoRoot)
+    // so selfBox is never one of the `inputs` instances
+    // as result selfBoxIndex is always (erroneously) returns -1 in ErgoTree v0, v1
     val selfBox = boxesToSpend(selfIndex).toTestBox(isCost)
     val ergoTreeVersion = currentErgoTreeVersion.getOrElse(
         Interpreter.error(s"Undefined context property: currentErgoTreeVersion"))

sigmastate/src/main/scala/org/ergoplatform/ErgoLikeInterpreter.scala

@@ -30,11 +30,11 @@ class ErgoLikeInterpreter(implicit val IR: IRContext) extends Interpreter {
               else
                 Some(outVal)
             case _ =>
-              // TODO HF (1h): this case is not possible because `ErgoBox.get`
+              // this case is not possible because `ErgoBox.get`
               //  returns lookups values from `additionalRegisters` Map with values
               //  of type EvaluatedValue, which are always Constant nodes in practice.
-              //  Also, this branch is never executed so can be safely removed
-              //  (better as part of the HF)
+              //  Also, this branch is never executed so can be safely removed, but
+              //  we keep it here explicitly for clarity
               None
           }
         }.orElse(d.default)

sigmastate/src/main/scala/sigmastate/DataValueComparer.scala

@@ -125,7 +125,6 @@ object DataValueComparer {
   final val OpDesc_EQ_COA_AvlTree = NamedDesc("EQ_COA_AvlTree")
   final val EQ_COA_AvlTree = OperationCostInfo(CostKind_EQ_COA_AvlTree, OpDesc_EQ_COA_AvlTree)
 
-  // TODO v5.0: update value after serialization is avoided to compute ErgoBox.id
   /** Equals two CollOverArray of Box type. */
   final val CostKind_EQ_COA_Box = PerItemCost(
     baseCost = JitCost(15), perChunkCost = JitCost(5), chunkSize = 1)

sigmastate/src/main/scala/sigmastate/UnprovenTree.scala

@@ -201,8 +201,6 @@ case class UnprovenDiffieHellmanTuple(override val proposition: ProveDHTuple,
   override def withPosition(updatedPosition: NodePosition) = this.copy(position = updatedPosition)
 }
 
-// TODO coverage (8h): write a test that restores the tree from this string and check that the result is equal,
-// in order to make sure this conversion is unambiguous
 object FiatShamirTree {
   /** Prefix byte which is put before the other ProofTreeConjecture serialized bytes. */
   val internalNodePrefix: Byte = 0

sigmastate/src/main/scala/sigmastate/Values.scala

@@ -3,15 +3,14 @@ package sigmastate
 import java.math.BigInteger
 import java.util
 import java.util.Objects
-
 import org.bitbucket.inkytonik.kiama.rewriting.Rewriter.{count, everywherebu, strategy}
 import org.ergoplatform.settings.ErgoAlgos
 import org.ergoplatform.validation.ValidationException
 import scalan.{Nullable, RType}
 import scalan.util.CollectionUtil._
 import sigmastate.SCollection.{SByteArray, SIntArray}
 import sigmastate.interpreter.CryptoConstants.EcPointType
-import sigmastate.interpreter.{CompanionDesc, CryptoConstants, ErgoTreeEvaluator, NamedDesc}
+import sigmastate.interpreter.{CompanionDesc, CryptoConstants, ErgoTreeEvaluator, Interpreter, NamedDesc}
 import sigmastate.serialization.{ConstantStore, OpCodes, _}
 import sigmastate.serialization.OpCodes._
 import sigmastate.TrivialProp.{FalseProp, TrueProp}
@@ -885,18 +884,14 @@ object Values {
     *
     * @param items source collection of expressions
     */
-  case class Tuple(items: IndexedSeq[Value[SType]]) extends EvaluatedValue[STuple] with EvaluatedCollection[SAny.type, STuple] {
+  case class Tuple(items: IndexedSeq[Value[SType]]) extends Value[STuple] {
     override def companion = Tuple
-    override def elementType = SAny
     override lazy val tpe = STuple(items.map(_.tpe))
-    override lazy val value = { // TODO coverage
-      val xs = items.cast[EvaluatedValue[SAny.type]].map(_.value)
-      Colls.fromArray(xs.toArray(SAny.classTag.asInstanceOf[ClassTag[SAny.WrappedType]]))(RType.AnyType)
-    }
+    override def opType: SFunc = ???
     protected final override def eval(env: DataEnv)(implicit E: ErgoTreeEvaluator): Any = {
       // in v5.0 version we support only tuples of 2 elements to be equivalent with v4.x
       if (items.length != 2)
-        error(s"Invalid tuple $this")
+        Interpreter.error(s"Invalid tuple $this")
 
       val item0 = items(0)
       val x = item0.evalTo[Any](env)
@@ -923,8 +918,8 @@ object Values {
   trait OptionValue[T <: SType] extends Value[SOption[T]] {
   }
 
-  // TODO HF (4h): SomeValue and NoneValue are not used in ErgoTree and can be
-  //  either removed or implemented in v4.x
+  // TODO v6.0 (4h): SomeValue and NoneValue are not used in ErgoTree and can be
+  //  either removed or implemented in v6.0
   case class SomeValue[T <: SType](x: Value[T]) extends OptionValue[T] {
     override def companion = SomeValue
     val tpe = SOption(x.tpe)
@@ -1175,13 +1170,7 @@ object Values {
 
     protected final override def eval(env: DataEnv)(implicit E: ErgoTreeEvaluator): Any = {
       addCost(FuncValue.costKind)
-      if (args.length == 0) {
-        // TODO coverage
-        () => {
-          body.evalTo[Any](env)
-        }
-      }
-      else if (args.length == 1) {
+      if (args.length == 1) {
         val arg0 = args(0)
         (vArg: Any) => {
           Value.checkType(arg0._2, vArg)
@@ -1194,22 +1183,8 @@ object Values {
           Value.checkType(body, res)
           res
         }
-      }
-      else {
-        // TODO coverage
-        (vArgs: Seq[Any]) => {
-          var env1 = env
-          val len = args.length
-          cfor(0)(_ < len, _ + 1) { i =>
-            val id = args(i)._1
-            val v = vArgs(i)
-            E.addFixedCost(FuncValue.AddToEnvironmentDesc_CostKind,
-                                   FuncValue.AddToEnvironmentDesc) {
-              env1 = env1 + (id -> v)
-            }
-          }
-          body.evalTo[Any](env1)
-        }
+      } else {
+        Interpreter.error(s"Function must have 1 argument, but was: $this")
       }
     }
   }