D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 9889 - Incorrect rounding on floating value formatting
Summary: Incorrect rounding on floating value formatting
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: x86 Windows
: P2 normal
Assignee: No Owner
URL:
Keywords: pull, wrong-code
Depends on:
Blocks:
 
Reported: 2013-04-06 02:51 UTC by Denis Shelomovskii
Modified: 2021-02-15 11:00 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Denis Shelomovskii 2013-04-06 02:51:19 UTC
`std.format.formatValue` uses C's `snprintf` which works this way in `snn.lib`:
---
import std.string;

void main()
{
    assert(format("%.1f", 0.09) == "0.1"); // Failed, formatted as "0.0"
    assert(format("%.1f", -0.09) == "-0.1"); // Failed, formatted as "-0.0"
    assert(format("%.1f", 0.095) == "0.1"); // OK
    assert(format("%.1f", -0.095) == "-0.1"); // OK
    assert(format("%.1f", 0.094) == "0.1"); // Failed, formatted as "0.0"
    assert(format("%.1f", -0.094) == "-0.1"); // Failed, formatted as "-0.0"
}
---


Workaround:
---
real preformatFloating(real n, ubyte digits)
{
    immutable k = 10.0L ^^ digits;
    return round(n * k) / k;
}
void main()
{
    assert(format("%.1f", preformatFloating(0.09, 1)) == "0.1");
}
---
Comment 1 AndyC 2015-01-24 14:44:47 UTC
Tested on DMD v2.066.1, this runs fine.

import std.string;

void main()
{
    assert(format("%.1f", 0.09) == "0.1"); // Failed, formatted as "0.0"
    assert(format("%.1f", -0.09) == "-0.1"); // Failed, formatted as "-0.0"
    assert(format("%.1f", 0.095) == "0.1"); // OK
    assert(format("%.1f", -0.095) == "-0.1"); // OK
    assert(format("%.1f", 0.094) == "0.1"); // Failed, formatted as "0.0"
    assert(format("%.1f", -0.094) == "-0.1"); // Failed, formatted as "-0.0"
}


Report was on windows, I tested in on linux, hopefully format() isnt os specific.
Report mentions std.format.formatValue but doesnt test it.
Comment 2 Andrei Alexandrescu 2015-01-24 14:54:45 UTC
Closing preemptively according to http://forum.dlang.org/thread/fsmjexsfxtqveqffiemc@forum.dlang.org#post-ybwwdcpzianfozqcflcv:40forum.dlang.org. Please reopen if necessary, thanks.
Comment 3 yebblies 2015-01-24 17:08:03 UTC
As the original comment states, this is specific to snn.lib (digital mars' c runtime) and therefore win32.
Comment 4 Andrei Alexandrescu 2015-01-24 17:21:07 UTC
Thanks @yebblies!
Comment 5 Denis Shelomovskii 2015-01-25 11:31:59 UTC
(In reply to Andrei Alexandrescu from comment #2)
> Closing preemptively according to
> http://forum.dlang.org/thread/fsmjexsfxtqveqffiemc@forum.dlang.org#post-
> ybwwdcpzianfozqcflcv:40forum.dlang.org. Please reopen if necessary, thanks.

Wat has just happened here?

There was a Windows-specific bug report. A person who isn't fimilar with Phobos and lokks like a newbie (not his fault by the way) tested it on Linux, said "testcase works" and changed OS to Linux, also proving the fact he has no idea how does formatting work by writing "format() isnt os specific" and "Report mentions std.format.formatValue but doesnt test it". And based on this the issue was closed by a Phobos developer.

So I'd like to ask Phobos devs: please, if your are tired and out of time, don't do thoughtless actions. In case something probably can be easily closed/fixed etc. please ask issue reporter or another person who has more time to induct because such actions in the worst case may remain unnoticed. Thanks.
Comment 6 yebblies 2015-01-25 11:49:16 UTC
(In reply to Denis Shelomovskij from comment #5)
> 
> Wat has just happened here?
> 
> There was a Windows-specific bug report. A person who isn't fimilar with
> Phobos and lokks like a newbie (not his fault by the way) tested it on
> Linux, said "testcase works" and changed OS to Linux, also proving the fact
> he has no idea how does formatting work by writing "format() isnt os
> specific" and "Report mentions std.format.formatValue but doesnt test it".
> And based on this the issue was closed by a Phobos developer.
> 
> So I'd like to ask Phobos devs: please, if your are tired and out of time,
> don't do thoughtless actions. In case something probably can be easily
> closed/fixed etc. please ask issue reporter or another person who has more
> time to induct because such actions in the worst case may remain unnoticed.
> Thanks.

The original reporter is CC'd when an issue is closed or updated, so mistakes can easily be corrected.  It's not a big deal, just a mistake.
Comment 7 Steven Schveighoffer 2015-01-26 13:01:34 UTC
Adjusting arch to be X86 only according to Daniel's comment.

One thing I would mention -- it's not readily apparent that any of the fields above have changed since the original report, the change is logged but you have to look at bug history to see the change. Please avoid changing them unless you are certain that the change is correct. AndyC, I appreciate you combing through old bugs, so please keep going!
Comment 8 Dlang Bot 2019-12-07 15:53:52 UTC
@berni44 updated dlang/phobos pull request #7264 "Partial replace call to snprintf for formating floatingpoint numbers." fixing this issue:

- Fix Issue 9889 - Incorrect rounding on floating value formatting

https://github.com/dlang/phobos/pull/7264
Comment 9 Dlang Bot 2021-01-20 17:41:52 UTC
@berni44 created dlang/phobos pull request #7757 "Partial replace call to snprintf for formating floatingpoint numbers for %f and %F" fixing this issue:

- Fix Issue 9889 - Incorrect rounding on floating value formatting

https://github.com/dlang/phobos/pull/7757
Comment 10 Dlang Bot 2021-02-15 11:00:59 UTC
dlang/phobos pull request #7757 "Partial replace call to snprintf for formating floatingpoint numbers for %f and %F" was merged into master:

- 3767b6c06af271f6aa72cbfbc7a210e582401447 by Bernhard Seckinger:
  Fix Issue 9889 - Incorrect rounding on floating value formatting

https://github.com/dlang/phobos/pull/7757