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

[stdlib] Change documented behavior of FloatingPoint.significand #26390

Merged
merged 1 commit into from Jul 29, 2019

Conversation

@xwu
Copy link
Collaborator

commented Jul 29, 2019

A seemingly innocuous but actually nontrivial change.

In SE-0067, the intended behavior of significand for exceptional values was as follows:

  • If x is zero, then x.significand is 0.0.
  • If x is infinity, then x.significand is 1.0.
  • If x is NaN, then x.significand is NaN.

However, the actual behavior of significand is as follows:

Float.infinity.significand  // +inf
Double.infinity.significand // +inf
// (etc.)

To align the actual behavior with the intended one would have been easy enough before ABI stability: the branch for finite normal values works just fine for infinite values in producing the desired result. However, the function is @inlinable and we've shipped the existing behavior, so observably changing the actual behavior now is somewhat suboptimal from that standpoint.

On the other hand, it's perfectly defensible for infinite values to have an infinite significand. The documentation currently states:

If a type’s radix is 2, then for finite nonzero numbers, the significand is in the range 1.0 ..< 2.0.

With the current actual behavior, we can make another guarantee: for infinite, zero, or NaN values, the significand is not in the range 1.0 ..< 2.0.

In any case, we've got to reconcile the documented behavior with the actual behavior one way or another.


(Not tackled here: the documentation for FloatingPoint requirements is repeated in two places; this should be unnecessary and is liable to fall out of sync.)

@xwu xwu requested review from natecook1000 and stephentyrone Jul 29, 2019

@stephentyrone

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

Yup, this is fine. Good catch.

@swift-ci smoke test and merge

@xwu

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 29, 2019

@swift-ci Please smoke test OS X platform

@stephentyrone stephentyrone merged commit a078c33 into apple:master Jul 29, 2019

2 of 3 checks passed

Test and Merge (smoke test) Build finished.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details

@xwu xwu deleted the xwu:significand-of-infinity branch Jul 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.