simplify, fix recursion in transaction equal instances (#903)

* Map case of Equal[Value] was not properly recursive

* protect SortedLookupList from equals incoherence

* simplify ImmArray creation

* clean up boilerplate in Equal[Value] definition

* use match2 for isReplayedBy
This commit is contained in:
Stephen Compall 2019-05-06 13:59:28 -04:00 committed by GitHub
parent 54c6e441f0
commit 78bf1b878c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 132 additions and 112 deletions

View File

@ -362,9 +362,7 @@ object ImmArray {
case ias: ImmArraySeq[T] => ias.toImmArray
case _ =>
val builder = ImmArray.newBuilder[T]
for (element <- elements) {
builder += element
}
builder ++= elements
builder.result()
}

View File

@ -8,4 +8,37 @@ import scalaz.Equal
private[lf] object ScalazEqual {
def withNatural[A](isNatural: Boolean)(c: (A, A) => Boolean): Equal[A] =
if (isNatural) Equal.equalA else Equal.equal(c)
/** Curry the typical pattern of matching equals by pairs, preserving exhaustiveness
* checking while reducing the boilerplate in each case.
*
* For example, this is unsafe:
*
* {{{
* (l, r) match {
* case (Left(l1), Left(l2)) => l1 == l2
* case (Right(r1), Right(r2)) => r1 == r2
* case _ => false
* }
* }}}
*
* because the third case disables exhaustiveness checking. And the easier
* it is to make this mistake, the stronger impulse to create the situation
* where it can occur, because the cost of writing out the false cases is
* quadratic.
*
* With this function, the above would be written
*
* {{{
* match2(fallback = false) {
* case Left(l1) => {case Left(l2) => l1 == l2}
* case Right(r1) => {case Right(r2) => r1 == r2}
* }
* }}}
*
* which preserves exhaustiveness checking for the left argument, which is
* perfectly sufficient for writing equals functions.
*/
def match2[A, B, C](fallback: C)(f: A => (B PartialFunction C))(a: A, b: B): C =
f(a).applyOrElse(b, (_: B) => fallback)
}

View File

@ -3,10 +3,15 @@
package com.digitalasset.daml.lf.data
import scalaz.Equal
import scalaz.std.string._
import scalaz.std.tuple._
import scalaz.syntax.equal._
import scala.collection.immutable.HashMap
/** We use this container to pass around DAML-LF maps as flat lists in various parts of the codebase. */
class SortedLookupList[+X] private (entries: ImmArray[(String, X)]) {
final class SortedLookupList[+X] private (entries: ImmArray[(String, X)]) extends Equals {
def mapValue[Y](f: X => Y) = new SortedLookupList(entries.map { case (k, v) => k -> f(v) })
@ -18,9 +23,11 @@ class SortedLookupList[+X] private (entries: ImmArray[(String, X)]) {
def toHashMap: HashMap[String, X] = HashMap(entries.toSeq: _*)
override def canEqual(that: Any): Boolean = that.isInstanceOf[SortedLookupList[_]]
override def equals(obj: Any): Boolean = {
obj match {
case other: SortedLookupList[X] => other.toImmArray == entries
case other: SortedLookupList[X] if other canEqual this => other.toImmArray == entries
case _ => false
}
}
@ -61,4 +68,9 @@ object SortedLookupList {
def empty[X]: SortedLookupList[X] = new SortedLookupList(ImmArray.empty)
implicit def `SLL Equal instance`[X: Equal]: Equal[SortedLookupList[X]] =
ScalazEqual.withNatural(Equal[X].equalIsNatural) { (self, other) =>
self.toImmArray === other.toImmArray
}
}

View File

@ -169,63 +169,56 @@ object Node {
private[transaction] def isReplayedBy[Cid: Equal, Val: Equal](
recorded: GenNode[Nothing, Cid, Val],
isReplayedBy: GenNode[Nothing, Cid, Val]): Boolean =
recorded match {
case nc: NodeCreate[Cid, Val] =>
isReplayedBy match {
case NodeCreate(coid2, coinst2, optLocation2 @ _, signatories2, stakeholders2, key2) =>
import nc._
// NOTE(JM): Do not compare location annotations as they may differ due to
// differing update expression constructed from the root node.
coid === coid2 && coinst === coinst2 &&
signatories == signatories2 && stakeholders == stakeholders2 && key === key2
case _ => false
}
case nf: NodeFetch[Cid] =>
isReplayedBy match {
case NodeFetch(
coid2,
templateId2,
optLocation2 @ _,
actingParties2,
signatories2,
stakeholders2) =>
import nf._
coid === coid2 && templateId == templateId2 &&
actingParties.forall(_ => actingParties == actingParties2) &&
signatories == signatories2 && stakeholders == stakeholders2
case _ => false
}
case ne: NodeExercises[Nothing, Cid, Val] =>
isReplayedBy match {
case NodeExercises(
targetCoid2,
templateId2,
choiceId2,
optLocation2 @ _,
consuming2,
actingParties2,
chosenValue2,
stakeholders2,
signatories2,
controllers2,
_,
exerciseResult2) =>
import ne._
targetCoid === targetCoid2 && templateId == templateId2 && choiceId == choiceId2 &&
consuming == consuming2 && actingParties == actingParties2 && chosenValue === chosenValue2 &&
stakeholders == stakeholders2 && signatories == signatories2 && controllers == controllers2 &&
exerciseResult.fold(true)(_ => exerciseResult === exerciseResult2)
case _ => false
}
case nl: NodeLookupByKey[Cid, Val] =>
isReplayedBy match {
case NodeLookupByKey(templateId2, optLocation2 @ _, key2, result2) =>
import nl._
templateId == templateId2 &&
key === key2 && result === result2
case _ => false
}
}
ScalazEqual.match2[recorded.type, isReplayedBy.type, Boolean](fallback = false) {
case nc: NodeCreate[Cid, Val] => {
case NodeCreate(coid2, coinst2, optLocation2 @ _, signatories2, stakeholders2, key2) =>
import nc._
// NOTE(JM): Do not compare location annotations as they may differ due to
// differing update expression constructed from the root node.
coid === coid2 && coinst === coinst2 &&
signatories == signatories2 && stakeholders == stakeholders2 && key === key2
case _ => false
}
case nf: NodeFetch[Cid] => {
case NodeFetch(
coid2,
templateId2,
optLocation2 @ _,
actingParties2,
signatories2,
stakeholders2) =>
import nf._
coid === coid2 && templateId == templateId2 &&
actingParties.forall(_ => actingParties == actingParties2) &&
signatories == signatories2 && stakeholders == stakeholders2
}
case ne: NodeExercises[Nothing, Cid, Val] => {
case NodeExercises(
targetCoid2,
templateId2,
choiceId2,
optLocation2 @ _,
consuming2,
actingParties2,
chosenValue2,
stakeholders2,
signatories2,
controllers2,
_,
exerciseResult2) =>
import ne._
targetCoid === targetCoid2 && templateId == templateId2 && choiceId == choiceId2 &&
consuming == consuming2 && actingParties == actingParties2 && chosenValue === chosenValue2 &&
stakeholders == stakeholders2 && signatories == signatories2 && controllers == controllers2 &&
exerciseResult.fold(true)(_ => exerciseResult === exerciseResult2)
}
case nl: NodeLookupByKey[Cid, Val] => {
case NodeLookupByKey(templateId2, optLocation2 @ _, key2, result2) =>
import nl._
templateId == templateId2 &&
key === key2 && result === result2
}
}(recorded, isReplayedBy)
/** Useful in various circumstances -- basically this is what a ledger implementation must use as
* a key.

View File

@ -197,56 +197,40 @@ object Value {
final case class ValueTuple[+Cid](fields: ImmArray[(String, Value[Cid])]) extends Value[Cid]
implicit def `Value Equal instance`[Cid: Equal]: Equal[Value[Cid]] =
ScalazEqual.withNatural(Equal[Cid].equalIsNatural) { (a, b) =>
a match {
case _: ValueInt64 | _: ValueDecimal | _: ValueText | _: ValueTimestamp | _: ValueParty |
_: ValueBool | _: ValueDate | ValueUnit =>
a == b
case r: ValueRecord[Cid] =>
b match {
case ValueRecord(tycon2, fields2) =>
import r._
tycon == tycon2 && fields === fields2
case _ => false
}
case v: ValueVariant[Cid] =>
b match {
case ValueVariant(tycon2, variant2, value2) =>
import v._
tycon == tycon2 && variant == variant2 && value === value2
case _ => false
}
case ValueContractId(value) =>
b match {
case ValueContractId(value2) =>
value === value2
case _ => false
}
case ValueList(values) =>
b match {
case ValueList(values2) =>
values === values2
case _ => false
}
case ValueOptional(value) =>
b match {
case ValueOptional(value2) =>
value === value2
case _ => false
}
case ValueTuple(fields) =>
b match {
case ValueTuple(fields2) =>
fields === fields2
case _ => false
}
case ValueMap(map1) =>
b match {
case ValueMap(map2) =>
map1 == map2
case _ =>
false
}
ScalazEqual.withNatural(Equal[Cid].equalIsNatural) {
ScalazEqual.match2(fallback = false) {
case a @ (_: ValueInt64 | _: ValueDecimal | _: ValueText | _: ValueTimestamp |
_: ValueParty | _: ValueBool | _: ValueDate | ValueUnit) => { case b => a == b }
case r: ValueRecord[Cid] => {
case ValueRecord(tycon2, fields2) =>
import r._
tycon == tycon2 && fields === fields2
}
case v: ValueVariant[Cid] => {
case ValueVariant(tycon2, variant2, value2) =>
import v._
tycon == tycon2 && variant == variant2 && value === value2
}
case ValueContractId(value) => {
case ValueContractId(value2) =>
value === value2
}
case ValueList(values) => {
case ValueList(values2) =>
values === values2
}
case ValueOptional(value) => {
case ValueOptional(value2) =>
value === value2
}
case ValueTuple(fields) => {
case ValueTuple(fields2) =>
fields === fields2
}
case ValueMap(map1) => {
case ValueMap(map2) =>
map1 === map2
}
}
}