Skip to content
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

Merged
merged 1 commit into from Feb 18, 2020

Conversation

eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented Feb 15, 2020

Fixes scala/bug#11844
Ref scala/bug#10511
Ref #6410
Ref #76

This changes the deprecation of DeprecatedDoubleOrdering to a migration warning to avoid List(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, the Ordering instance used semantics consistent with java.lang.Double.compare.

Scala 2.10.0 changed the implementation of lt, equiv, min, etc., to be IEEE 754 compliant, while keeping the compare method NOT compliant, creating an internally inconsistent instance. IEEE 754 specifies that 0.0 == -0.0. In addition, it requires all comparisons with Double.NaN return false thus 0.0 < Double.NaN, 0.0 > Double.NaN, and Double.NaN == Double.NaN all yield false, analogous None in flatMap.

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, and Ordering.Double.TotalOrdering, which brings back the java.lang.Double.compare semantics for all operations. The default extends TotalOrdering.

List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).sorted      // List(-Infinity, 0.0, 1.0, NaN)
List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).min         // -Infinity
implicitly[Ordering[Double]].lt(0.0, 0.0 / 0.0)   // true
{
  import Ordering.Double.IeeeOrdering
  List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).sorted    // List(-Infinity, 0.0, 1.0, NaN)
  List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).min       // NaN
  implicitly[Ordering[Double]].lt(0.0, 0.0 / 0.0) // false
}

* 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! 👍

@dwijnand
Copy link
Member

Suggested some tweaks: eed3si9n#1

@SethTisue SethTisue self-assigned this Feb 17, 2020
Copy link
Member

@SethTisue SethTisue left a 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

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Feb 18, 2020
@SethTisue SethTisue changed the title Un-deprecate default floating point Orderings, and change to migration Un-deprecate default floating point Orderings; issue migration warning instead under -Xmigration Feb 18, 2020
Copy link
Member

@dwijnand dwijnand left a 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

@eed3si9n
Copy link
Member Author

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.
@SethTisue SethTisue merged commit a093401 into scala:2.13.x Feb 18, 2020
@eed3si9n eed3si9n deleted the wip/migrate-ordering branch February 18, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
6 participants