Revert ANF changes and add a testcase for evaluation order (#6645)

* Revert ANF changes and add a testcase for evaluation order After careful consideration, we decided that the change in evaluation order that was accidentally introduced by the ANF changes should be considered a breaking change or arguably even a bug and should not land in 1.3.0. Therefore, this PR reverts the following commits: 1. 353d0da 2. a45b510 3. 04c7b2a 4. a624dd7 5. b3aab72 Other PRs mostly had trivial merge conflicts that I resolved. The two most interesting ones here are probably 1. #6576 which was easy to resolve and the change to return SEValue instead of SExpr is still nice and useful even if we do not need the guarantees. 2. it #6542 which required some changes since the constructors changed. If you want to review those changes in detail (they are pretty straightforward so not too important), it’s probably easiest to check out this PR and run ``` git diff 2cd2a8f daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala ``` to see the diff to the parent commit of the first commit that introduced ANF. changelog_begin changelog_end
digital-asset · Jul 8, 2020 · 02c59d4 · 02c59d4
1 parent 9a135fa
commit 02c59d4
Show file tree

Hide file tree

Showing 26 changed files with 815 additions and 1,358 deletions.
diff --git a/compiler/damlc/tests/daml-test-files/EvaluationOrder.daml b/compiler/damlc/tests/daml-test-files/EvaluationOrder.daml
@@ -0,0 +1,12 @@
+-- Copyright (c) 2020, Digital Asset (Switzerland) GmbH and/or its affiliates.
+-- All rights reserved.
+
+-- @ERROR Aborted: BOOM
+
+module EvaluationOrder where
+
+boom = scenario do
+  let f x = error "BOOM"
+  let _ = f 1 (error "BANG")
+  let _ = f 1 2
+  pure ()
diff --git a/compiler/repl-service/server/src/main/scala/com/digitalasset/daml/lf/ReplServiceMain.scala b/compiler/repl-service/server/src/main/scala/com/digitalasset/daml/lf/ReplServiceMain.scala
@@ -12,7 +12,6 @@ import com.daml.lf.data.Ref._
 import com.daml.lf.engine.script._
 import com.daml.lf.language.Ast._
 import com.daml.lf.language.{Ast, LanguageVersion}
-import com.daml.lf.speedy.AExpr
 import com.daml.lf.speedy.SExpr._
 import com.daml.lf.speedy.{Compiler, SValue, SExpr, SError}
 import com.daml.grpc.adapter.{AkkaExecutionSequencerPool, ExecutionSequencerFactory}
@@ -191,7 +190,7 @@ class ReplService(
     mat: Materializer)
     extends ReplServiceGrpc.ReplServiceImplBase {
   var packages: Map[PackageId, Package] = Map.empty
-  var compiledDefinitions: Map[SDefinitionRef, AExpr] = Map.empty
+  var compiledDefinitions: Map[SDefinitionRef, SExpr] = Map.empty
   var results: Seq[SValue] = Seq()
   implicit val ec_ = ec
   implicit val esf_ = esf

diff --git a/...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Context.scala b/...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Context.scala
@@ -15,7 +15,7 @@ import com.daml.lf.speedy.Compiler
 import com.daml.lf.speedy.ScenarioRunner
 import com.daml.lf.speedy.SError._
 import com.daml.lf.speedy.Speedy
-import com.daml.lf.speedy.AExpr
+import com.daml.lf.speedy.SExpr
 import com.daml.lf.speedy.SValue
 import com.daml.lf.speedy.SExpr.{LfDefRef, SDefinitionRef}
 import com.daml.lf.transaction.TransactionVersions
@@ -54,10 +54,10 @@ class Context(val contextId: Context.ContextId) {
   val homePackageId: PackageId = PackageId.assertFromString("-homePackageId-")
 
   private var extPackages: Map[PackageId, Ast.Package] = HashMap.empty
-  private var extDefns: Map[SDefinitionRef, AExpr] = HashMap.empty
+  private var extDefns: Map[SDefinitionRef, SExpr] = HashMap.empty
   private var modules: Map[ModuleName, Ast.Module] = HashMap.empty
-  private var modDefns: Map[ModuleName, Map[SDefinitionRef, AExpr]] = HashMap.empty
-  private var defns: Map[SDefinitionRef, AExpr] = HashMap.empty
+  private var modDefns: Map[ModuleName, Map[SDefinitionRef, SExpr]] = HashMap.empty
+  private var defns: Map[SDefinitionRef, SExpr] = HashMap.empty
 
   def loadedModules(): Iterable[ModuleName] = modules.keys
   def loadedPackages(): Iterable[PackageId] = extPackages.keys
@@ -150,7 +150,7 @@ class Context(val contextId: Context.ContextId) {
     for {
       defn <- defns.get(LfDefRef(identifier))
     } yield
-      Speedy.Machine.fromScenarioAExpr(
+      Speedy.Machine.fromScenarioSExpr(
         compiledPackages,
         txSeeding,
         defn,

diff --git a/...lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ConcurrentCompiledPackages.scala b/...lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ConcurrentCompiledPackages.scala
@@ -18,15 +18,15 @@ import scala.collection.concurrent.{Map => ConcurrentMap}
 final class ConcurrentCompiledPackages extends MutableCompiledPackages {
   private[this] val _packages: ConcurrentMap[PackageId, Package] =
     new ConcurrentHashMap().asScala
-  private[this] val _defns: ConcurrentHashMap[speedy.SExpr.SDefinitionRef, speedy.AExpr] =
+  private[this] val _defns: ConcurrentHashMap[speedy.SExpr.SDefinitionRef, speedy.SExpr] =
     new ConcurrentHashMap()
   private[this] val _packageDeps: ConcurrentHashMap[PackageId, Set[PackageId]] =
     new ConcurrentHashMap()
   private[this] var _profilingMode: speedy.Compiler.ProfilingMode = speedy.Compiler.NoProfile
   private[this] var _stackTraceMode: speedy.Compiler.StackTraceMode = speedy.Compiler.FullStackTrace
 
   def getPackage(pId: PackageId): Option[Package] = _packages.get(pId)
-  def getDefinition(dref: speedy.SExpr.SDefinitionRef): Option[speedy.AExpr] =
+  def getDefinition(dref: speedy.SExpr.SDefinitionRef): Option[speedy.SExpr] =
     Option(_defns.get(dref))
 
   override def profilingMode = _profilingMode

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
@@ -8,7 +8,7 @@ import com.daml.lf.command._
 import com.daml.lf.data._
 import com.daml.lf.data.Ref.{PackageId, ParticipantId, Party}
 import com.daml.lf.language.Ast._
-import com.daml.lf.speedy.{InitialSeeding, Pretty}
+import com.daml.lf.speedy.{InitialSeeding, Pretty, SExpr}
 import com.daml.lf.speedy.Speedy.Machine
 import com.daml.lf.speedy.SResult._
 import com.daml.lf.transaction.{TransactionVersions, Transaction => Tx}
@@ -289,7 +289,8 @@ class Engine(config: Engine.Config) {
         compiledPackages = compiledPackages,
         submissionTime = submissionTime,
         initialSeeding = seeding,
-        anf = Machine.makeApplyToToken(compiledPackages.compiler.unsafeCompile(commands)),
+        expr = SExpr
+          .SEApp(compiledPackages.compiler.unsafeCompile(commands), Array(SExpr.SEValue.Token)),
         globalCids = globalCids,
         committers = submitters,
         supportedValueVersions = config.allowedOutputValueVersions,

diff --git a/daml-lf/interpreter/BUILD.bazel b/daml-lf/interpreter/BUILD.bazel
@@ -53,7 +53,6 @@ da_scala_test_suite(
         "@maven//:org_scalaz_scalaz_core_2_12",
         "@maven//:org_scalaz_scalaz_scalacheck_binding_2_12",
         "@maven//:org_slf4j_slf4j_api",
-        "@maven//:org_typelevel_paiges_core_2_12",
     ],
 )
 

diff --git a/daml-lf/interpreter/perf/BUILD.bazel b/daml-lf/interpreter/perf/BUILD.bazel
@@ -32,8 +32,6 @@ da_scala_binary(
     data = [
         ":Examples.dar",
         ":Examples.dar.pp",
-        ":JsonParser.dar",
-        ":JsonParser.dar.pp",
     ],
     main_class = "com.daml.lf.speedy.explore.ExploreDar",
     runtime_deps = [

diff --git a/daml-lf/interpreter/perf/src/main/scala/com/daml/lf/explore/Explore.scala b/daml-lf/interpreter/perf/src/main/scala/com/daml/lf/explore/Explore.scala
@@ -30,8 +30,8 @@ object PlaySpeedy {
 
     names.foreach { name =>
       val (expected, expr) = examples(name)
-      val anf = compiler.unsafeCompilationPipeline(expr)
-      val machine = Speedy.Machine.fromPureAExpr(noPackages, anf)
+      val converted = compiler.unsafeClosureConvert(expr)
+      val machine = Speedy.Machine.fromPureSExpr(noPackages, converted)
       runMachine(name, machine, expected)
     }
   }
@@ -84,7 +84,7 @@ object PlaySpeedy {
     }
   }
 
-  final case class MachineProblem(s: String) extends RuntimeException(s)
+  final case class MachineProblem(s: String) extends RuntimeException(s, null, false, false)
 
   def examples: Map[String, (Int, SExpr)] = {
 
@@ -98,8 +98,6 @@ object PlaySpeedy {
     def subtract2(x: SExpr, y: SExpr): SExpr = SEApp(SEBuiltin(SBSubInt64), Array(x, y))
     val subtract = SEAbs(2, subtract2(SEVar(2), SEVar(1)))
 
-    val identity = SEAbs(1, SEVar(1))
-
     def twice2(f: SExpr, x: SExpr): SExpr = SEApp(f, Array(SEApp(f, Array(x))))
     val twice = SEAbs(2, twice2(SEVar(2), SEVar(1)))
 
@@ -119,10 +117,6 @@ object PlaySpeedy {
         "subF", //88-55
         33,
         SEApp(subtract, Array(num(88), num(55)))),
-      (
-        "id-id", // id id 77
-        77,
-        SEApp(identity, Array(identity, num(77)))),
       (
         "thrice", // thrice (\x -> x - 1) 0
         -3,

diff --git a/daml-lf/interpreter/perf/src/main/scala/com/daml/lf/explore/ExploreDar.scala b/daml-lf/interpreter/perf/src/main/scala/com/daml/lf/explore/ExploreDar.scala
@@ -30,7 +30,6 @@ object PlaySpeedy {
   }
 
   def parseArgs(args0: List[String]): Config = {
-    var moduleName: String = "Examples"
     var funcName: String = "triangle"
     var argValue: Long = 10
     var stacktracing: Compiler.StackTraceMode = Compiler.NoStackTrace
@@ -44,19 +43,15 @@ object PlaySpeedy {
       case "--stacktracing" :: args =>
         stacktracing = Compiler.FullStackTrace
         loop(args)
-      case "--base" :: x :: args =>
-        moduleName = x
-        loop(args)
       case x :: args =>
         funcName = x
         loop(args)
     }
     loop(args0)
-    Config(moduleName, funcName, argValue, stacktracing)
+    Config(funcName, argValue, stacktracing)
   }
 
   final case class Config(
-      moduleName: String,
       funcName: String,
       argValue: Long,
       stacktracing: Compiler.StackTraceMode,
@@ -65,8 +60,8 @@ object PlaySpeedy {
   def main(args0: List[String]) = {
 
     println("Start...")
+    val base = "Examples"
     val config = parseArgs(args0)
-    val base = config.moduleName
     val dar = s"daml-lf/interpreter/perf/${base}.dar"
     val darFile = new File(rlocation(dar))
 

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/CompiledPackages.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/CompiledPackages.scala
@@ -6,18 +6,18 @@ package com.daml.lf
 import com.daml.lf.data.Ref.PackageId
 import com.daml.lf.language.Ast.Package
 import com.daml.lf.speedy.SExpr.SDefinitionRef
-import com.daml.lf.speedy.{Compiler, AExpr}
+import com.daml.lf.speedy.{Compiler, SExpr}
 
 /** Trait to abstract over a collection holding onto DAML-LF package definitions + the
   * compiled speedy expressions.
   */
 trait CompiledPackages {
   def getPackage(pkgId: PackageId): Option[Package]
-  def getDefinition(dref: SDefinitionRef): Option[AExpr]
+  def getDefinition(dref: SDefinitionRef): Option[SExpr]
 
   def packages: PartialFunction[PackageId, Package] = Function.unlift(this.getPackage)
   def packageIds: Set[PackageId]
-  def definitions: PartialFunction[SDefinitionRef, AExpr] =
+  def definitions: PartialFunction[SDefinitionRef, SExpr] =
     Function.unlift(this.getDefinition)
 
   def stackTraceMode: Compiler.StackTraceMode
@@ -28,13 +28,13 @@ trait CompiledPackages {
 
 final class PureCompiledPackages private (
     packages: Map[PackageId, Package],
-    defns: Map[SDefinitionRef, AExpr],
+    defns: Map[SDefinitionRef, SExpr],
     stacktracing: Compiler.StackTraceMode,
     profiling: Compiler.ProfilingMode,
 ) extends CompiledPackages {
   override def packageIds: Set[PackageId] = packages.keySet
   override def getPackage(pkgId: PackageId): Option[Package] = packages.get(pkgId)
-  override def getDefinition(dref: SDefinitionRef): Option[AExpr] = defns.get(dref)
+  override def getDefinition(dref: SDefinitionRef): Option[SExpr] = defns.get(dref)
   override def stackTraceMode = stacktracing
   override def profilingMode = profiling
 }
@@ -46,7 +46,7 @@ object PureCompiledPackages {
     */
   private[lf] def apply(
       packages: Map[PackageId, Package],
-      defns: Map[SDefinitionRef, AExpr],
+      defns: Map[SDefinitionRef, SExpr],
       stacktracing: Compiler.StackTraceMode,
       profiling: Compiler.ProfilingMode
   ): PureCompiledPackages =

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/AExpr.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/AExpr.scala