From 86deeb1baf8dc2073294e92a092fb940a4236ee7 Mon Sep 17 00:00:00 2001 From: David Strawn Date: Tue, 21 Dec 2021 10:06:51 -0700 Subject: [PATCH 1/4] Add Order and PartialOrder For SortedMap --- .../scala/cats/kernel/laws/LawTests.scala | 4 ++ kernel/src/main/scala/cats/kernel/Eq.scala | 14 ++++++- .../kernel/instances/SortedMapInstances.scala | 39 ++++++++++++++++++- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala b/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala index 93fd7b39a6..d3e925645b 100644 --- a/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala +++ b/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala @@ -148,6 +148,9 @@ class Tests extends TestsConfig with DisciplineSuite { checkAll("PartialOrder[Vector[HasPartialOrder[Int]]]", PartialOrderTests[Vector[HasPartialOrder[Int]]].partialOrder) checkAll("PartialOrder[Stream[HasPartialOrder[Int]]]", PartialOrderTests[Stream[HasPartialOrder[Int]]].partialOrder) checkAll("PartialOrder[Queue[HasPartialOrder[Int]]]", PartialOrderTests[Queue[HasPartialOrder[Int]]].partialOrder) + checkAll("PartialOrder[SortedMap[Int, HasPartialOrder[Int]]]", + PartialOrderTests[SortedMap[Int, HasPartialOrder[Int]]].partialOrder + ) checkAll("Semilattice.asMeetPartialOrder[Set[Int]]", PartialOrderTests(Semilattice.asMeetPartialOrder[Set[Int]]).partialOrder ) @@ -171,6 +174,7 @@ class Tests extends TestsConfig with DisciplineSuite { checkAll("Order[Stream[Int]]", OrderTests[Stream[Int]].order) checkAll("Order[Queue[Int]]", OrderTests[Queue[Int]].order) checkAll("Order[SortedSet[String]", OrderTests[SortedSet[String]].order) + checkAll("Order[SortedMap[Int, String]]", OrderTests[SortedMap[Int, String]].order) checkAll("fromOrdering[Int]", OrderTests(Order.fromOrdering[Int]).order) checkAll("Order.reverse(Order[Int])", OrderTests(Order.reverse(Order[Int])).order) checkAll("Order.reverse(Order.reverse(Order[Int]))", OrderTests(Order.reverse(Order.reverse(Order[Int]))).order) diff --git a/kernel/src/main/scala/cats/kernel/Eq.scala b/kernel/src/main/scala/cats/kernel/Eq.scala index 7de7eb694d..3321866b7f 100644 --- a/kernel/src/main/scala/cats/kernel/Eq.scala +++ b/kernel/src/main/scala/cats/kernel/Eq.scala @@ -55,7 +55,7 @@ object Eq with EqToEquivConversion with ScalaVersionSpecificOrderInstances with instances.TupleOrderInstances - with OrderInstances0 { + with OrderInstances1 { /** * Access an implicit `Eq[A]`. @@ -224,7 +224,12 @@ private[kernel] trait OrderInstances0 extends PartialOrderInstances { cats.kernel.instances.seq.catsKernelStdOrderForSeq[A] } -private[kernel] trait PartialOrderInstances extends PartialOrderInstances0 { +private[kernel] trait OrderInstances1 extends OrderInstances0 { + implicit def catsKernelOrderForSortedMap[K: Order, V: Order]: Order[SortedMap[K, V]] = + cats.kernel.instances.sortedMap.catsKernelStdOrderForSortedMap[K, V] +} + +private[kernel] trait PartialOrderInstances extends PartialOrderInstances1 { implicit def catsKernelPartialOrderForOption[A: PartialOrder]: PartialOrder[Option[A]] = cats.kernel.instances.option.catsKernelStdPartialOrderForOption[A] implicit def catsKernelPartialOrderForList[A: PartialOrder]: PartialOrder[List[A]] = @@ -242,6 +247,11 @@ private[kernel] trait PartialOrderInstances0 extends HashInstances { cats.kernel.instances.seq.catsKernelStdPartialOrderForSeq[A] } +private[kernel] trait PartialOrderInstances1 extends PartialOrderInstances0 { + implicit def catsKernelPartialOrderForSortedMap[K: PartialOrder, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = + cats.kernel.instances.sortedMap.catsKernelStdPartialOrderForSortedMap[K, V] +} + private[kernel] trait HashInstances extends HashInstances0 { implicit def catsKernelHashForSet[A]: Hash[Set[A]] = cats.kernel.instances.set.catsKernelStdHashForSet[A] implicit def catsKernelHashForOption[A: Hash]: Hash[Option[A]] = diff --git a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala index 61ce5ea5f9..3dde56c1e1 100644 --- a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala @@ -3,7 +3,7 @@ package instances import scala.collection.immutable.SortedMap -trait SortedMapInstances extends SortedMapInstances2 { +trait SortedMapInstances extends SortedMapInstances3 { implicit def catsKernelStdHashForSortedMap[K: Hash, V: Hash]: Hash[SortedMap[K, V]] = new SortedMapHash[K, V] @@ -35,6 +35,43 @@ private[instances] trait SortedMapInstances2 extends SortedMapInstances1 { implicit def catsKernelStdMonoidForSortedMap[K: Order, V: Semigroup]: Monoid[SortedMap[K, V]] = new SortedMapMonoid[K, V] + + implicit def catsKernelStdPartialOrderForSortedMap[K: PartialOrder, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = + new SortedMapPartialOrder[K, V] +} + +private[instances] trait SortedMapInstances3 extends SortedMapInstances2 { + implicit def catsKernelStdOrderForSortedMap[K: Order, V: Order]: Order[SortedMap[K, V]] = + new SortedMapOrder[K, V] +} + +private[instances] class SortedMapOrder[K, V](implicit K: Order[K], V: Order[V]) extends Order[SortedMap[K, V]] { + override def compare(x: SortedMap[K, V], y: SortedMap[K, V]): Int = + if (x eq y) { + 0 + } else { + x.size.compare(y.size) match { + case 0 => + StaticMethods.iteratorCompare(x.iterator, y.iterator) + case otherwise => + otherwise + } + } +} + +private[instances] class SortedMapPartialOrder[K, V](implicit K: PartialOrder[K], V: PartialOrder[V]) + extends PartialOrder[SortedMap[K, V]] { + override def partialCompare(x: SortedMap[K, V], y: SortedMap[K, V]): Double = + if (x eq y) { + 0.0 + } else { + PartialOrder[Int].partialCompare(x.size, y.size) match { + case 0 => + StaticMethods.iteratorPartialCompare(x.iterator, y.iterator) + case otherwise => + otherwise + } + } } class SortedMapHash[K, V](implicit V: Hash[V], K: Hash[K]) extends SortedMapEq[K, V]()(V) with Hash[SortedMap[K, V]] { From 2abdbb74283f2fd6d1673b1ae5f58a77fa5bc607 Mon Sep 17 00:00:00 2001 From: David Strawn Date: Tue, 21 Dec 2021 11:50:02 -0700 Subject: [PATCH 2/4] Remove Size Check Optimization It makes the code confusing, and it is _possible_ that someone is using an implementation of SortedMap which doesn't have a precomputed size. --- .../cats/kernel/instances/SortedMapInstances.scala | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala index 3dde56c1e1..2a429383a4 100644 --- a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala @@ -50,12 +50,7 @@ private[instances] class SortedMapOrder[K, V](implicit K: Order[K], V: Order[V]) if (x eq y) { 0 } else { - x.size.compare(y.size) match { - case 0 => - StaticMethods.iteratorCompare(x.iterator, y.iterator) - case otherwise => - otherwise - } + StaticMethods.iteratorCompare(x.iterator, y.iterator) } } @@ -65,12 +60,7 @@ private[instances] class SortedMapPartialOrder[K, V](implicit K: PartialOrder[K] if (x eq y) { 0.0 } else { - PartialOrder[Int].partialCompare(x.size, y.size) match { - case 0 => - StaticMethods.iteratorPartialCompare(x.iterator, y.iterator) - case otherwise => - otherwise - } + StaticMethods.iteratorPartialCompare(x.iterator, y.iterator) } } From 48f6909baacba3b8474f184f3dd5805e1915c7a3 Mon Sep 17 00:00:00 2001 From: David Strawn Date: Fri, 7 Jan 2022 07:56:02 -0700 Subject: [PATCH 3/4] Remove PartialOrder and Order constraints on SortedMap K --- .../cats/kernel/instances/SortedMapInstances.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala index 2a429383a4..dea710e8ca 100644 --- a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala @@ -45,23 +45,28 @@ private[instances] trait SortedMapInstances3 extends SortedMapInstances2 { new SortedMapOrder[K, V] } -private[instances] class SortedMapOrder[K, V](implicit K: Order[K], V: Order[V]) extends Order[SortedMap[K, V]] { - override def compare(x: SortedMap[K, V], y: SortedMap[K, V]): Int = +private[instances] class SortedMapOrder[K, V](implicit V: Order[V]) extends Order[SortedMap[K, V]] { + override def compare(x: SortedMap[K, V], y: SortedMap[K, V]): Int = { + implicit val order: Order[K] = Order.fromOrdering(x.ordering) if (x eq y) { 0 } else { StaticMethods.iteratorCompare(x.iterator, y.iterator) } + } } -private[instances] class SortedMapPartialOrder[K, V](implicit K: PartialOrder[K], V: PartialOrder[V]) +private[instances] class SortedMapPartialOrder[K, V](implicit V: PartialOrder[V]) extends PartialOrder[SortedMap[K, V]] { - override def partialCompare(x: SortedMap[K, V], y: SortedMap[K, V]): Double = + override def partialCompare(x: SortedMap[K, V], y: SortedMap[K, V]): Double = { + implicit val order: Order[K] = Order.fromOrdering(x.ordering) + if (x eq y) { 0.0 } else { StaticMethods.iteratorPartialCompare(x.iterator, y.iterator) } + } } class SortedMapHash[K, V](implicit V: Hash[V], K: Hash[K]) extends SortedMapEq[K, V]()(V) with Hash[SortedMap[K, V]] { From 3d38c3632ded5acbb82bd0d048b896628601b131 Mon Sep 17 00:00:00 2001 From: David Strawn Date: Mon, 10 Jan 2022 08:14:02 -0700 Subject: [PATCH 4/4] Remove Unneeded Constraints --- kernel/src/main/scala/cats/kernel/Eq.scala | 4 ++-- .../main/scala/cats/kernel/instances/SortedMapInstances.scala | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/src/main/scala/cats/kernel/Eq.scala b/kernel/src/main/scala/cats/kernel/Eq.scala index 3321866b7f..67f9877e3e 100644 --- a/kernel/src/main/scala/cats/kernel/Eq.scala +++ b/kernel/src/main/scala/cats/kernel/Eq.scala @@ -225,7 +225,7 @@ private[kernel] trait OrderInstances0 extends PartialOrderInstances { } private[kernel] trait OrderInstances1 extends OrderInstances0 { - implicit def catsKernelOrderForSortedMap[K: Order, V: Order]: Order[SortedMap[K, V]] = + implicit def catsKernelOrderForSortedMap[K, V: Order]: Order[SortedMap[K, V]] = cats.kernel.instances.sortedMap.catsKernelStdOrderForSortedMap[K, V] } @@ -248,7 +248,7 @@ private[kernel] trait PartialOrderInstances0 extends HashInstances { } private[kernel] trait PartialOrderInstances1 extends PartialOrderInstances0 { - implicit def catsKernelPartialOrderForSortedMap[K: PartialOrder, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = + implicit def catsKernelPartialOrderForSortedMap[K, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = cats.kernel.instances.sortedMap.catsKernelStdPartialOrderForSortedMap[K, V] } diff --git a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala index dea710e8ca..a2d76c9531 100644 --- a/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/SortedMapInstances.scala @@ -36,12 +36,12 @@ private[instances] trait SortedMapInstances2 extends SortedMapInstances1 { implicit def catsKernelStdMonoidForSortedMap[K: Order, V: Semigroup]: Monoid[SortedMap[K, V]] = new SortedMapMonoid[K, V] - implicit def catsKernelStdPartialOrderForSortedMap[K: PartialOrder, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = + implicit def catsKernelStdPartialOrderForSortedMap[K, V: PartialOrder]: PartialOrder[SortedMap[K, V]] = new SortedMapPartialOrder[K, V] } private[instances] trait SortedMapInstances3 extends SortedMapInstances2 { - implicit def catsKernelStdOrderForSortedMap[K: Order, V: Order]: Order[SortedMap[K, V]] = + implicit def catsKernelStdOrderForSortedMap[K, V: Order]: Order[SortedMap[K, V]] = new SortedMapOrder[K, V] }