This is an archive of the discontinued LLVM Phabricator instance.

[libc] move printf to use StringViews
ClosedPublic

Authored by michaelrj on Aug 16 2022, 2:05 PM.

Details

Summary

The FormatSection and the writer functions both previously took a char*
and a length to represent a string. Now they use the StringView class to
represent that more succinctly. This change also required fixing
everywhere these were used, so it touches a lot of files.

Diff Detail

Event Timeline

michaelrj created this revision.Aug 16 2022, 2:05 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 16 2022, 2:05 PM
michaelrj requested review of this revision.Aug 16 2022, 2:05 PM
sivachandra added inline comments.Aug 17 2022, 12:56 AM
libc/src/stdio/printf_core/char_converter.h
35

Here and elsewhere, instead of calling the StringView constructor explicitly, can you do {&c, string_len}?

libc/src/stdio/printf_core/file_writer.h
34–39

While we are at it, we should make these static methods of FileWriter and simplify the names to write_str and write_chars.

libc/src/stdio/printf_core/string_writer.h
41

Make these static methods of StringWriter.

libc/src/stdio/printf_core/writer.h
51

Something we should have done earlier is to name all of these methods as just write. So, I think we should have:

int write(cpp::StringView);
int write(char c, size_t count);
int write(char c);

I am suggesting a separate write(char) to avoid calling, for example, memcpy to write a single char to a string buffer. Likewise, the FileWriter can implement a char writer by calling putc_unlocked when it is available. More importantly though, you will avoid the ugliness of writer->write(StringView(&some_char, 1)) at many places in this change.

libc/test/src/stdio/printf_core/parser_test.cpp
57

Can you do:

expected.raw_string = {str, 4};
michaelrj updated this revision to Diff 454993.Aug 23 2022, 3:42 PM
michaelrj marked 5 inline comments as done.

address comments

sivachandra accepted this revision.Aug 23 2022, 11:41 PM

A comment about inconsistent usage of a pattern but good to go otherwise.

libc/test/src/stdio/printf_core/string_writer_test.cpp
36

Can we follow the pattern of {"abc", 3} here and elsewhere in the patch.

This revision is now accepted and ready to land.Aug 23 2022, 11:41 PM
michaelrj marked an inline comment as done.

fix final string_view initialization

This revision was landed with ongoing or failed builds.Aug 24 2022, 10:28 AM
This revision was automatically updated to reflect the committed changes.