This is an archive of the discontinued LLVM Phabricator instance.

[libc] add exponent format to printf
ClosedPublic

Authored by michaelrj on Dec 14 2022, 11:28 AM.

Details

Summary

Add support for the %e/E conversion in printf, as well as unit tests. It
does not yet support long doubles.

Diff Detail

Event Timeline

michaelrj created this revision.Dec 14 2022, 11:28 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 14 2022, 11:28 AM
michaelrj requested review of this revision.Dec 14 2022, 11:28 AM

Mostly LGTM with a question inline.

libc/src/stdio/printf_core/float_dec_converter.h
191

Is -exponent guaranteed to not overflow here? If not, then may be you should just let IntegerToString handle the sign for negative numbers, but add a + if exponent is non-negative.

michaelrj marked an inline comment as done.Dec 21 2022, 2:17 PM
michaelrj added inline comments.
libc/src/stdio/printf_core/float_dec_converter.h
191

yes exponent is guaranteed not to overflow. It is an int (usually 32 bits, but a minimum of 16) and long doubles have a maximum of 15 bits for their exponent.

sivachandra added inline comments.Dec 21 2022, 2:28 PM
libc/src/stdio/printf_core/float_dec_converter.h
191

Ah, OK! It will be good to include a note with this information and a static assert to go with it. Some thing like:

static_assert(FloatProperties<long double>::EXPONENT_WIDTH < sizeof(int));
sivachandra accepted this revision.Dec 21 2022, 2:31 PM
This revision is now accepted and ready to land.Dec 21 2022, 2:31 PM
michaelrj updated this revision to Diff 484685.Dec 21 2022, 2:40 PM
michaelrj marked 2 inline comments as done.

add comment and assert explaining why the sign handling is always safe.

This revision was automatically updated to reflect the committed changes.