-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Un-deprecate default floating point Orderings; issue migration warning instead under -Xmigration #8721
Conversation
* import Ordering.Float.IeeeOrdering | ||
* List(0.0F, 1.0F, 0.0F / 0.0F, -1.0F / 0.0F).sorted // List(-Infinity, 0.0, 1.0, NaN) | ||
* List(0.0F, 1.0F, 0.0F / 0.0F, -1.0F / 0.0F).min // NaN | ||
* implicitly[Ordering[Float]].lt(0.0F, 0.0F / 0.0F) // false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! 👍
Suggested some tweaks: eed3si9n#1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
squash before merge, otherwise LGTM, thanks Eugene, and thanks @NthPortal @Ichoran @odersky and everyone else who helped this along
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested some tweaks eed3si9n#1
I just pulled @dwijnand's fix ups in. I'll squash them all now. |
Fixes scala/bug#11844 Ref scala/bug#10511 Ref scala#6410 Ref scala#76 This change the deprecation of `DeprecatedDoubleOrdering` to a migration warning instead to avoid `List(1.0, -1.0).sorted` giving deprecation warning. This also provides some documentation on the ordering instances in Scaladoc.
ee86732
to
4bda33d
Compare
Fixes scala/bug#11844
Ref scala/bug#10511
Ref #6410
Ref #76
This changes the deprecation of
DeprecatedDoubleOrdering
to a migration warning to avoidList(1.0, -1.0).sorted
giving a deprecation warning.This also provides some documentation on the ordering instances in Scaladoc:
The behavior of the comparison operations provided by the default (implicit) ordering on
Double
changed in 2.10.0 and 2.13.0. Prior to Scala 2.10.0, theOrdering
instance used semantics consistent withjava.lang.Double.compare
.Scala 2.10.0 changed the implementation of
lt
,equiv
,min
, etc., to be IEEE 754 compliant, while keeping thecompare
method NOT compliant, creating an internally inconsistent instance. IEEE 754 specifies that0.0 == -0.0
. In addition, it requires all comparisons withDouble.NaN
returnfalse
thus0.0 < Double.NaN
,0.0 > Double.NaN
, andDouble.NaN == Double.NaN
all yieldfalse
, analogousNone
inflatMap
.Recognizing the limitation of the IEEE 754 semantics in terms of ordering, Scala 2.13.0 created two instances:
Ordering.Double.IeeeOrdering
, which retains the IEEE 754 semantics from Scala 2.12.x, andOrdering.Double.TotalOrdering
, which brings back thejava.lang.Double.compare
semantics for all operations. The default extendsTotalOrdering
.