Address Moritz's review

digital-asset · Jun 16, 2021 · 10abb2a · 10abb2a
1 parent 2a74893
commit 10abb2a
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 23 deletions.
diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala
@@ -238,15 +238,18 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
 
   @inline
   private[lf] def runSafely[X](
-      funcName: String,
-      handleMissingDependencies: => Result[Unit],
+      funcName: String
   )(run: => Result[X]): Result[X] = {
     def start: Result[X] =
       try {
         run
       } catch {
-        case speedy.Compiler.PackageNotFound(_) =>
-          handleMissingDependencies.flatMap(_ => start)
+        // The two following error should be prevented by the type checking does by translateCommand
+        // so it’s an internal error.
+        case error: speedy.Compiler.PackageNotFound |  =>
+          ResultError(
+            Error.Preprocessing.Internal(funcName, s"CompilationError: ${error.getMessage}")
+          )
         case speedy.Compiler.CompilationError(error) =>
           ResultError(Error.Preprocessing.Internal(funcName, s"CompilationError: $error"))
       }
@@ -270,10 +273,7 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
       seeding: speedy.InitialSeeding,
       globalCids: Set[Value.ContractId],
   ): Result[(SubmittedTransaction, Tx.Metadata)] =
-    runSafely(
-      NameOf.qualifiedNameOfCurrentFunc,
-      loadPackages(commands.foldLeft(Set.empty[PackageId])(_ + _.templateId.packageId).toList),
-    ) {
+    runSafely(NameOf.qualifiedNameOfCurrentFunc) {
       val sexpr = compiledPackages.compiler.unsafeCompile(commands)
       val machine = Machine(
         compiledPackages = compiledPackages,

diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Error.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Error.scala
@@ -99,7 +99,7 @@ object Error {
         }
     }
 
-    final case class Type(
+    final case class TypeMismatch(
         typ: Ast.Type,
         value: Value[Value.ContractId],
         override val msg: String,
@@ -112,6 +112,8 @@ object Error {
 
     final case class RootNode(nodeId: NodeId, override val msg: String) extends Error
 
+    // TODO https://github.com/digital-asset/daml/issues/9974
+    //  get ride of ContractIdFreshness
     final case class ContractIdFreshness(
         localContractIds: Set[Value.ContractId],
         globalContractIds: Set[Value.ContractId],

diff --git a/...rc/main/scala/com/digitalasset/daml/lf/engine/preprocessing/TransactionPreprocessor.scala b/...rc/main/scala/com/digitalasset/daml/lf/engine/preprocessing/TransactionPreprocessor.scala
@@ -33,7 +33,7 @@ private[preprocessing] final class TransactionPreprocessor(
     )
   }
 
-  private[this] def invalideRootNode(nodeId: NodeId, message: String) =
+  private[this] def invalidRootNode(nodeId: NodeId, message: String) =
     throw Error.Preprocessing.RootNode(nodeId, message)
 
   /*
@@ -115,14 +115,14 @@ private[preprocessing] final class TransactionPreprocessor(
               val newLocalCids = GenTransaction(tx.nodes, ImmArray(id)).localContracts.keys
               acc.update(newCids, newLocalCids, cmd)
             case _: Node.NodeFetch[_] =>
-              invalideRootNode(id, s"Transaction contains a fetch root node $id")
+              invalidRootNode(id, s"Transaction contains a fetch root node $id")
             case _: Node.NodeLookupByKey[_] =>
-              invalideRootNode(id, s"Transaction contains a lookup by key root node $id")
+              invalidRootNode(id, s"Transaction contains a lookup by key root node $id")
           }
         case Some(_: Node.NodeRollback[NodeId]) =>
-          invalideRootNode(id, s"invalid transaction, root refers to a rollback node $id")
+          invalidRootNode(id, s"invalid transaction, root refers to a rollback node $id")
         case None =>
-          invalideRootNode(id, s"invalid transaction, root refers to non-existing node $id")
+          invalidRootNode(id, s"invalid transaction, root refers to non-existing node $id")
       }
     }
 

diff --git a/...engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala b/...engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala
@@ -54,7 +54,7 @@ private[engine] final class ValueTranslator(interface: language.Interface) {
       } else {
         val newNesting = nesting + 1
         def typeError(msg: String = s"mismatching type: $ty and value: $value0") =
-          throw Error.Preprocessing.Type(ty, value0, msg)
+          throw Error.Preprocessing.TypeMismatch(ty, value0, msg)
         val (ty1, tyArgs) = AstUtil.destructApp(ty0)
         ty1 match {
           case TBuiltin(bt) =>

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
@@ -222,7 +222,7 @@ class EngineTest
         .preprocessCommands(ImmArray(command))
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
 
@@ -301,7 +301,7 @@ class EngineTest
         .preprocessCommands(ImmArray(command))
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
         error.msg should startWith("Missing record label n for record")
       }
     }
@@ -336,7 +336,7 @@ class EngineTest
         .preprocessCommands(ImmArray(command))
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
 
@@ -402,7 +402,7 @@ class EngineTest
         .preprocessCommands(ImmArray(command))
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
 
@@ -423,7 +423,7 @@ class EngineTest
         .preprocessCommands(ImmArray(command))
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
 
@@ -469,7 +469,7 @@ class EngineTest
         )
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
   }
@@ -1257,7 +1257,7 @@ class EngineTest
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
 
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }
 
@@ -1299,7 +1299,7 @@ class EngineTest
         .consume(lookupContract, lookupPackage, lookupKey, allKeysVisible)
 
       inside(res) { case Left(Error.Preprocessing(error)) =>
-        error shouldBe a[Error.Preprocessing.Type]
+        error shouldBe a[Error.Preprocessing.TypeMismatch]
       }
     }