From f8933d1f4bc6ed56eea3e98507b1ca02a7cb8ad3 Mon Sep 17 00:00:00 2001 From: Sofia Faro Date: Mon, 22 Nov 2021 13:27:48 +0000 Subject: [PATCH] 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 --- .../daml/lf/engine/ValueEnricher.scala | 9 +++---- .../daml/lf/engine/InterfacesTest.scala | 12 +++------ .../daml/lf/language/PackageInterface.scala | 26 +++++++++---------- .../daml/lf/engine/script/ScriptF.scala | 2 +- 4 files changed, 21 insertions(+), 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 index 0a22a7353c26..6b4f11b7ce98 100644 --- 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 index d29a876daf60..7ca1c8e73d94 100644 --- 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 index 9992135c606e..b247ebeac54e 100644 --- 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 index 09bb365e84d6..59390d02962e 100644 --- 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 {