SI-10086 NumericRange.min|max with custom Integral#5575
Conversation
|
I didn't see this. You can copy the unit test. It passed locally. https://github.com/scala/scala/compare/2.12.x...som-snytt:issue/10086-max-order?expand=1 |
|
It's not obvious to me that the following |
946e9cc to
8800717
Compare
a9a4523 to
ca016a4
Compare
ca016a4 to
de0a1fa
Compare
|
This is ready for review. |
|
Probably review by @Ichoran "Ichorange". |
| case class A(v: Int) | ||
|
|
||
| implicit object aIsIntegral extends scala.math.Integral[A] { | ||
| def compare(x: A, y: A): Int = x.v - y.v |
There was a problem hiding this comment.
This is inconsistent. Orderings must be transitive with respect to the sign of compare but this is not true with A(Int.MinValue), A(0), and A(Int.MaxValue). Though extreme values are not used in this test, it is nonobvious that this method fails, and there is no advantage to using subtraction rather than the compare method on the underlying Ints.
|
It would be good to have something--at least a commit message--that explains why |
|
Updated. Please let me know when you would like me to squash the commits. |
|
LGTM, squashing on merge. |
No description provided.