Fix DAML runtime for the new DAML-LF type Map (digital-asset#204)

* add a test for daml-lf/interface * fix scala code gen * fix extractor * fix navigator backend/frontend * key of Map are strings in proto/json
iggy-da · Apr 10, 2019 · 0785845 · 0785845
1 parent aee5af8
commit 0785845
Show file tree

Hide file tree

Showing 64 changed files with 885 additions and 266 deletions.
diff --git a/compiler/scenario-service/protos/scenario_service.proto b/compiler/scenario-service/protos/scenario_service.proto
@@ -316,6 +316,7 @@ message Map {
 
     repeated Entry entries = 1;
 }
+
 // Modules that can be loaded. The actual message types are not
 // used here as LF modules can have deep nesting depth and it is
 // not easy to change the recursion depth limit in gRPC.

diff --git a/...cenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Conversions.scala b/...cenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Conversions.scala
@@ -616,9 +616,10 @@ case class Conversions(homePackageId: Ref.PackageId) {
         builder.setOptional(optionalBuilder)
       case V.ValueMap(map) =>
         val mapBuilder = v1.Map.newBuilder
-        map.foreach {
+        map.toImmArray.foreach {
           case (k, v) =>
             mapBuilder.addEntries(v1.Map.Entry.newBuilder().setKey(k).setValue(convertValue(v)))
+            ()
         }
         builder.setMap(mapBuilder)
     }

diff --git a/daml-lf/archive/da/daml_lf_1.proto b/daml-lf/archive/da/daml_lf_1.proto
@@ -233,7 +233,7 @@ message Type {
     // Name of the type constructor name 
     TypeConName tycon = 1;
 
-    // Type to wich the constructor name is applied.
+    // Type to which the constructor name is applied.
     repeated Type args = 2;
   }
 

diff --git a/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/data/SortedLookupList.scala b/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/data/SortedLookupList.scala
@@ -0,0 +1,64 @@
+// Copyright (c) 2019 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.digitalasset.daml.lf.data
+
+import scala.collection.immutable.HashMap
+
+/** We use this container to pass around DAML-LF maps as flat lists in various parts of the codebase. */
+class SortedLookupList[+X] private (entries: ImmArray[(String, X)]) {
+
+  def mapValue[Y](f: X => Y) = new SortedLookupList(entries.map { case (k, v) => k -> f(v) })
+
+  def toImmArray: ImmArray[(String, X)] = entries
+
+  def keys: ImmArray[String] = entries.map(_._1)
+
+  def values: ImmArray[X] = entries.map(_._2)
+
+  def toHashMap: HashMap[String, X] = HashMap(entries.toSeq: _*)
+
+  override def equals(obj: Any): Boolean = {
+    obj match {
+      case other: SortedLookupList[X] => other.toImmArray == entries
+      case _ => false
+    }
+  }
+
+  override def hashCode(): Int = entries.hashCode()
+
+  override def toString: String =
+    s"SortedMap(${entries.map { case (k, v) => k -> v }.toSeq.mkString(",")})"
+}
+
+object SortedLookupList {
+
+  // Note: it's important that this ordering is the same as the DAML-LF ordering.
+  private implicit val keyOrdering: Ordering[String] = UTF8.ordering
+
+  def fromImmArray[X](entries: ImmArray[(String, X)]): Either[String, SortedLookupList[X]] = {
+    entries.toSeq
+      .groupBy(_._1)
+      .collectFirst {
+        case (k, l) if l.size > 1 => s"key $k duplicated when trying to build map"
+      }
+      .toLeft(new SortedLookupList(entries.toSeq.sortBy(_._1).toImmArray))
+  }
+
+  def fromSortedImmArray[X](entries: ImmArray[(String, X)]): Either[String, SortedLookupList[X]] = {
+    entries
+      .map(_._1)
+      .toSeq
+      .sliding(2)
+      .collectFirst {
+        case Seq(k1, k2) if keyOrdering.gteq(k1, k2) => s"the list $entries is not sorted by key"
+      }
+      .toLeft(new SortedLookupList(entries))
+  }
+
+  def apply[X](entries: Map[String, X]): SortedLookupList[X] =
+    new SortedLookupList(ImmArray(entries.toSeq.sortBy(_._1)))
+
+  def empty[X]: SortedLookupList[X] = new SortedLookupList(ImmArray.empty)
+
+}
diff --git a/daml-lf/data/src/test/scala/com/digitalasset/daml/lf/data/SortedLookupListSpec.scala b/daml-lf/data/src/test/scala/com/digitalasset/daml/lf/data/SortedLookupListSpec.scala
@@ -0,0 +1,50 @@
+// Copyright (c) 2019 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.digitalasset.daml.lf.data
+
+import org.scalatest.prop.PropertyChecks
+import org.scalatest.{Matchers, WordSpec}
+
+class SortedLookupListSpec extends WordSpec with Matchers with PropertyChecks {
+
+  "SortMap.fromImmArray should fails if the input list contians duplicate keys" in {
+
+    val negativeTestCases = Table(
+      "list",
+      ImmArray.empty[(String, Int)],
+      ImmArray("1" -> 1),
+      ImmArray("1" -> 1, "2" -> 2, "3" -> 3),
+      ImmArray("2" -> 2, "3" -> 3, "1" -> 1))
+
+    val positiveTestCases =
+      Table("list", ImmArray("1" -> 1, "1" -> 2), ImmArray("1" -> 1, "2" -> 2, "3" -> 3, "1" -> 2))
+
+    forAll(negativeTestCases)(l => SortedLookupList.fromImmArray(l) shouldBe 'right)
+
+    forAll(positiveTestCases)(l => SortedLookupList.fromImmArray(l) shouldBe 'left)
+
+  }
+
+  "SortMap.fromSortedImmArray should fails if the input list is not sorted" in {
+
+    val negativeTestCases =
+      Table(
+        "list",
+        ImmArray.empty[(String, Int)],
+        ImmArray("1" -> 1),
+        ImmArray("1" -> 1, "2" -> 2, "3" -> 3))
+
+    val positiveTestCases = Table(
+      "list",
+      ImmArray("1" -> 1, "1" -> 2),
+      ImmArray("1" -> 1, "2" -> 2, "3" -> 3, "1" -> 2),
+      ImmArray("2" -> 2, "3" -> 3, "1" -> 1))
+
+    forAll(negativeTestCases)(l => SortedLookupList.fromSortedImmArray(l) shouldBe 'right)
+
+    forAll(positiveTestCases)(l => SortedLookupList.fromSortedImmArray(l) shouldBe 'left)
+
+  }
+
+}
diff --git a/daml-lf/engine/BUILD.bazel b/daml-lf/engine/BUILD.bazel
@@ -37,7 +37,7 @@ da_scala_test(
     timeout = "moderate",
     srcs = glob(["src/test/**/*.scala"]),
     resources = [
-        "//daml-foundations/daml-ghc/package-database/deprecated:daml-prim-1.1.dalf",
+        "//daml-foundations/daml-ghc/package-database/deprecated:daml-prim-1.3.dalf",
         "//daml-lf/tests:BasicTests.dalf",
         "//daml-lf/tests:LargeTransaction.dalf",
         "//daml-lf/tests:Optional.dalf",

diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/CommandTranslation.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/CommandTranslation.scala
@@ -14,9 +14,45 @@ private[engine] object CommandTranslation {
   def apply(compiledPackages: ConcurrentCompiledPackages): CommandTranslation = {
     new CommandTranslation(compiledPackages)
   }
+
+  private def tEntry(elemType: Type) =
+    TTuple(ImmArray(("key", TBuiltin(BTText)), ("value", elemType)))
+
+  private def entry(key: String, value: Expr): Expr =
+    ETupleCon(ImmArray("key" -> EPrimLit(PLText(key)), "value" -> value))
+
+  private def emptyMap(elemType: Type) =
+    EBuiltin(BMapEmpty) eTyApp elemType
+
+  private def fold(aType: Type, bType: Type, f: Expr, b: Expr, as: Expr) =
+    EBuiltin(BFoldl) eTyApp (aType, bType) eApp (f, b, as)
+
+  private def insert(elemType: Type, key: Expr, value: Expr, map: Expr) =
+    EBuiltin(BMapInsert) eTyApp elemType eApp (key, value, map)
+
+  private def get(key: String, tuple: Expr) =
+    ETupleProj(key, tuple)
+
+  private def buildList(elemType: Type, list: ImmArray[Expr]) =
+    ECons(elemType, list, ENil(elemType))
+
+  private def buildMap(elemType: Type, list: Expr): Expr = {
+    val f =
+      EAbs(
+        "acc" -> TMap(elemType),
+        EAbs(
+          "entry" -> tEntry(elemType),
+          insert(elemType, get("key", EVar("entry")), get("value", EVar("entry")), EVar("acc")),
+          None),
+        None)
+    fold(tEntry(elemType), TMap(elemType), f, emptyMap(elemType), list)
+  }
+
 }
 
 private[engine] class CommandTranslation(compiledPackages: ConcurrentCompiledPackages) {
+  import CommandTranslation._
+
   // we use this for easier error handling in translateValues
   private[this] case class CommandTranslationException(err: Error)
       extends RuntimeException(err.toString, null, true, false)
@@ -117,6 +153,15 @@ private[engine] class CommandTranslation(compiledPackages: ConcurrentCompiledPac
                 .traverseU(go(newNesting, elemType, _))
                 .map(es => ECons(elemType, es, ENil(elemType)))
             }
+
+          // map
+          case (TMap(elemType), ValueMap(map)) =>
+            map.toImmArray
+              .traverseU {
+                case (key0, value0) => go(newNesting, elemType, value0).map(entry(key0, _))
+              }
+              .map(l => buildMap(elemType, buildList(tEntry(elemType), l)))
+
           // variants
           case (TTyConApp(tyCon, tyConArgs), ValueVariant(mbVariantId, constructorName, value)) =>
             val variantId = tyCon

diff --git a/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala b/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala
@@ -46,7 +46,7 @@ class EngineTest extends WordSpec with Matchers {
   private val (basicTestsPkgId, basicTestsPkg) =
     loadPackage("daml-lf/tests/BasicTests.dalf")
   private val (ghcPrimPkgId, ghcPrimPkg) =
-    loadPackage("daml-foundations/daml-ghc/package-database/deprecated/daml-prim-1.1.dalf")
+    loadPackage("daml-foundations/daml-ghc/package-database/deprecated/daml-prim-1.3.dalf")
 
   private[this] def makeAbsoluteContractId(coid: ContractId): AbsoluteContractId =
     coid match {

diff --git a/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/LargeTransactionTest.scala b/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/LargeTransactionTest.scala
@@ -20,7 +20,7 @@ class LargeTransactionTest extends WordSpec with Matchers {
 
   private[this] val largeTransactionId = "daml-lf/tests/LargeTransaction.dalf"
   private[this] val ghcPrimId =
-    "daml-foundations/daml-ghc/package-database/deprecated/daml-prim-1.1.dalf"
+    "daml-foundations/daml-ghc/package-database/deprecated/daml-prim-1.3.dalf"
 
   private[this] val largeTx: (PackageId, Ast.Package) = loadArchiveAsResource(largeTransactionId)
   private[this] val ghcPrim: (PackageId, Ast.Package) =

diff --git a/.../interface/src/test/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReaderSpec.scala b/.../interface/src/test/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReaderSpec.scala
@@ -16,6 +16,7 @@ import com.digitalasset.daml.lf.data.Ref.{
 import com.digitalasset.daml_lf.DamlLf1
 import org.scalatest.{Inside, Matchers, WordSpec}
 import scalaz.\/-
+import scala.collection.JavaConverters._
 
 class InterfaceReaderSpec extends WordSpec with Matchers with Inside {
 
@@ -115,6 +116,35 @@ class InterfaceReaderSpec extends WordSpec with Matchers with Inside {
     actual shouldBe \/-(expectedRecord)
   }
 
+  "map should extract a map" in {
+    val recordDataType = DamlLf1.DefDataType
+      .newBuilder()
+      .setName(dottedName("MapRecord"))
+      .setRecord(
+        DamlLf1.DefDataType.Fields
+          .newBuilder()
+          .addFields(
+            primField(
+              "map",
+              DamlLf1.PrimType.MAP,
+              DamlLf1.Type
+                .newBuilder()
+                .setPrim(DamlLf1.Type.Prim.newBuilder().setPrim(DamlLf1.PrimType.INT64))
+                .build()))
+          .build())
+      .build()
+
+    val actual = InterfaceReader.record(moduleName, ctx)(recordDataType)
+    actual shouldBe \/-(
+      (
+        QualifiedName(moduleName, dnfs("MapRecord")),
+        ImmArraySeq(),
+        Record(
+          ImmArraySeq(
+            ("map", TypePrim(PrimTypeMap, ImmArraySeq(TypePrim(PrimTypeInt64, ImmArraySeq()))))
+          ))))
+  }
+
   private def dottedName(str: String): DamlLf1.DottedName =
     DamlLf1.DottedName.newBuilder().addSegments(str).build()
 
@@ -138,12 +168,17 @@ class InterfaceReaderSpec extends WordSpec with Matchers with Inside {
       .setType(DamlLf1.Type.newBuilder().setVar(DamlLf1.Type.Var.newBuilder.setVar(var_).build))
       .build()
 
-  private def primField(field: String, type_ : DamlLf1.PrimType): DamlLf1.FieldWithType =
+  private def primField(
+      field: String,
+      primType: DamlLf1.PrimType,
+      args: DamlLf1.Type*): DamlLf1.FieldWithType = {
+    val typ = DamlLf1.Type.Prim.newBuilder.setPrim(primType).addAllArgs(args.asJava).build()
     DamlLf1.FieldWithType
       .newBuilder()
       .setField(field)
-      .setType(DamlLf1.Type.newBuilder().setPrim(DamlLf1.Type.Prim.newBuilder.setPrim(type_).build))
+      .setType(DamlLf1.Type.newBuilder().setPrim(typ).build)
       .build()
+  }
 
   private def typeConstructorField(field: String, segments: List[String]): DamlLf1.FieldWithType =
     DamlLf1.FieldWithType

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala
@@ -380,10 +380,10 @@ object Pretty {
       case ValueOptional(Some(v1)) => text("Option(") + prettyValue(verbose)(v1) + char(')')
       case ValueOptional(None) => text("None")
       case ValueMap(map) =>
-        val list = map.toList.sortBy(_._1).map {
+        val list = map.toImmArray.map {
           case (k, v) => text(k) + text(" -> ") + prettyValue(verbose)(v)
         }
-        text("Map(") + intercalate(text(", "), list) + text(")")
+        text("Map(") + intercalate(text(", "), list.toSeq) + text(")")
     }
 
   object SExpr {

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala
@@ -7,7 +7,7 @@ import java.security.MessageDigest
 import java.util
 
 import com.digitalasset.daml.lf.data.Ref._
-import com.digitalasset.daml.lf.data.{Decimal, FrontStack, ImmArray, Time, UTF8}
+import com.digitalasset.daml.lf.data._
 import com.digitalasset.daml.lf.lfpackage.Ast._
 import com.digitalasset.daml.lf.speedy.SError._
 import com.digitalasset.daml.lf.speedy.SExpr._
@@ -314,8 +314,8 @@ object SBuiltin {
     def execute(args: util.ArrayList[SValue], machine: Machine): Unit = {
       machine.ctrl = CtrlValue(args.get(0) match {
         case SMap(map) =>
-          val entries = map.toList.sortBy(_._1)(UTF8.ordering)
-          SList((entries :\ FrontStack.empty[STuple]) { case ((k, v), acc) => acc.+:(entry(k, v)) })
+          val entries = SortedLookupList(map).toImmArray
+          SList(FrontStack(entries.map { case (k, v) => entry(k, v) }))
         case x =>
           throw SErrorCrash(s"type mismatch SBMaptoList, expected Map get $x")
       })

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SValue.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SValue.scala
@@ -3,13 +3,14 @@
 
 package com.digitalasset.daml.lf.speedy
 
+import java.util.ArrayList
+
 import com.digitalasset.daml.lf.data.Decimal.Decimal
-import com.digitalasset.daml.lf.data.{FrontStack, ImmArray, Time}
 import com.digitalasset.daml.lf.data.Ref._
+import com.digitalasset.daml.lf.data.{FrontStack, ImmArray, SortedLookupList, Time}
 import com.digitalasset.daml.lf.lfpackage.Ast._
 import com.digitalasset.daml.lf.speedy.SError.SErrorCrash
 import com.digitalasset.daml.lf.value.{Value => V}
-import java.util.ArrayList
 
 import scala.collection.JavaConverters._
 import scala.collection.immutable.HashMap
@@ -64,7 +65,7 @@ sealed trait SValue {
       case SOptional(mbV) =>
         V.ValueOptional(mbV.map(_.toValue))
       case SMap(mVal) =>
-        V.ValueMap(mVal.transform((_, v) => v.toValue))
+        V.ValueMap(SortedLookupList(mVal).mapValue(_.toValue))
       case SContractId(coid) =>
         V.ValueContractId(coid)
 
@@ -240,7 +241,7 @@ object SValue {
         SOptional(mbV.map(fromValue))
 
       case V.ValueMap(map) =>
-        SMap(map.transform((_, v) => fromValue(v)))
+        SMap(map.mapValue(fromValue).toHashMap)
 
       case V.ValueVariant(Some(id), variant, value) =>
         SVariant(id, variant, fromValue(value))

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/types/Ledger.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/types/Ledger.scala
@@ -1044,7 +1044,7 @@ object Ledger {
           })
         case ValueVariant(tycon, variant, value) =>
           ValueVariant(tycon, variant, rewrite(value))
-        case ValueList(vs) => ValueList(vs.map(rewrite(_)))
+        case ValueList(vs) => ValueList(vs.map(rewrite))
         case ValueContractId(coid) =>
           val acoid = contractIdToAbsoluteContractId(commitPrefix, coid)
           ValueContractId(acoid)
@@ -1057,7 +1057,7 @@ object Ledger {
         case vlit: ValueDate => vlit
         case ValueUnit => ValueUnit
         case ValueOptional(mbV) => ValueOptional(mbV.map(rewrite))
-        case ValueMap(map) => ValueMap(map.transform((_, v) => rewrite(v)))
+        case ValueMap(map) => ValueMap(map.mapValue(rewrite))
       }
     rewrite(value)
   }