Partial replace call to snprintf for formating floatingpoint numbers for %f and %F

[berni44]

This is a copy of #7264 and a followup on #7318 and #7285.

I rebased it, added some minor fixes and a test that can be run to compare the result of snprintf and the result of printFloat.

To run the test uncomment the line // version = printFloatTest; near the end of the file and run a normal unittest. This takes 30 minutes and compares random input. Some known bugs of snprintf are written at the top of the test section. They will, of course, be reported as differences.

For more information see the original PR. I was able to restore the PDF document mentioned there, thanks to @skoppe and @m3m0ry who saved it.

@thewilsonator asked there one question I didn't answer yet:

Following the success of the float zero factorisation, I'm beginning to think that the best way to organise the printing is not by format specifier (%a,%f,%g,%e) but by "category" (for want of a better term), i.e.

• inf/nan (handled above),
• Algorithm A: For large exponents (exp >= T.mant_dig)
• Algorithm B: For small exponents (exp < T.mant_dig - 61)
• Algorithm C: For exponents close to 0. (already done)

Since there seems to be much more in common between the different ranges of numbers than there is between the different format specifiers.

Yes, you're right. I allready thought about this too (and snprintf does this too). But I'm a little bit reluctant do so immediately for two reasons: First, I fear, that the result would be slower because several if switches might be needed. Second and more important, I have some ideas how cases A and B can be speed up. Implementing these ideas might be much more difficult, when I have to code for all specifiers simultaneously.

[dlang-bot]

Thanks for your pull request and interest in making D better, @berni44! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	9889	normal	Incorrect rounding on floating value formatting
✗	20320	normal	format("%f") leeds to wrong output
✗	20363	normal	painting XMM registers as integers leads to codegen bugs
✗	20371	normal	std.format limited to 500 characters for floats

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7757"

[berni44]

I don't know why the bot thinks issue 20636 is touched here. This is no enhancement!

[RazvanN7]

Commit message of 686c5e5 contains the following text:

"Bug 20636 has meanwhile been fixed, so no hack needed anymore
Remove two missleading comments (there is no more cast needed)
Adapt new convention for naming issues
Add missing spaces arround operators"

The bolded words have triggered the bot. Maybe you can rephrase the commit message to avoid those words.

[berni44]

The number was wrong anyway: 20636 instead of the correct 20363... Now, at least, he added the bug fix label, but the enhancement remains... Maybe, someone with write access could just delete the enhancement label?

[schveiguy]

I mean, it is an enhancement, no? Or does this not remove the requirement of snprintf?

[berni44]

I mean, it is an enhancement, no? Or does this not remove the requirement of snprintf?

Well, it replaces snprintf (partially). But is that already an enhancement? Is it an enhancement replacing quicksort by timsort internally? I don't know. Might depend on the definition of enhancement...

[PetarKirov]

Making Druntime and Phobos more independent of libc (even partially) is an important improvement in my book.

[RazvanN7]

Approving the idea, however, this needs a proper review.

[ibuclaw]

You mention "does not fit into a ulong" below. Are you assuming that mant_dig is always <= 64? What happens when the mant_dig is 106 or 113?

[berni44]

This PR targets only floats, doubles and 64bit reals. See the template constraints of this function.
The "does not fit" comment refers to the unrolled number, where all the zeros (counted in exp) are attached either left or right to the mantissa; these are really large numbers, taking up to a little bit more than 1000 bits.

[berni44]

I wrote some explanations accompanied by source code snippets and illustrations:
https://github.com/berni44/printFloat/blob/master/walkthrough.pdf
I hope, this will make the review easier and tear down the fears...

[berni44]

Resolved merge conflicts.

With PR #7762 being merged, printing floats with %f will automatically be CTFEable too. I added unittests and corrected the error message.

I also fixed a small bug in an other unittest - FloatingPointControl is not available on targets with soft float.

[thewilsonator]

This PR has a number of inconsistent (missing) whitespace between binary operands, but otherwise looks fine.

[thewilsonator]

See comments

[jondegenhardt]

This PR ended up fixing a case of incorrect floating point rounding occurring on Windows in the tsv-utils test suite. 👍