std.format: format floats and doubles with %g / %G / %s

[berni44]

There where mainly three things I had to do to implement %g:

1. Find the exact break points when to use %e and when to us %f
2. Remove trailing 0s and dot, when # is not present
3. Take care of the different meaning of precision (significant digits instead of fractional digits)
4. (oops, one more): Take care of 0.0.

As usual I ran lots of external tests comparing the current output with the new one: All floats; all combinations of flags/rounding mode/selection of number,width and precision; several billion random samples. Aside from known bugs in the old implementation no differences where found. (I also ran the tests for %e and %f to make sure I didn't mess up anything there.)

[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
✗	20451	normal	comparing identical floating points does not work on Win32 and FreeBSD32.
✓	21641	minor	std.format: %g produces in rare circumstances inconsistent result

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#7804"

[berni44]

I expected somewhat, that these unittests would fail on some processors, probably those with real==double. The floating point calculations in the switch in printFloatG have got rounding errors there, while not on other computers due to the internal use of (large enough) reals.

Finding a way around this, is difficult. The selection between %e and %f in snprintf is based on the decimal representation, not on the binary. That means, if we want to get around the floating point calculations, we would need some (expensive) BigInt calculations or a (large) lookup table. I dislike both, because it only applies in really, really rare occasions and the effort seems not to pay off here.

What happens in these situations is, that it prints the correct number, just the decision between scientific and "normal" representation is wrong (i. E. 1e-04 instead of 0.0001 or 0.0000999999 instead of 9.99999e-05).

One solution, although I do not know if you could accept it, would be to replace the unittest with a little bit more relaxed one, like this:

    double val = nextUp(0.00009999995);
    assert(printFloat(buf[], val, f) == "0.0001");
    val = nextDown(val);
    auto result = printFloat(buf[], val, f);
    assert(result == "0.0001" || result == "1e-04");
    val = nextDown(val);
    result = printFloat(buf[], val, f);
    assert(result == "9.99999e-05" || result == "0.0000999999");
    val = nextDown(val);
    assert(printFloat(buf[], val, f) == "9.99999e-05");

What do you think? Any ideas, how to get around this problem?

PS: Maybe it's possible to check the result with a regex and call the correct function in case the result is wrong... I'll have to sleep on it.

[berni44]

To get around that problem, I replaced the calls to ^^ by a specialized function using ulongs. It fixes some really rare corner cases I overlooked before.

Unfortunately freebsd32 and win32 still fail on one unittest. After narrowing down the bug I ended up with a wrong floating point calculation:

unittest
{
        double a = 0.00009999995;
        double b = 0.00000000005;

        writefln!"%.80f"(a);
        writefln!"%.80f"(0.0001 - b);

        assert(!(a < 0.0001 - b));
}

The two writeflns show, that both values have identical bit patterns, but anyway the assert fires on the two 32-bit-computers.

Because fixing this bug is beyond this PR (*) I commented out the offending unittest.

(*) It would mean to add support for soft floats, or at least the parts, that are needed for the calculations in printFloatG.

PS: I think this is good to go now.

[berni44]

@RazvanN7 Could you remove the WIP label please? I can't do so...

[RazvanN7]

I haven't thoroughly reviewed this, but there are some things that I noticed at a cursory glance:

• please do not mix refactorings (even if they are small) with code additions. There are some places where a-b is transformed to a - b. Try to keep the diff as small as possible.
• is there any way that this PR can be broken in several smaller PRs? It is very hard to review it as is. For example, is it possible to incrementally add the modifications to the printFloatE and printFloatF functions and then make a PR that adds printFloatG? That would make the reviewers job a lot easier.
• Is there an alternative to using if(is_g) everywhere? This is effectively adding a lot of branches and might negatively impact performance. Normally, the format string should be known at compile time and therefore static ifs could be used, but I don't know exactly if this applies in this situation.

[berni44]

• please do not mix refactorings (even if they are small) with code additions. There are some places where a-b is transformed to a - b. Try to keep the diff as small as possible.

You're right. I probably did this, because I was asked to do so in the %f PR. Anyway, I'll open a separate PR for that.

• is there any way that this PR can be broken in several smaller PRs?

Yes, I think so. But I'll start with adding printFloatG, because this is the part that is most problematic with regards to the content and I want to get this discussed first.

• Is there an alternative to using if(is_g) everywhere? This is effectively adding a lot of branches and might negatively impact performance.

I avoided them inside of loops which means, the impact should be little.

Normally, the format string should be known at compile time and therefore static ifs could be used, but I don't know exactly if this applies in this situation.

Well that's not always true. But we do not need the format string here, to make the decision static, we only need to know if we are processing %e/%f or %g, which we do know.

I'm closing this, to keep it as a reference, and start new PRs.

[RazvanN7]

Thanks @berni44 ! I look forward to the split PRs.