This is an archive of the discontinued LLVM Phabricator instance.

[libc] add printf hexadecimal float conversion
ClosedPublic

Authored by michaelrj on Jul 6 2022, 4:55 PM.

Details

Summary

This patch adds the %a/A conversions to printf, as well as the compiler
flag to disable floating point handling entirely. This will allow our
printf implementation to display every type of argument allowed by
printf, although some formats are still incomplete.

Diff Detail

Event Timeline

michaelrj created this revision.Jul 6 2022, 4:55 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 6 2022, 4:55 PM
michaelrj requested review of this revision.Jul 6 2022, 4:55 PM
sivachandra accepted this revision.Jul 8 2022, 10:14 AM

Few nits inline.

libc/src/stdio/printf_core/converter.cpp
47

A comment for future: Do we have the CMake plumbing for any of these? If not, we should probably do it and have explicitly listed tests for them?

libc/src/stdio/printf_core/float_hex_converter.h
104

Why should this be static?

117

Same here, why static?

174

Ditto.

214

But is missing here!

218

And here!

libc/test/src/stdio/sprintf_test.cpp
20

What is this comment about?

This revision is now accepted and ready to land.Jul 8 2022, 10:14 AM
michaelrj marked 6 inline comments as done.Jul 8 2022, 10:31 AM
michaelrj added inline comments.
libc/src/stdio/printf_core/converter.cpp
47

There isn't CMake plumbing for these yet, I was planning on waiting until the main code was done to work on the cmake. The tests in sprintf_test.cpp are already set up to work when pieces are disabled by using the same flags, although I haven't added anything to test behavior when pieces are disabled.

libc/src/stdio/printf_core/float_hex_converter.h
104

I don't remember, why I had some constexpr variables marked as static, so I've changed them all to not be static.

libc/test/src/stdio/sprintf_test.cpp
20

Ah, I forgot to remove that. I used it to make sure that my tests were correct by switching my implementation of printf with the system's.

michaelrj updated this revision to Diff 443284.Jul 8 2022, 10:34 AM
michaelrj marked 2 inline comments as done.

address comments

michaelrj updated this revision to Diff 443380.Jul 8 2022, 3:57 PM

comment out unused conversion specifiers to prevent unexpected behavior

This revision was landed with ongoing or failed builds.Jul 8 2022, 3:58 PM
This revision was automatically updated to reflect the committed changes.