Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dbl printing #2954

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix dbl printing #2954

wants to merge 2 commits into from

Conversation

cpq
Copy link
Member

@cpq cpq commented Nov 6, 2024

No description provided.

@cpq cpq requested a review from scaprile November 6, 2024 13:28
Comment on lines +2155 to +2158
TESTDOUBLE_NOHOSTCHECK("%g", 0.00001, "0.00001"); // "1e-05"
TESTDOUBLE("%g", 0.000001, "1e-06");
TESTDOUBLE("%g", -0.0001, "-0.0001");
TESTDOUBLE_NOHOSTCHECK("%g", -0.00001, "-0.00001"); // "-1e-05"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these tests.
We "fail" those marked as NOHOSTCHECK as we don't go scientific at 5 decimals
I mean, the host prints "1e-05" and we don't

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NOHOSTCHECK tests are added cause the result is implementation specific, and not stable across platforms

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explicitly added those two I'm marking here, please see I added an extra commit to this PR. I did this because I knew we had some issues before and wanted to check.
Those tests I added fail, unless I mark them as NOHOSTCHECK because

AFAICS we are expected to print 1e-05 and yet we print 0.00001

So either we:

  • make them work as Linux/? do
  • let them as they are until someone complains (if ever)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants