Fix Decimal

[xwu]

What's in this pull request?

This PR fixes buggy implementations of FloatingPoint protocol members in Decimal.

First, it fixes significand.
Previously, Decimal(1.35).significand == 1350 and Decimal(1.35).exponent == -2.
This was obviously incorrect: the expected value for Decimal(1.35).significand is 135.

Second, it fixes two bugs in init(sign:exponent:significand:).
Previously, only the mantissa of the significand argument was preserved; also, the sign was incorrectly flipped. So, as reported in SR-2491, it was observed that Decimal(sign: .plus, exponent: -2, significand: 100) == -0.01.
This was clearly incorrect: the expected value is 1, in agreement with documentation that the result should be equal to (sign == .minus ? -1 : 1) * significand * Decimal.radix ** exponent.

Third, it aligns ulp with the documentation.
It is unclear what the previous code was trying to do. Now, x.ulp == Decimal.nan if !x.isFinite; otherwise, the value returned is 10 ** exponent, the unit of the least significant digit in the significand.

Resolved bug number: (SR-2491)

---

Before merging this pull request to apple/swift repository:

• Test pull request on Swift continuous integration.

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

Platform	Comment
All supported platforms	@swift-ci Please smoke test
All supported platforms	@swift-ci Please smoke test and merge
OS X platform	@swift-ci Please smoke test OS X platform
Linux platform	@swift-ci Please smoke test Linux platform

A smoke test on macOS does the following:

1. Builds the compiler incrementally.
2. Builds the standard library only for macOS. Simulator standard libraries and
   device standard libraries are not built.
3. lldb is not built.
4. The test and validation-test targets are run only for macOS. The optimized
   version of these tests are not run.

A smoke test on Linux does the following:

1. Builds the compiler incrementally.
2. Builds the standard library incrementally.
3. lldb is built incrementally.
4. The swift test and validation-test targets are run. The optimized version of these
   tests are not run.
5. lldb is tested.

Validation Testing

Platform	Comment
All supported platforms	@swift-ci Please test
All supported platforms	@swift-ci Please clean test
All supported platforms	@swift-ci Please test and merge
OS X platform	@swift-ci Please test OS X platform
OS X platform	@swift-ci Please clean test OS X platform
OS X platform	@swift-ci Please benchmark
Linux platform	@swift-ci Please test Linux platform
Linux platform	@swift-ci Please clean test Linux platform

Lint Testing

Language	Comment
Python	@swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

[xwu]

@stephentyrone Would you be willing to review?

[stephentyrone]

The changes you've made look correct. I think there are still some outstanding issues, which can be handled later; in particular, "20" should really be 20 x 10^0 (i.e. have an exponent of zero, not one), so I'm not wild about the specific test case that you've chosen, even if it matches the current behavior of the library. Can you construct the number in the test case from parts instead so the test you added is somewhat future-proofed?

[xwu]

@stephentyrone Yes, I'll go ahead and make that change to the test.

However, I'm confused as to why "20" should be 20 * 10 ^ 0. I was of the understanding that NSDecimalNumber canonicalizes the representation and is really storing an exponent of 1 and a significand/mantissa of 2. Is that not the case? Or are you saying that it is the case currently, but that it may change in the future?

[stephentyrone]

Right, that's what NSDecimalNumber does, but not what a fully-correct Decimal type should do; IEEE-754 decimal types aren't generally normalized; if one is initialized from an integer literal or integer, it should be represented with an exponent of zero if it's possible to do so.

[xwu]

Got it; I'll construct the test example as an NSDecimalNumber from parts, then bridge to Decimal and test the relevant properties.

[xwu]

Done. Who should I ping to get this tested? And is it appropriate to nominate for Swift 3.0?

[stephentyrone]

@swift-ci Please smoke test and merge

[stephentyrone]

I believe that it's too late for Swift 3.0. @tkremenek, can you confirm?

[xwu]

Hmm, no smoke testing seems to be happening.

[gparker42]

@swift-ci Please smoke test and merge

[tkremenek]

@xwu It is too late for Swift 3.0 proper, but there may be future releases based on that branch. Please go ahead and submit a pull request against the swift-3.0-branch and put "[Swift 3.0.1]" in the PR title.