interface support in the interface library #13154

S11001001 · 2022-03-03T19:52:30Z

This adds the presence of interface names, as well as lists of choices and their argument/return types for each interface.

It also changes the semantics of DefTemplate and EnvironmentInterface: each may contain "unresolved choices", which are the choices inherited from interfaces for each implementing template; EnvironmentInterface#resolveChoices will copy the full type information as a concrete choice for each implementing template, which should be much simpler for codegen and json-api.

A type generator should take account of the astInterfaces as well as typeDecls in EnvironmentInterface; interfaces aren't included in typeDecls because where I is an interface only ContractId I is serializable.

Unfortunately the name of the entity "interface" inconveniently clashes with the name of the larger concept the library deals with, "environment interface", so we refer to the former as "ast interface" or "def interface". So iface.Interface is the prior "interface of a dalf", iface.EnvironmentInterface is the "interface of a whole environment/dar", and iface.DefInterface is the new concept.

Fixes #13036.

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description
If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

- it seems to be more convenient for interface library users to *not* treat LF interfaces as InterfaceTypes, but that's not 100% clear yet

…ace-library-with-interfaces

…ce-library-with-interfaces

…ace-library-with-interfaces

CHANGELOG_BEGIN CHANGELOG_END

S11001001 · 2022-03-10T19:28:54Z

...-lf/interface/src/test/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReaderSpec.scala

+        }
+      }
+    }
+    lazy val itpEI = EnvironmentInterface.fromReaderInterfaces(itp).resolveChoices


Should fromReaderInterfaces just go ahead and resolveChoices here?

S11001001 · 2022-03-10T19:31:20Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/EnvironmentInterface.scala

+    astInterfaces: Map[Identifier, DefInterface.FWT],
+) {
+
+  // TODO(SC #13154) should fail instead in case of unresolved inherited choices?


This is about how codegen & json-api should detect and report (or just ignore!) unresolved inherited choices.

S11001001 · 2022-03-10T19:32:01Z

daml-lf/interface/BUILD.bazel

+daml_compile(
+    name = "InterfaceTestPackage",
+    srcs = ["src/test/daml/InterfaceTestPackage.daml"],
+    target = lf_version_configuration.get("dev"),  # TODO(SC) remove when interfaces in default


To spin off to a separate issue.

S11001001 · 2022-03-10T19:32:51Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/DefDataType.scala

-final case class DefTemplate[+Ty](choices: Map[Ref.Name, TemplateChoice[Ty]], key: Option[Ty]) {
+final case class DefTemplate[+Ty](
+    choices: Map[Ref.ChoiceName, TemplateChoice[Ty]],
+    inheritedChoices: Map[Ref.ChoiceName, Ref.TypeConName],


The "goal" is for this to be empty; I haven't provided any way to represent that phase transition for an EnvironmentInterface at the type level in this PR.

we should probably call this differently i.e. unresolvedInheritedChoices.

S11001001 · 2022-03-10T19:33:22Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/DefDataType.scala

+    * `astInterfaces` from an [[EnvironmentInterface]].  If the result has any
+    * `inheritedChoices` left, these choices were not found.
+    */
+  def resolveChoices[O >: Ty](


Possibly should be private as a utility for the function of same name in EnvironmentInterface.

I agree, however I think this could solve itself with resolving the issue that inheritedChoices should actually be empty in the end for the user.

It became apparent looking at #12689 that having EnvironmentInterface only as an entry point would be really inconvenient, hence the profusion of additional accessible utilities in this PR.

S11001001 · 2022-03-10T19:34:22Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/EnvironmentInterface.scala

+    *  l.resolveChoices |+| r.resolveChoices
+    *  // is possibly less well-resolved than
+    *  (l |+| r).resolveChoices


Moreover forall l, r, (l.resolveChoices |+| r.resolveChoices).resolveChoices = (l |+| r).resolveChoices.

S11001001 · 2022-03-10T19:38:16Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/EnvironmentInterface.scala

@@ -15,24 +15,60 @@ import scalaz.Semigroup
 final case class EnvironmentInterface(
    metadata: Map[PackageId, PackageMetadata],
    typeDecls: Map[Identifier, InterfaceType],
-)
+    astInterfaces: Map[Identifier, DefInterface.FWT],


Bikeshed away.

- noticed when checking how well this will go in PackageService

…ace-library-with-interfaces

S11001001 · 2022-03-15T16:23:43Z

One important thing to think about when considering this library is that it is beholden to its clients, not the LF format.

It was originally the internal ADT for representing LF signatures in the Scala codegen, so its choices of what to represent and what to leave out are largely driven by the needs of something like Scala codegen.

So the way to think about it is "what is a convenient ADT for a downstream application that needs to evaluate the external signatures in an LF file?"

realvictorprm

I think I've understood 95% of the changes, at least currently :D Left some comments to ensure that my understanding is correct but I'm still fine with the changes. Thanks for implementing this :)

realvictorprm · 2022-03-15T15:19:00Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/DefDataType.scala

+final case class DefInterface[+Ty](choices: Map[Ref.ChoiceName, TemplateChoice[Ty]])
+    extends DefTemplate.GetChoices[Ty]
+
+object DefInterface extends FWTLike[DefInterface]


So far I see we just add this to have a default alias called FWT on this type where the inner type is already set to Type.

realvictorprm · 2022-03-15T15:24:42Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/EnvironmentInterface.scala

+    val typeDecls = (i +: o).iterator.flatMap { case Interface(packageId, _, typeDecls, _) =>
      typeDecls mapKeys (Identifier(packageId, _))
    }.toMap
-    val metadata = (i +: o).iterator.flatMap { case Interface(packageId, metadata, _) =>
+    val astInterfaces = (i +: o).iterator.flatMap {
+      case Interface(packageId, _, _, astInterfaces) =>
+        astInterfaces mapKeys (Identifier(packageId, _))
+    }.toMap
+    val metadata = (i +: o).iterator.flatMap { case Interface(packageId, metadata, _, _) =>
      metadata.iterator.map(md => packageId -> md)
    }.toMap
-    EnvironmentInterface(metadata, typeDecls)
+    EnvironmentInterface(metadata, typeDecls, astInterfaces)


Why do we construct the sequence multiple times if we could just make e.g. a list from it? Is lazyness the reason?

def fromReaderInterfaces(i: Interface, o: Interface*): EnvironmentInterface = { val it = (i +: o).toList val typeDecls = it.flatMap { case Interface(packageId, _, typeDecls, _) => typeDecls mapKeys (Identifier(packageId, _)) }.toMap val astInterfaces = it.flatMap { case Interface(packageId, _, _, astInterfaces) => astInterfaces mapKeys (Identifier(packageId, _)) }.toMap val metadata = it.flatMap { case Interface(packageId, metadata, _, _) => metadata.iterator.map(md => packageId -> md) }.toMap EnvironmentInterface(metadata, typeDecls, astInterfaces) }

There was one, copied to two, ... 163ec7b

realvictorprm · 2022-03-15T15:40:13Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/Interface.scala

+    */
+  def resolveChoices(findInterface: PartialFunction[Identifier, DefInterface.FWT]): Interface = {
+    val outside = findInterface.lift
+    def findIface(id: Identifier) =


Noticed that resolveChoices actually expects Ref.TypeConName which however is nothing else than an alias to Identifier. Things are already difficult here due to domain we are in and this isn't making it much better 😅 Anyways wanted to note this to see what you think about it @S11001001.

I can change it to be universally one or the other (e: with respect to interface names as addressed in this PR only), LMK which one you think is better.

realvictorprm · 2022-03-15T16:14:22Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReader.scala

+              // ^ never actually used, as far as I can tell -SC
+              \/-(none)


So the DataInterface never is within the module defs but always separately listed in module.interfaces? Weird indeed then but how can we be sure?

We can be sure because prior to 895fa13 I was trying to use this code path on a real dar, and it was never hit.

I realized an alternative explanation: they are present, but marked serializable=false. Which would still mean this code path is never exercised, just for a slightly different reason.

Either way, defensively ignoring them at multiple levels works fine for the interface library's clients.

realvictorprm · 2022-03-15T16:25:38Z

...-lf/interface/src/test/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReaderSpec.scala

+    lazy val itpEI = EnvironmentInterface.fromReaderInterfaces(itp).resolveChoices
+
+    "load without errors" in {
+      itp shouldBe itp


interesting approach xD

Side effects!

realvictorprm · 2022-03-15T16:35:59Z

...-lf/interface/src/test/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReaderSpec.scala

+      itp.main.astInterfaces(TIf).choices get Useless should ===(Some(theUselessChoice))
+    }
+
+    def foundUselessChoice(foo: Option[InterfaceType]) = inside(foo) {


using inside for test failure if foo isn't of case Some isn't common to me but it works and I don't have a better proposal how to do this without too much boilerplate.

It is for sure the best way to do it, especially if you are doing further pattern matching of the contents therein (as we are here).

realvictorprm · 2022-03-15T17:01:22Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/DefDataType.scala

-final case class DefTemplate[+Ty](choices: Map[Ref.Name, TemplateChoice[Ty]], key: Option[Ty]) {
+final case class DefTemplate[+Ty](
+    choices: Map[Ref.ChoiceName, TemplateChoice[Ty]],
+    inheritedChoices: Map[Ref.ChoiceName, Ref.TypeConName],


we should probably call this differently i.e. unresolvedInheritedChoices.

realvictorprm · 2022-03-15T17:03:29Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/DefDataType.scala

+    * `astInterfaces` from an [[EnvironmentInterface]].  If the result has any
+    * `inheritedChoices` left, these choices were not found.
+    */
+  def resolveChoices[O >: Ty](


I agree, however I think this could solve itself with resolving the issue that inheritedChoices should actually be empty in the end for the user.

@realvictorprm

- suggested by @realvictorprm; thanks

@realvictorprm

- suggested by @realvictorprm; thanks

@realvictorprm

- suggested by @realvictorprm; thanks

…ace-library-with-interfaces

look up the full interface definition

fe79479

S11001001 self-assigned this Mar 3, 2022

S11001001 added the team/ledger-clients Related to the Ledger Clients team's components. label Mar 3, 2022

S11001001 added 27 commits March 3, 2022 19:35

create a proper dar for inspection in the interface library tests

942ae43

reformat

3e5eab3

test interface using the old syntax

ddfcea7

compile the test interface with 1.dev

378a924

new DefInterface for iface.Interface

8b7efe8

- it seems to be more convenient for interface library users to *not* treat LF interfaces as InterfaceTypes, but that's not 100% clear yet

build foldModule's state all at once

4b2557d

only pull the fixedChoices from the interfaces

196ac55

add the DefInterfaces to the Interface and EnvironmentInterface boxes

93cde92

remove state-stepping functions

9a67993

failing test that the choice made it all the way through

9e52766

Merge commit '1f1e50514bb3561afd0a333497a4ad1d581cddc1' into 13036-if…

05ebc96

…ace-library-with-interfaces

Merge commit '7bfd5e40c87187f61e94f952a65f7eafdab7245' into 13036-ifa…

b265fc9

…ce-library-with-interfaces

copy inheritedChoices with delayed lookup into DefTemplate

50cc32f

function to resolve inherited choices given an EnvironmentInterface

2a62879

document resolveChoices

8b2df63

test inheritedChoices' presence

4ba8add

Ast.DataInterface is empty, just use the interfaces map directly

895fa13

working test for the whole structure of the interface data

a855041

resolve inherited choices for the whole EnvironmentInterface

e876425

Merge commit 'e73963c88de98c6494cbbacb68370777b35855d3' into 13036-if…

3fb3ef9

…ace-library-with-interfaces

test inherited choice resolution

730346b

use value comparison to check the choice's structure instead

c975d22

port navigator tests for interface API changes

7459ac6

port json-api for interface API changes; does not solve #12689

cbd5d18

port Scala codegen for interface API changes; does not add support

d830be4

port Java codegen for interface API changes; does not solve #11350

3aac2e8

Merge commit 'e1030365f0870e7f1061a50377e2e336263593a6' into 13036-if…

82a233d

…ace-library-with-interfaces

S11001001 added 2 commits March 10, 2022 14:26

no changelog

feee1a8

CHANGELOG_BEGIN CHANGELOG_END

remove test println

4508dc3

S11001001 commented Mar 10, 2022

View reviewed changes

missed reformat

a98898e

S11001001 marked this pull request as ready for review March 10, 2022 19:54

S11001001 requested review from stefanobaghino-da, ray-roestenburg-da and remyhaemmerle-da as code owners March 10, 2022 19:54

S11001001 requested a review from realvictorprm March 10, 2022 19:54

S11001001 added component/daml-lf DAML-LF interfaces mvp labels Mar 10, 2022

S11001001 added this to the Daml Interfaces milestone Mar 10, 2022

variant of resolveChoices better for incrementally building environments

c960d4d

- noticed when checking how well this will go in PackageService

S11001001 mentioned this pull request Mar 15, 2022

remove target=dev from interface-specific test packages #13296

Open

6 tasks

S11001001 added 3 commits March 15, 2022 12:09

Merge commit '0ffb88a6ca3074082a6d539eec912d2f3fba55dd' into 13036-if…

534ca70

…ace-library-with-interfaces

doc typo

9531dd9

another utility for choice resolution

99ac8b4

realvictorprm approved these changes Mar 15, 2022

View reviewed changes

S11001001 added 6 commits March 15, 2022 13:18

rename inheritedChoices to unresolvedInheritedChoices

a75fe76

- suggested by @realvictorprm; thanks

seal a trait

00787e4

rename inheritedChoices to unresolvedInheritedChoices in doc

8751634

factor cons

163ec7b

- suggested by @realvictorprm; thanks

use Ref.TypeConName for interface names

09ccbb0

- suggested by @realvictorprm; thanks

Merge commit '739173bf8460e237d6bffb904bfb96077d2f80c2' into 13036-if…

524df88

…ace-library-with-interfaces

S11001001 enabled auto-merge (squash) March 15, 2022 18:09

S11001001 merged commit 0b5ad7a into main Mar 15, 2022

S11001001 deleted the 13036-iface-library-with-interfaces branch March 15, 2022 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

interface support in the interface library #13154

interface support in the interface library #13154

S11001001 commented Mar 3, 2022 •

edited

Loading

S11001001 Mar 10, 2022

S11001001 Mar 10, 2022

S11001001 Mar 10, 2022

S11001001 Mar 15, 2022

S11001001 Mar 10, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

S11001001 Mar 10, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

S11001001 Mar 10, 2022

S11001001 Mar 10, 2022

S11001001 commented Mar 15, 2022

realvictorprm left a comment

realvictorprm Mar 15, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022 •

edited

Loading

S11001001 Mar 15, 2022 •

edited

Loading

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 18, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

realvictorprm Mar 15, 2022

S11001001 Mar 15, 2022

realvictorprm Mar 15, 2022

realvictorprm Mar 15, 2022

interface support in the interface library #13154

interface support in the interface library #13154

Conversation

S11001001 commented Mar 3, 2022 • edited Loading

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S11001001 commented Mar 15, 2022

realvictorprm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S11001001 Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

S11001001 Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S11001001 commented Mar 3, 2022 •

edited

Loading

S11001001 Mar 15, 2022 •

edited

Loading

S11001001 Mar 15, 2022 •

edited

Loading