Skip to content

Fix stdio fmt of "%.0e" and "%.0g" #544

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

Merged
merged 3 commits into from
Aug 18, 2022
Merged

Fix stdio fmt of "%.0e" and "%.0g" #544

merged 3 commits into from
Aug 18, 2022

Conversation

G4Vi
Copy link
Collaborator

@G4Vi G4Vi commented Aug 17, 2022

Fixes #543

"%.0e" was fixed by always using dtoa mode 2: max(1,ndigits) significant digits so the number will still be rounded even if no decimal places will be printed, the change should only change behavior when precision is 0.

The fix to "%.0g" is similar, according to man 3 printf "if the precision is zero, it is treated as 1", so perc is now rounded up to 1 instead of 0 if necessary. This also leads to always using dtoa mode 2.

EDIT: Changing this to WIP as "%.0g", 10 and "%.0g", -10. appears to be failing.
EDIT2: Not sure if the behavior required by tests is desired, see next comment.

@G4Vi G4Vi changed the title Fix stdio fmt of "%.0e" and "%.0g" [WIP] Fix stdio fmt of "%.0e" and "%.0g" Aug 17, 2022
@G4Vi
Copy link
Collaborator Author

G4Vi commented Aug 17, 2022

Is there a specific spec of printf cosmo is trying to follow? The failing tests below from fmt_test.c in {fmt, input, expected} form (do ensure that "%.0g" prints the shortest representation of 10 and -10, however it differs from glibc and my interpretation of the linux man 3 printf

{"%.0g", 10., "10"},
{"%.0g", -10., "-10"},

glibc prints

1e+01
-1e+01

for those tests.

My interpretation of the g, G conversion specifier in psuedo code is as follows:

if(precision == undef)
{
    precision = 6;
}
else if(precision == 0)
{
    precision = 1;
}
if((exponent_from_conversion < -4) || (exponent_from_conversion >= precision))
{
    usestylee();
}
else
{
    usestylef();
}

The "%g" patch currently included causes 10 and -10 to match the glibc behavior of encoding them as 1e+01 and -1e+01.

@G4Vi G4Vi changed the title [WIP] Fix stdio fmt of "%.0e" and "%.0g" [FEEDBACK REQ] Fix stdio fmt of "%.0e" and "%.0g" Aug 17, 2022
@jart
Copy link
Owner

jart commented Aug 17, 2022

Is there a specific spec of printf cosmo is trying to follow?

Glibc is the gold standard of C libraries. Anything that helps us be closer to Glibc in terms of behavior would be very welcome.

If we have tests that specify a behavior that's inconsistent with Glibc, then our tests are wrong. Many of them were borrowed from Paland's printf() implementation which was intended to be a simple solution for embedded systems. We've modified our implementation a lot since then to bring ourselves into closer conformance. For instance, a lot of the double formatting code in our printf() implementation was borrowed from the dtoa authors.

Since you're already a contributor, please ping me when this is ready to merge. Then I'll approve it.

@G4Vi G4Vi changed the title [FEEDBACK REQ] Fix stdio fmt of "%.0e" and "%.0g" Fix stdio fmt of "%.0e" and "%.0g" Aug 18, 2022
@G4Vi
Copy link
Collaborator Author

G4Vi commented Aug 18, 2022

@jart Thanks for the quick reviews. I fixed the %g tests to match glibc and added more. LGTM.

Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Thank you!

@jart jart merged commit 897e33c into jart:master Aug 18, 2022
@G4Vi G4Vi deleted the sprintf_fixes branch August 19, 2022 16:20
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.

stdio fmt issues with "%0.e" and "%.0g"
2 participants