[stdlib] More verbose debugDescription for NaN #2786
Conversation
| #if !arch(i386) | ||
| expectDebugPrinted("snan", Float.signalingNaN) | ||
| expectDebugPrinted("snan(0xffff)", Float(nan: 65535, signaling: true)) | ||
| expectDebugPrinted("-snan(0xffff)", -Float(nan: 65535, signaling: true)) |
There was a problem hiding this comment.
Could you add tests with the largest possible payload, to make sure that high bits are not dropped in any of the conversions?
| expectDebugPrinted("nan(0xffff)", Float(nan: 65535, signaling: false)) | ||
| expectDebugPrinted("-nan(0xffff)", -Float(nan: 65535, signaling: false)) | ||
| expectDebugPrinted("nan(0x1fffff)", Float(bitPattern: 0x7fff_ffff)) | ||
| expectDebugPrinted("nan(0x1fffff)", Float(bitPattern: 0x7fdf_ffff)) |
There was a problem hiding this comment.
Actually, current implementation drops MSB of payload bits,
because that bit is reserved for placeholder payload of signaling NaN.
I'm not sure whether this behavior is reasonable or not. @stephentyrone ?
|
To clarify the largest NaN payload value. |
|
@swift-ci Please test |
| } | ||
| let isSignaling = (significand & Float${bits}._quietNaNMask) == 0 | ||
| let payload = significand & ((Float${bits}._quietNaNMask >> 1) - 1) | ||
| return |
There was a problem hiding this comment.
Please add a FIXME note here, as these string manipulations are inefficient enough to make someone want to optimize them.
There was a problem hiding this comment.
For example, we could move some of the string concatenation into the C function that we are calling below.
There was a problem hiding this comment.
We usually use FIXME(performance) for these kinds of notes.
|
Minor note: there is enough repetition in |
|
Oh, and yes, for the record, Linux build failure is unrelated. |
|
@moiseev Format looks good. |
The format of debugDescription for NaN is: [ '-' ] ( 'snan' | 'nan' ) [ '(0x' hexadecimal-digits ')' ]
|
Added |
What's in this pull request?
Follow-up on #1922
Now, the format of
debugDescriptionfor NaN is:-[(0xhexadecimal-digits)]@stephentyrone
Could you confirm the format?
@gribozavr
Should we use more high level APIs?
Before merging this pull request to apple/swift repository: