Skip to content

Commit 096463d

Browse files
michaelrj-googlemichaelrj-google
committed
[libc] move printf to use StringViews
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. Reviewed By: sivachandra Differential Revision: https://reviews.llvm.org/D131994
1 parent 3d68a67 commit 096463d

27 files changed

+374
-299
lines changed

‎libc/src/stdio/printf_core/CMakeLists.txt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ add_header_library(
33
core_structs
44
HDRS
55
core_structs.h
6+
DEPENDS
7+
libc.src.__support.CPP.string_view
8+
libc.src.__support.FPUtil.fputil
69
)
710

811
add_object_library(
@@ -17,7 +20,7 @@ add_object_library(
1720
libc.src.__support.ctype_utils
1821
libc.src.__support.str_to_integer
1922
libc.src.__support.CPP.bit
20-
libc.src.string.memory_utils.memset_implementation
23+
libc.src.__support.CPP.string_view
2124
)
2225

2326
add_object_library(
@@ -27,7 +30,9 @@ add_object_library(
2730
HDRS
2831
string_writer.h
2932
DEPENDS
33+
libc.src.__support.CPP.string_view
3034
libc.src.string.memory_utils.memcpy_implementation
35+
libc.src.string.memory_utils.memset_implementation
3136
.core_structs
3237
)
3338

@@ -37,8 +42,6 @@ add_object_library(
3742
writer.cpp
3843
HDRS
3944
writer.h
40-
DEPENDS
41-
libc.src.string.memory_utils.memset_implementation
4245
)
4346

4447
add_object_library(
@@ -95,6 +98,8 @@ add_object_library(
9598
file_writer.h
9699
DEPENDS
97100
libc.src.__support.File.file
101+
libc.src.__support.CPP.string_view
102+
libc.src.string.memory_utils.memset_implementation
98103
.core_structs
99104
)
100105

‎libc/src/stdio/printf_core/char_converter.h

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
1010
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
1111

12+
#include "src/__support/CPP/string_view.h"
1213
#include "src/stdio/printf_core/converter_utils.h"
1314
#include "src/stdio/printf_core/core_structs.h"
1415
#include "src/stdio/printf_core/writer.h"
@@ -19,18 +20,25 @@ namespace printf_core {
1920
int inline convert_char(Writer *writer, const FormatSection &to_conv) {
2021
char c = to_conv.conv_val_raw;
2122

22-
if (to_conv.min_width > 1) {
23-
if ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) ==
24-
FormatFlags::LEFT_JUSTIFIED) {
25-
RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
26-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', to_conv.min_width - 1));
27-
} else {
28-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', to_conv.min_width - 1));
29-
RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
30-
}
31-
} else {
32-
RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
23+
constexpr int string_len = 1;
24+
25+
size_t padding_spaces =
26+
to_conv.min_width > string_len ? to_conv.min_width - string_len : 0;
27+
28+
// If the padding is on the left side, write the spaces first.
29+
if (padding_spaces > 0 &&
30+
(to_conv.flags & FormatFlags::LEFT_JUSTIFIED) == 0) {
31+
RET_IF_RESULT_NEGATIVE(writer->write(' ', to_conv.min_width - string_len));
32+
}
33+
34+
RET_IF_RESULT_NEGATIVE(writer->write(c));
35+
36+
// If the padding is on the right side, write the spaces last.
37+
if (padding_spaces > 0 &&
38+
(to_conv.flags & FormatFlags::LEFT_JUSTIFIED) != 0) {
39+
RET_IF_RESULT_NEGATIVE(writer->write(' ', to_conv.min_width - string_len));
3340
}
41+
3442
return WRITE_OK;
3543
}
3644

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ namespace printf_core {
2626

2727
int convert(Writer *writer, const FormatSection &to_conv) {
2828
if (!to_conv.has_conv)
29-
return writer->write(to_conv.raw_string, to_conv.raw_len);
29+
return writer->write(to_conv.raw_string);
3030

3131
switch (to_conv.conv_name) {
3232
case '%':
33-
return writer->write("%", 1);
33+
return writer->write("%");
3434
case 'c':
3535
return convert_char(writer, to_conv);
3636
case 's':
@@ -63,7 +63,7 @@ int convert(Writer *writer, const FormatSection &to_conv) {
6363
case 'p':
6464
return convert_pointer(writer, to_conv);
6565
default:
66-
return writer->write(to_conv.raw_string, to_conv.raw_len);
66+
return writer->write(to_conv.raw_string);
6767
}
6868
return -1;
6969
}

‎libc/src/stdio/printf_core/core_structs.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ enum FormatFlags : uint8_t {
3737
struct FormatSection {
3838
bool has_conv;
3939

40-
const char *__restrict raw_string;
41-
size_t raw_len;
40+
cpp::string_view raw_string;
4241

4342
// Format Specifier Values
4443
FormatFlags flags = FormatFlags(0);
@@ -58,8 +57,7 @@ struct FormatSection {
5857
if (has_conv != other.has_conv)
5958
return false;
6059

61-
if (cpp::string_view(raw_string, raw_len) !=
62-
cpp::string_view(other.raw_string, other.raw_len))
60+
if (raw_string != other.raw_string)
6361
return false;
6462

6563
if (has_conv) {

‎libc/src/stdio/printf_core/file_writer.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/stdio/printf_core/file_writer.h"
10+
#include "src/__support/CPP/string_view.h"
1011
#include "src/__support/File/file.h"
1112
#include "src/stdio/printf_core/core_structs.h"
1213
#include <stddef.h>
@@ -23,10 +24,29 @@ int FileWriter::write(const char *__restrict to_write, size_t len) {
2324
return written;
2425
}
2526

26-
int write_to_file(void *raw_pointer, const char *__restrict to_write,
27-
size_t len) {
27+
int FileWriter::write_str(void *raw_pointer, cpp::string_view new_string) {
2828
FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
29-
return file_writer->write(to_write, len);
29+
return file_writer->write(new_string.data(), new_string.size());
30+
}
31+
32+
int FileWriter::write_chars(void *raw_pointer, char new_char, size_t len) {
33+
FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
34+
constexpr size_t BUFF_SIZE = 8;
35+
char buff[BUFF_SIZE] = {new_char};
36+
int result;
37+
while (len > BUFF_SIZE) {
38+
result = file_writer->write(buff, BUFF_SIZE);
39+
if (result < 0)
40+
return result;
41+
len -= BUFF_SIZE;
42+
}
43+
return file_writer->write(buff, len);
44+
}
45+
46+
// TODO(michaelrj): Move this to putc_unlocked once that is available.
47+
int FileWriter::write_char(void *raw_pointer, char new_char) {
48+
FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
49+
return file_writer->write(&new_char, 1);
3050
}
3151

3252
} // namespace printf_core

‎libc/src/stdio/printf_core/file_writer.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H
1010
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H
1111

12+
#include "src/__support/CPP/string_view.h"
1213
#include "src/__support/File/file.h"
1314

1415
#include <stddef.h>
@@ -29,12 +30,13 @@ class FileWriter {
2930
~FileWriter() { file->unlock(); }
3031

3132
int write(const char *__restrict to_write, size_t len);
32-
};
3333

34-
// write_to_file treats raw_pointer as a File and calls its write
35-
// function.
36-
int write_to_file(void *raw_pointer, const char *__restrict to_write,
37-
size_t len);
34+
// These write functions take a FileWriter as a void* in raw_pointer, and
35+
// call the appropriate write function on it.
36+
static int write_str(void *raw_pointer, cpp::string_view new_string);
37+
static int write_chars(void *raw_pointer, char new_char, size_t len);
38+
static int write_char(void *raw_pointer, char new_char);
39+
};
3840

3941
} // namespace printf_core
4042
} // namespace __llvm_libc

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

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FLOAT_HEX_CONVERTER_H
1010
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FLOAT_HEX_CONVERTER_H
1111

12+
#include "src/__support/CPP/string_view.h"
1213
#include "src/__support/FPUtil/FEnvImpl.h"
1314
#include "src/__support/FPUtil/FPBits.h"
1415
#include "src/stdio/printf_core/converter_utils.h"
@@ -193,6 +194,7 @@ int inline convert_float_hex_exp(Writer *writer, const FormatSection &to_conv) {
193194
char prefix[PREFIX_LEN];
194195
prefix[0] = '0';
195196
prefix[1] = a + ('x' - 'a');
197+
const cpp::string_view prefix_str(prefix, PREFIX_LEN);
196198

197199
// If the precision is greater than the actual result, pad with 0s
198200
if (to_conv.precision > static_cast<int>(mant_digits - 1))
@@ -201,7 +203,7 @@ int inline convert_float_hex_exp(Writer *writer, const FormatSection &to_conv) {
201203
bool has_hexadecimal_point =
202204
(mant_digits > 1) || ((to_conv.flags & FormatFlags::ALTERNATE_FORM) ==
203205
FormatFlags::ALTERNATE_FORM);
204-
constexpr char HEXADECIMAL_POINT = '.';
206+
constexpr cpp::string_view HEXADECIMAL_POINT(".");
205207

206208
// This is for the letter 'p' before the exponent.
207209
const char exp_seperator = a + ('p' - 'a');
@@ -218,42 +220,42 @@ int inline convert_float_hex_exp(Writer *writer, const FormatSection &to_conv) {
218220
// The pattern is (sign), 0x, digit, (.), (other digits), (zeroes), p,
219221
// exponent, (spaces)
220222
if (sign_char > 0)
221-
RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
222-
RET_IF_RESULT_NEGATIVE(writer->write(prefix, PREFIX_LEN));
223-
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer, 1));
223+
RET_IF_RESULT_NEGATIVE(writer->write(sign_char));
224+
RET_IF_RESULT_NEGATIVE(writer->write(prefix_str));
225+
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer[0]));
224226
if (has_hexadecimal_point)
225-
RET_IF_RESULT_NEGATIVE(writer->write(&HEXADECIMAL_POINT, 1));
227+
RET_IF_RESULT_NEGATIVE(writer->write(HEXADECIMAL_POINT));
226228
if (mant_digits > 1)
227-
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer + 1, mant_digits - 1));
229+
RET_IF_RESULT_NEGATIVE(writer->write({mant_buffer + 1, mant_digits - 1}));
228230
if (trailing_zeroes > 0)
229-
RET_IF_RESULT_NEGATIVE(writer->write_chars('0', trailing_zeroes));
230-
RET_IF_RESULT_NEGATIVE(writer->write(&exp_seperator, EXP_SEPERATOR_LEN));
231+
RET_IF_RESULT_NEGATIVE(writer->write('0', trailing_zeroes));
232+
RET_IF_RESULT_NEGATIVE(writer->write(exp_seperator));
231233
RET_IF_RESULT_NEGATIVE(
232-
writer->write(exp_buffer + exp_cur, EXP_LEN - exp_cur));
234+
writer->write({exp_buffer + exp_cur, EXP_LEN - exp_cur}));
233235
if (padding > 0)
234-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', padding));
236+
RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
235237
} else {
236238
// The pattern is (spaces), (sign), 0x, (zeroes), digit, (.), (other
237239
// digits), (zeroes), p, exponent
238240
if ((padding > 0) && ((to_conv.flags & FormatFlags::LEADING_ZEROES) !=
239241
FormatFlags::LEADING_ZEROES))
240-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', padding));
242+
RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
241243
if (sign_char > 0)
242-
RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
243-
RET_IF_RESULT_NEGATIVE(writer->write(prefix, PREFIX_LEN));
244+
RET_IF_RESULT_NEGATIVE(writer->write(sign_char));
245+
RET_IF_RESULT_NEGATIVE(writer->write(prefix_str));
244246
if ((padding > 0) && ((to_conv.flags & FormatFlags::LEADING_ZEROES) ==
245247
FormatFlags::LEADING_ZEROES))
246-
RET_IF_RESULT_NEGATIVE(writer->write_chars('0', padding));
247-
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer, 1));
248+
RET_IF_RESULT_NEGATIVE(writer->write('0', padding));
249+
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer[0]));
248250
if (has_hexadecimal_point)
249-
RET_IF_RESULT_NEGATIVE(writer->write(&HEXADECIMAL_POINT, 1));
251+
RET_IF_RESULT_NEGATIVE(writer->write(HEXADECIMAL_POINT));
250252
if (mant_digits > 1)
251-
RET_IF_RESULT_NEGATIVE(writer->write(mant_buffer + 1, mant_digits - 1));
253+
RET_IF_RESULT_NEGATIVE(writer->write({mant_buffer + 1, mant_digits - 1}));
252254
if (trailing_zeroes > 0)
253-
RET_IF_RESULT_NEGATIVE(writer->write_chars('0', trailing_zeroes));
254-
RET_IF_RESULT_NEGATIVE(writer->write(&exp_seperator, EXP_SEPERATOR_LEN));
255+
RET_IF_RESULT_NEGATIVE(writer->write('0', trailing_zeroes));
256+
RET_IF_RESULT_NEGATIVE(writer->write(exp_seperator));
255257
RET_IF_RESULT_NEGATIVE(
256-
writer->write(exp_buffer + exp_cur, EXP_LEN - exp_cur));
258+
writer->write({exp_buffer + exp_cur, EXP_LEN - exp_cur}));
257259
}
258260
return WRITE_OK;
259261
}

‎libc/src/stdio/printf_core/float_inf_nan_converter.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,19 @@ int inline convert_inf_nan(Writer *writer, const FormatSection &to_conv) {
5959

6060
if (padding > 0 && ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) !=
6161
FormatFlags::LEFT_JUSTIFIED))
62-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', padding));
62+
RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
6363

6464
if (sign_char)
65-
RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
65+
RET_IF_RESULT_NEGATIVE(writer->write(sign_char));
6666
if (mantissa == 0) { // inf
67-
RET_IF_RESULT_NEGATIVE(writer->write((a == 'a' ? "inf" : "INF"), 3));
67+
RET_IF_RESULT_NEGATIVE(writer->write(a == 'a' ? "inf" : "INF"));
6868
} else { // nan
69-
RET_IF_RESULT_NEGATIVE(writer->write((a == 'a' ? "nan" : "NAN"), 3));
69+
RET_IF_RESULT_NEGATIVE(writer->write(a == 'a' ? "nan" : "NAN"));
7070
}
7171

7272
if (padding > 0 && ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) ==
7373
FormatFlags::LEFT_JUSTIFIED))
74-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', padding));
74+
RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
7575

7676
return WRITE_OK;
7777
}

‎libc/src/stdio/printf_core/int_converter.h

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ namespace printf_core {
2727
constexpr char inline to_lower(char a) { return a | 32; }
2828
constexpr bool inline is_lower(char a) { return (a & 32) > 0; }
2929

30-
cpp::optional<cpp::string_view> inline num_to_strview(
31-
uintmax_t num, cpp::span<char> bufref, char conv_name) {
30+
cpp::optional<cpp::string_view> inline num_to_strview(uintmax_t num,
31+
cpp::span<char> bufref,
32+
char conv_name) {
3233
if (to_lower(conv_name) == 'x') {
3334
return IntegerToString::hex(num, bufref, is_lower(conv_name));
3435
} else if (conv_name == 'o') {
@@ -88,7 +89,7 @@ int inline convert_int(Writer *writer, const FormatSection &to_conv) {
8889

8990
// prefix is "0x" for hexadecimal, or the sign character for signed
9091
// conversions. Since hexadecimal is unsigned these will never conflict.
91-
int prefix_len;
92+
size_t prefix_len;
9293
char prefix[2];
9394
if ((to_lower(to_conv.conv_name) == 'x') &&
9495
((flags & FormatFlags::ALTERNATE_FORM) != 0)) {
@@ -141,23 +142,23 @@ int inline convert_int(Writer *writer, const FormatSection &to_conv) {
141142
if ((flags & FormatFlags::LEFT_JUSTIFIED) == FormatFlags::LEFT_JUSTIFIED) {
142143
// If left justified it goes prefix zeroes digits spaces
143144
if (prefix_len != 0)
144-
RET_IF_RESULT_NEGATIVE(writer->write(prefix, prefix_len));
145+
RET_IF_RESULT_NEGATIVE(writer->write({prefix, prefix_len}));
145146
if (zeroes > 0)
146-
RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
147+
RET_IF_RESULT_NEGATIVE(writer->write('0', zeroes));
147148
if (digits_written > 0)
148-
RET_IF_RESULT_NEGATIVE(writer->write(str->data(), digits_written));
149+
RET_IF_RESULT_NEGATIVE(writer->write(*str));
149150
if (spaces > 0)
150-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
151+
RET_IF_RESULT_NEGATIVE(writer->write(' ', spaces));
151152
} else {
152153
// Else it goes spaces prefix zeroes digits
153154
if (spaces > 0)
154-
RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
155+
RET_IF_RESULT_NEGATIVE(writer->write(' ', spaces));
155156
if (prefix_len != 0)
156-
RET_IF_RESULT_NEGATIVE(writer->write(prefix, prefix_len));
157+
RET_IF_RESULT_NEGATIVE(writer->write({prefix, prefix_len}));
157158
if (zeroes > 0)
158-
RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
159+
RET_IF_RESULT_NEGATIVE(writer->write('0', zeroes));
159160
if (digits_written > 0)
160-
RET_IF_RESULT_NEGATIVE(writer->write(str->data(), digits_written));
161+
RET_IF_RESULT_NEGATIVE(writer->write(*str));
161162
}
162163
return WRITE_OK;
163164
}

‎libc/src/stdio/printf_core/parser.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "src/__support/arg_list.h"
1414

1515
#include "src/__support/CPP/bit.h"
16+
#include "src/__support/CPP/string_view.h"
1617
#include "src/__support/FPUtil/FPBits.h"
1718
#include "src/__support/ctype_utils.h"
1819
#include "src/__support/str_to_integer.h"
@@ -28,7 +29,6 @@ namespace printf_core {
2829

2930
FormatSection Parser::get_next_section() {
3031
FormatSection section;
31-
section.raw_string = str + cur_pos;
3232
size_t starting_pos = cur_pos;
3333
if (str[cur_pos] == '%') {
3434
// format section
@@ -158,7 +158,7 @@ FormatSection Parser::get_next_section() {
158158
while (str[cur_pos] != '%' && str[cur_pos] != '\0')
159159
++cur_pos;
160160
}
161-
section.raw_len = cur_pos - starting_pos;
161+
section.raw_string = {str + starting_pos, cur_pos - starting_pos};
162162
return section;
163163
}
164164

0 commit comments

Comments
 (0)