Fix sNaN behavior of .minimum, .maximum, .minimumMagnitude and .maximumMagnitude. #3361
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and .maximumMagnitude in line with the IEEE 754 rules.
Specifically, if either argument is a signaling NaN, the result
should be a quiet NaN. Previously, signaling NaN inputs were
treated identically to quiet NaN inputs.
Some other changes folded in with this:
- Formally define which argument is returned if lhs == rhs or
abs(lhs) == abs(rhs) in the case of the Magnitude versions.
This guarantees that the set {min,max} is identitical to
the set {lhs,rhs} so long as neither argument is NaN. This
is not only a nod to formalism; it's also a critical property
for generic head-tail arithmetic to simulate wider types.
- Added test coverage for these four operations.
|
@swift-ci please test. |
|
@gribozavr I would appreciate another set of eyes on this if you can spare a few minutes. |
|
OS X build failure is Foundation noise unrelated to this. |
| // IEEE 754 basic operations, this property is critical to ensure | ||
| // that clamping to a valid range behaves as expected. | ||
| else if lhs.isNaN && !rhs.isNaN { | ||
| print("lhs is \(lhs.bitPattern). rhs is \(rhs.bitPattern).") |
There was a problem hiding this comment.
Yup, thanks. Will push a fix shortly.
|
LGTM, that's how these operations signal an invalid operation exception. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specifically, if either argument is a signaling NaN, the result should be a quiet NaN. Previously, signaling NaN inputs were treated identically to quiet NaN inputs.
Some other changes folded in with this:
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.