Skip to content

Commit

Permalink
Handle sums-of-products correctly in daml ledger export (digital-as…
Browse files Browse the repository at this point in the history 
…set#15152)

* Extract encodeVariant

* Add special case for encoding record members of variants

* Test special case for encoding record members of variants

* Test variant records in 'daml ledger export' golden test 'export-values'

* Update TODOs

* Add comments for untested cases in export-values golden test

changelog_begin
- [Daml export] Fixed an issue with the handling of sums-of-products (digital-asset#14723)
changelog_end
  • Loading branch information
@akrmn
akrmn authored Oct 4, 2022
1 parent ae037ab commit b26eab3
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 48 deletions.
11 changes: 9 additions & 2 deletions daml-script/export/integration-tests/golden/export-values/Export.daml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,17 @@ export Args{parties, contracts} = do
intOpts = [None, (Some 77), None, (Some 78)], intListOpts = [None,
(Some []), (Some [79]), (Some [80, 81, 82])],
variants = [(Values.VarInt 83), (Values.VarText "84"),
(Values.VarRec (Values.Record {int = 85, text = "86", bool = True}))],
(Values.VarRec (Values.Record {int = 85, text = "86", bool = True})),
(Values.VarFields {rec = (Values.Record {int = 87, text = "88",
bool = True}), int = 89, text = "90"})],
hkVariants = [(Values.HkVarInt 91), (Values.HkVarText "92"),
(Values.HkVarAye Values.Cee), (Values.HkVarBee 93),
(Values.HkVarCee ()), (Values.HkVarRec (Values.HkRecord {int = 94,
text = "95", bool = True, aye = Values.Bee, bee = 96,
ayeBees = [(Values.Aye, 97), (Values.Cee, 98)]}))]})
ayeBees = [(Values.Aye, 97), (Values.Cee, 98)]})),
(Values.HkVarFields {hkRec = (Values.HkRecord {int = 99, text = "100",
bool = True, aye = Values.Dee, bee = 101,
ayeBees = [(Values.Bee, 102), (Values.Cee, 103)]}), int = 104,
text = "105", ayeBeeCees = [(Values.Dee, 106, ()), (Values.Aye,
107, ())]})]})
pure ()
63 changes: 25 additions & 38 deletions daml-script/export/integration-tests/golden/export-values/Values.daml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ data Variant
-}
| VarInt Int
| VarText Text
{- The compiler fails with error
> Constructors with multiple fields must give explicit field names,
> e.g. Foo with bar : Int; baz : Int
so we don't test this case.
| VarIntText Int Text
-}
| VarRec Record
| VarFields with
rec : Record
Expand All @@ -87,11 +93,24 @@ data Variant

data HkVariant a b c
= HkVarUnit
{- The compiler warns
> Variant type HkVariant constructor HkVarExplicitUnit has a single argument
> of type (). The argument will not be preserved when importing this
> package via data-dependencies.
so we don't test this case.
| HkVarExplicitUnit ()
-}
| HkVarInt Int
| HkVarText Text
| HkVarAye a
| HkVarBee b
| HkVarCee c
{- The compiler fails with error
> Constructors with multiple fields must give explicit field names,
> e.g. Foo with bar : Int; baz : Int
so we don't test this case.
| VarIntTextAyeBee Int Text a b
-}
| HkVarRec (HkRecord a b)
| HkVarFields with
hkRec : HkRecord a b
Expand Down Expand Up @@ -153,49 +172,35 @@ initializeWith LedgerParties {bank} = do
intListOpts = [None, Some [], Some [79], Some [80, 81, 82]]
variants =
[ {-
VarUnit -- TODO: Handle sums of products properly
VarUnit -- TODO: Handle variant without arguments properly
-- Currently exported with a spurious argument as
-- ```
-- Values.VarUnit ()`
-- ```
-- https://github.com/digital-asset/daml/issues/14723
-- https://github.com/digital-asset/daml/issues/15153
, -}
VarInt 83
, VarText "84"
, VarRec (Record with
int = 85
text = "86"
bool = True)
{-
, (VarFields with -- TODO: Handle sums of products properly
-- Currently exported with a spurious record
-- constructor `Values.Variant.VarFields` as
-- ```
-- (Values.VarFields (Values.Variant.VarFields
-- { rec = (Values.Record
-- { int = 87
-- , text = "88"
-- , bool = True })
-- , int = 89
-- , text = "90" }))
-- ```
-- https://github.com/digital-asset/daml/issues/14723
, (VarFields with
rec = Record with
int = 87
text = "88"
bool = True
int = 89
text = "90")
-}
]
hkVariants =
[ {-
HkVarUnit -- TODO: Handle sums of products properly
HkVarUnit -- TODO: Handle variant without arguments properly
-- Currently exported with a spurious argument as
-- ```
-- Values.HkVarUnit ()`
-- ```
-- https://github.com/digital-asset/daml/issues/14723
-- https://github.com/digital-asset/daml/issues/15153
, -}
HkVarInt 91
, HkVarText "92"
Expand All @@ -209,24 +214,7 @@ initializeWith LedgerParties {bank} = do
aye = Bee
bee = 96
ayeBees = [(Aye, 97), (Cee, 98)])
{-
, (HkVarFields with -- TODO: Handle sums of products properly
-- Currently exported with a spurious record
-- constructor `Values.HkVariant.HkVarFields` as
-- ```
-- (Values.HkVarFields (Values.HkVariant.HkVarFields
-- { hkRec = (Values.HkRecord
-- { int = 99
-- , text = "100"
-- , bool = True
-- , aye = Values.Dee
-- , bee = 101
-- , ayeBees = [(Values.Bee, 102), (Values.Cee, 103)]})
-- , int = 104
-- , text = "105"
-- , ayeBeeCees = [(Values.Dee, 106, ()), (Values.Aye, 107, ())]}))
-- ```
-- https://github.com/digital-asset/daml/issues/14723
, (HkVarFields with
hkRec = HkRecord with
int = 99
text = "100"
Expand All @@ -237,7 +225,6 @@ initializeWith LedgerParties {bank} = do
int = 104
text = "105"
ayeBeeCees = [(Dee, 106, ()), (Aye, 107, ())])
-}
]
pure ()

Expand Down
36 changes: 28 additions & 8 deletions daml-script/export/src/main/scala/com/daml/script/export/Encode.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import java.time.{LocalDate, ZoneId, ZonedDateTime}
import com.daml.ledger.api.refinements.ApiTypes.{Choice, ContractId, Party, TemplateId}
import com.daml.ledger.api.v1.event.CreatedEvent
import com.daml.ledger.api.v1.value.Value.Sum
import com.daml.ledger.api.v1.value.{Identifier, Record, RecordField, Value}
import com.daml.ledger.api.v1.value.{Identifier, Record, RecordField, Value, Variant}
import com.daml.lf.data.Ref
import com.daml.lf.data.Time.{Date, Timestamp}
import com.daml.lf.language.Ast
Expand Down Expand Up @@ -190,13 +190,7 @@ private[export] object Encode {
case Sum.Record(value) if isTupleId(value.getRecordId) =>
tuple(value.fields.map(f => go(f.getValue.sum)))
case Sum.Record(value) => encodeRecord(partyMap, cidMap, value)
// TODO Handle sums of products properly
// https://github.com/digital-asset/daml/issues/14723
case Sum.Variant(value) =>
parens(
qualifyId(value.getVariantId.copy(entityName = value.constructor)) +
Doc.text(" ") + go(value.getValue.sum)
)
case Sum.Variant(value) => encodeVariant(partyMap, cidMap, value)
case Sum.ContractId(c) => encodeCid(cidMap, ContractId(c))
case Sum.List(value) =>
list(value.elements.map(v => go(v.sum)))
Expand Down Expand Up @@ -361,6 +355,32 @@ private[export] object Encode {

}

private def variantFieldsRecordId(
variant: Variant
): Identifier =
variant.getVariantId.withEntityName(
variant.getVariantId.entityName + "." + variant.constructor
)

private def encodeVariant(
partyMap: Map[Party, String],
cidMap: Map[ContractId, String],
v: Variant,
): Doc = {
val constrId = v.getVariantId.copy(entityName = v.constructor)
v.getValue.sum match {
case Sum.Record(rec) if rec.getRecordId == variantFieldsRecordId(v) =>
// The record carries the fields of a variant constructor
// declared with record syntax, so we encode directly as a record.
encodeRecord(partyMap, cidMap, rec.withRecordId(constrId))
case _ =>
parens(
qualifyId(constrId) +
Doc.text(" ") + encodeValue(partyMap, cidMap, v.getValue.sum)
)
}
}

private def encodeField(
partyMap: Map[Party, String],
cidMap: Map[ContractId, String],
Expand Down
16 changes: 16 additions & 0 deletions daml-script/export/src/test/scala/com/daml/script/export/EncodeValueSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ class EncodeValueSpec extends AnyFreeSpec with Matchers {
val variant =
v.Value().withVariant(v.Variant(Some(id), "Constr", Some(v.Value().withInt64(1))))
encodeValue(Map.empty, Map.empty, variant.sum).render(80) shouldBe "(M.Constr 1)"

// Tests a variant constructor declared with record syntax
val fields = record.withRecordId(id.withEntityName("V.ConstrFields"))
val variantFields =
v.Value()
.withVariant(v.Variant(Some(id), "ConstrFields", Some(v.Value().withRecord(fields))))
encodeValue(Map.empty, Map.empty, variantFields.sum).render(
80
) shouldBe "(M.ConstrFields {a = 1, b = (M.R2 {c = 42}), c = M.R3})"

// Tests a variant constructor declared with an argument of a record type.
val variantRec =
v.Value().withVariant(v.Variant(Some(id), "ConstrRec", Some(v.Value().withRecord(record))))
encodeValue(Map.empty, Map.empty, variantRec.sum).render(
80
) shouldBe "(M.ConstrRec (M.R1 {a = 1, b = (M.R2 {c = 42}), c = M.R3}))"
}
"contract id" in {
val cid = v.Value().withContractId("my-contract-id")
Expand Down

0 comments on commit b26eab3

Please sign in to comment.