Make lookupTemplateChoice return only choices in the template. (#11808)

This fixes a bug in the typechecker (#11558) and the command preprocessor, since those were written with this behavior of lookupTemplateChoice in mind. Enables the engine test that caught this. changelog_begin changelog_end
digital-asset · Nov 22, 2021 · f8933d1 · f8933d1
1 parent 3e0ac71
commit f8933d1
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 28 deletions.
diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala
@@ -85,17 +85,16 @@ final class ValueEnricher(
       choiceName: Name,
       value: Value,
   ): Result[Value] =
-    handleLookup(interface.lookupTemplateChoice(tyCon, choiceName))
-      .flatMap(choice => enrichValue(choice.argBinder._2, value))
+    handleLookup(interface.lookupChoice(tyCon, choiceName))
+      .flatMap(choiceInfo => enrichValue(choiceInfo.choice.argBinder._2, value))
 
   def enrichChoiceResult(
       tyCon: Identifier,
       choiceName: Name,
       value: Value,
   ): Result[Value] =
-    handleLookup(interface.lookupTemplateChoice(tyCon, choiceName)).flatMap(choice =>
-      enrichValue(choice.returnType, value)
-    )
+    handleLookup(interface.lookupChoice(tyCon, choiceName))
+      .flatMap(choiceInfo => enrichValue(choiceInfo.choice.returnType, value))
 
   def enrichContractKey(tyCon: Identifier, value: Value): Result[Value] =
     handleLookup(interface.lookupTemplateKey(tyCon))

diff --git a/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/InterfacesTest.scala b/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/InterfacesTest.scala
@@ -192,14 +192,10 @@ class InterfacesTest
       val command = ExerciseTemplateCommand(idI1, cid1, "C1", ValueRecord(None, ImmArray.empty))
       preprocess(command) shouldBe a[Left[_, _]]
     }
-    // TODO https://github.com/digital-asset/daml/issues/11558
-    //   Fix lookupTemplateChoice to not return inherited choices.
-    /*
-      "be unable to exercise T1 inherited choice via exercise template (stopped in preprocessor)" in {
-        val command = ExerciseTemplateCommand(idT1, cid1, "C1", ValueRecord(None, ImmArray.empty))
-        preprocess(command) shouldBe a[Left[_, _]]
-      }
-     */
+    "be unable to exercise T1 inherited choice via exercise template (stopped in preprocessor)" in {
+      val command = ExerciseTemplateCommand(idT1, cid1, "C1", ValueRecord(None, ImmArray.empty))
+      preprocess(command) shouldBe a[Left[_, _]]
+    }
     "be able to exercise T1 own choice via exercise template" in {
       val command =
         ExerciseTemplateCommand(idT1, cid1, "OwnChoice", ValueRecord(None, ImmArray.empty))

diff --git a/daml-lf/language/src/main/scala/com/digitalasset/daml/lf/language/PackageInterface.scala b/daml-lf/language/src/main/scala/com/digitalasset/daml/lf/language/PackageInterface.scala
@@ -192,27 +192,22 @@ private[lf] class PackageInterface(signatures: PartialFunction[PackageId, Packag
   def lookupInterface(name: TypeConName): Either[LookupError, DefInterfaceSignature] =
     lookupInterface(name, Reference.Interface(name))
 
+  /** Look up a template's choice by name.
+    * This purposefully does not return choices inherited via interfaces.
+    * Use lookupChoice for a more flexible lookup.
+    */
   private[this] def lookupTemplateChoice(
       tmpName: TypeConName,
       chName: ChoiceName,
       context: => Reference,
   ): Either[LookupError, TemplateChoiceSignature] =
     lookupTemplate(tmpName, context).flatMap(template =>
-      template.choices.get(chName) match {
-        case Some(choice) => Right(choice)
-        case None =>
-          template.inheritedChoices.get(chName) match {
-            case None => Left(LookupError(Reference.TemplateChoice(tmpName, chName), context))
-            case Some(ifaceName) =>
-              lookupInterface(ifaceName, context).flatMap(iface =>
-                iface.fixedChoices
-                  .get(chName)
-                  .toRight(LookupError(Reference.TemplateChoice(ifaceName, chName), context))
-              )
-          }
-      }
+      template.choices
+        .get(chName)
+        .toRight(LookupError(Reference.TemplateChoice(tmpName, chName), context))
     )
 
+  /** Look up a template's own choice. Does not return choices inherited via interfaces. */
   def lookupTemplateChoice(
       tmpName: TypeConName,
       chName: ChoiceName,
@@ -441,7 +436,10 @@ object PackageInterface {
   // - iden refers to an interface that defines a choice chName
   // - iden refers to a template that defines a choice chName
   // - iden refers to a template that inherits from a interface than defined chName
-  sealed trait ChoiceInfo extends Serializable with Product
+  sealed trait ChoiceInfo extends Serializable with Product {
+    val choice: TemplateChoiceSignature
+  }
+
   object ChoiceInfo {
 
     final case class Interface(choice: TemplateChoiceSignature) extends ChoiceInfo

diff --git a/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/ScriptF.scala b/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/ScriptF.scala
@@ -76,7 +76,7 @@ object ScriptF {
         _clients.copy(party_participants = _clients.party_participants + (party -> participant))
     }
     def lookupChoice(id: Identifier, choice: Name): Either[String, Ast.TemplateChoiceSignature] =
-      compiledPackages.interface.lookupTemplateChoice(id, choice).left.map(_.pretty)
+      compiledPackages.interface.lookupChoice(id, choice).map(_.choice).left.map(_.pretty)
 
     def lookupKeyTy(id: Identifier): Either[String, Ast.Type] =
       compiledPackages.interface.lookupTemplateKey(id) match {