[libc-commits] [libc] [libc][NFC] clean up printf_core and scanf_core (PR #74535)

via libc-commits libc-commits at lists.llvm.org
Wed Dec 20 15:12:42 PST 2023


https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/74535

>From 53b35cca5b21f9ed2b163cfd86ca2ce63bd01efd Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 5 Dec 2023 15:13:51 -0800
Subject: [PATCH 1/2] [libc][NFC] clean up printf_core and scanf_core

Add LIBC_INLINE annotations to functions and fix variable cases within
printf_core and scanf_core.
---
 libc/src/stdio/printf_core/core_structs.h     | 12 +++----
 .../stdio/printf_core/float_dec_converter.h   | 31 ++++++++++---------
 libc/src/stdio/printf_core/writer.h           |  2 +-
 libc/src/stdio/scanf_core/core_structs.h      |  2 +-
 libc/src/stdio/scanf_core/reader.h            |  9 +++---
 5 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/libc/src/stdio/printf_core/core_structs.h b/libc/src/stdio/printf_core/core_structs.h
index 37538362fa3e72..7634d45568ab84 100644
--- a/libc/src/stdio/printf_core/core_structs.h
+++ b/libc/src/stdio/printf_core/core_structs.h
@@ -53,7 +53,7 @@ struct FormatSection {
 
   // This operator is only used for testing and should be automatically
   // optimized out for release builds.
-  bool operator==(const FormatSection &other) const {
+  LIBC_INLINE bool operator==(const FormatSection &other) const {
     if (has_conv != other.has_conv)
       return false;
 
@@ -93,11 +93,11 @@ template <typename T> LIBC_INLINE constexpr TypeDesc type_desc_from_type() {
   if constexpr (cpp::is_same_v<T, void>) {
     return TypeDesc{0, PrimaryType::Unknown};
   } else {
-    constexpr bool isPointer = cpp::is_pointer_v<T>;
-    constexpr bool isFloat = cpp::is_floating_point_v<T>;
-    return TypeDesc{sizeof(T), isPointer ? PrimaryType::Pointer
-                               : isFloat ? PrimaryType::Float
-                                         : PrimaryType::Integer};
+    constexpr bool IS_POINTER = cpp::is_pointer_v<T>;
+    constexpr bool IS_FLOAT = cpp::is_floating_point_v<T>;
+    return TypeDesc{sizeof(T), IS_POINTER ? PrimaryType::Pointer
+                               : IS_FLOAT ? PrimaryType::Float
+                                          : PrimaryType::Integer};
   }
 }
 
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 458f494d5edfda..97906c8cf8cdaa 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -103,14 +103,14 @@ class PaddingWriter {
   size_t min_width = 0;
 
 public:
-  PaddingWriter() {}
-  PaddingWriter(const FormatSection &to_conv, char init_sign_char)
+  LIBC_INLINE PaddingWriter() {}
+  LIBC_INLINE PaddingWriter(const FormatSection &to_conv, char init_sign_char)
       : left_justified((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) > 0),
         leading_zeroes((to_conv.flags & FormatFlags::LEADING_ZEROES) > 0),
         sign_char(init_sign_char),
         min_width(to_conv.min_width > 0 ? to_conv.min_width : 0) {}
 
-  int write_left_padding(Writer *writer, size_t total_digits) {
+  LIBC_INLINE int write_left_padding(Writer *writer, size_t total_digits) {
     // The pattern is (spaces) (sign) (zeroes), but only one of spaces and
     // zeroes can be written, and only if the padding amount is positive.
     int padding_amount =
@@ -133,7 +133,7 @@ class PaddingWriter {
     return 0;
   }
 
-  int write_right_padding(Writer *writer, size_t total_digits) {
+  LIBC_INLINE int write_right_padding(Writer *writer, size_t total_digits) {
     // If and only if the conversion is left justified, there may be trailing
     // spaces.
     int padding_amount =
@@ -170,7 +170,7 @@ class FloatWriter {
   Writer *writer;                   // Writes to the final output.
   PaddingWriter padding_writer; // Handles prefixes/padding, uses total_digits.
 
-  int flush_buffer(bool round_up_max_blocks = false) {
+  LIBC_INLINE int flush_buffer(bool round_up_max_blocks = false) {
     const char MAX_BLOCK_DIGIT = (round_up_max_blocks ? '0' : '9');
 
     // Write the most recent buffered block, and mark has_written
@@ -249,17 +249,18 @@ class FloatWriter {
                 (sizeof(int) * 8));
 
 public:
-  FloatWriter(Writer *init_writer, bool init_has_decimal_point,
-              const PaddingWriter &init_padding_writer)
+  LIBC_INLINE FloatWriter(Writer *init_writer, bool init_has_decimal_point,
+                          const PaddingWriter &init_padding_writer)
       : has_decimal_point(init_has_decimal_point), writer(init_writer),
         padding_writer(init_padding_writer) {}
 
-  void init(size_t init_total_digits, size_t init_digits_before_decimal) {
+  LIBC_INLINE void init(size_t init_total_digits,
+                        size_t init_digits_before_decimal) {
     total_digits = init_total_digits;
     digits_before_decimal = init_digits_before_decimal;
   }
 
-  void write_first_block(BlockInt block, bool exp_format = false) {
+  LIBC_INLINE void write_first_block(BlockInt block, bool exp_format = false) {
     const DecimalString buf(block);
     const cpp::string_view int_to_str = buf.view();
     size_t digits_buffered = int_to_str.size();
@@ -280,7 +281,7 @@ class FloatWriter {
     }
   }
 
-  int write_middle_block(BlockInt block) {
+  LIBC_INLINE int write_middle_block(BlockInt block) {
     if (block == MAX_BLOCK) { // Buffer max blocks in case of rounding
       ++max_block_count;
     } else { // If a non-max block has been found
@@ -301,9 +302,9 @@ class FloatWriter {
     return 0;
   }
 
-  int write_last_block(BlockInt block, size_t block_digits,
-                       RoundDirection round, int exponent = 0,
-                       char exp_char = '\0') {
+  LIBC_INLINE int write_last_block(BlockInt block, size_t block_digits,
+                                   RoundDirection round, int exponent = 0,
+                                   char exp_char = '\0') {
     bool has_exp = (exp_char != '\0');
 
     char end_buff[BLOCK_SIZE];
@@ -458,13 +459,13 @@ class FloatWriter {
     return WRITE_OK;
   }
 
-  int write_zeroes(uint32_t num_zeroes) {
+  LIBC_INLINE int write_zeroes(uint32_t num_zeroes) {
     RET_IF_RESULT_NEGATIVE(flush_buffer());
     RET_IF_RESULT_NEGATIVE(writer->write('0', num_zeroes));
     return 0;
   }
 
-  int right_pad() {
+  LIBC_INLINE int right_pad() {
     return padding_writer.write_right_padding(writer, total_digits);
   }
 };
diff --git a/libc/src/stdio/printf_core/writer.h b/libc/src/stdio/printf_core/writer.h
index e4f503abc34c52..67513eca972883 100644
--- a/libc/src/stdio/printf_core/writer.h
+++ b/libc/src/stdio/printf_core/writer.h
@@ -45,7 +45,7 @@ struct WriteBuffer {
   // write as much of new_str to the buffer as it can. The current position in
   // the buffer will be reset iff stream_writer is called. Calling this with an
   // empty string will flush the buffer if relevant.
-  int overflow_write(cpp::string_view new_str) {
+  LIBC_INLINE int overflow_write(cpp::string_view new_str) {
     // If there is a stream_writer, write the contents of the buffer, then
     // new_str, then clear the buffer.
     if (stream_writer != nullptr) {
diff --git a/libc/src/stdio/scanf_core/core_structs.h b/libc/src/stdio/scanf_core/core_structs.h
index 246f770e0cabed..5f13287ec41dd6 100644
--- a/libc/src/stdio/scanf_core/core_structs.h
+++ b/libc/src/stdio/scanf_core/core_structs.h
@@ -46,7 +46,7 @@ struct FormatSection {
 
   cpp::bitset<256> scan_set;
 
-  bool operator==(const FormatSection &other) {
+  LIBC_INLINE bool operator==(const FormatSection &other) {
     if (has_conv != other.has_conv)
       return false;
 
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index d8647fe2c4ec7b..f750c4341a8d75 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -38,10 +38,11 @@ class Reader {
 
 public:
   // TODO: Set buff_len with a proper constant
-  Reader(ReadBuffer *string_buffer) : rb(string_buffer) {}
+  LIBC_INLINE Reader(ReadBuffer *string_buffer) : rb(string_buffer) {}
 
-  Reader(void *stream, StreamGetc stream_getc_in, StreamUngetc stream_ungetc_in,
-         ReadBuffer *stream_buffer = nullptr)
+  LIBC_INLINE Reader(void *stream, StreamGetc stream_getc_in,
+                     StreamUngetc stream_ungetc_in,
+                     ReadBuffer *stream_buffer = nullptr)
       : rb(stream_buffer), input_stream(stream), stream_getc(stream_getc_in),
         stream_ungetc(stream_ungetc_in) {}
 
@@ -63,7 +64,7 @@ class Reader {
   // this is a file reader, else c is ignored.
   void ungetc(char c);
 
-  size_t chars_read() { return cur_chars_read; }
+  LIBC_INLINE size_t chars_read() { return cur_chars_read; }
 };
 
 } // namespace scanf_core

>From 69ecbfcb9cb9d4298665b8c0aedc8c9e66ee67dc Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 5 Dec 2023 15:45:29 -0800
Subject: [PATCH 2/2] clean up unnecessary includes.

---
 libc/src/stdio/printf_core/char_converter.h          | 6 ++----
 libc/src/stdio/printf_core/converter_utils.h         | 1 -
 libc/src/stdio/printf_core/float_dec_converter.h     | 4 ----
 libc/src/stdio/printf_core/float_hex_converter.h     | 2 --
 libc/src/stdio/printf_core/float_inf_nan_converter.h | 1 -
 libc/src/stdio/printf_core/int_converter.h           | 1 -
 libc/src/stdio/printf_core/parser.h                  | 3 ---
 libc/src/stdio/printf_core/ptr_converter.h           | 3 ---
 libc/src/stdio/printf_core/string_converter.h        | 1 -
 libc/src/stdio/printf_core/write_int_converter.h     | 1 -
 libc/src/stdio/printf_core/writer.cpp                | 2 --
 libc/src/stdio/scanf_core/converter_utils.h          | 2 --
 libc/src/stdio/scanf_core/core_structs.h             | 1 -
 libc/src/stdio/scanf_core/current_pos_converter.h    | 1 -
 libc/src/stdio/scanf_core/parser.h                   | 1 -
 15 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/libc/src/stdio/printf_core/char_converter.h b/libc/src/stdio/printf_core/char_converter.h
index 9b1501ff24b0d8..13596b8ed4f23b 100644
--- a/libc/src/stdio/printf_core/char_converter.h
+++ b/libc/src/stdio/printf_core/char_converter.h
@@ -9,8 +9,6 @@
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
 
-#include "src/__support/CPP/string_view.h"
-#include "src/__support/common.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
@@ -21,10 +19,10 @@ namespace printf_core {
 LIBC_INLINE int convert_char(Writer *writer, const FormatSection &to_conv) {
   char c = static_cast<char>(to_conv.conv_val_raw);
 
-  constexpr int string_len = 1;
+  constexpr int STRING_LEN = 1;
 
   size_t padding_spaces =
-      to_conv.min_width > string_len ? to_conv.min_width - string_len : 0;
+      to_conv.min_width > STRING_LEN ? to_conv.min_width - STRING_LEN : 0;
 
   // If the padding is on the left side, write the spaces first.
   if (padding_spaces > 0 &&
diff --git a/libc/src/stdio/printf_core/converter_utils.h b/libc/src/stdio/printf_core/converter_utils.h
index 4540bba6346e2f..54f0a870d0ac4a 100644
--- a/libc/src/stdio/printf_core/converter_utils.h
+++ b/libc/src/stdio/printf_core/converter_utils.h
@@ -10,7 +10,6 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_UTILS_H
 
 #include "src/__support/CPP/limits.h"
-#include "src/__support/common.h"
 #include "src/stdio/printf_core/core_structs.h"
 
 #include <inttypes.h>
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 97906c8cf8cdaa..798bb955cca145 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -10,13 +10,9 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FLOAT_DEC_CONVERTER_H
 
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/FPUtil/FEnvImpl.h"
 #include "src/__support/FPUtil/FPBits.h"
 #include "src/__support/FPUtil/FloatProperties.h"
 #include "src/__support/FPUtil/rounding_mode.h"
-#include "src/__support/UInt.h"
-#include "src/__support/UInt128.h"
-#include "src/__support/common.h"
 #include "src/__support/float_to_string.h"
 #include "src/__support/integer_to_string.h"
 #include "src/__support/libc_assert.h"
diff --git a/libc/src/stdio/printf_core/float_hex_converter.h b/libc/src/stdio/printf_core/float_hex_converter.h
index a3a8c0420beffc..5ccae81b430c57 100644
--- a/libc/src/stdio/printf_core/float_hex_converter.h
+++ b/libc/src/stdio/printf_core/float_hex_converter.h
@@ -10,10 +10,8 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FLOAT_HEX_CONVERTER_H
 
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/FPUtil/FEnvImpl.h"
 #include "src/__support/FPUtil/FPBits.h"
 #include "src/__support/FPUtil/rounding_mode.h"
-#include "src/__support/common.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/float_inf_nan_converter.h"
diff --git a/libc/src/stdio/printf_core/float_inf_nan_converter.h b/libc/src/stdio/printf_core/float_inf_nan_converter.h
index a0310dc88b5604..8669dc374cb297 100644
--- a/libc/src/stdio/printf_core/float_inf_nan_converter.h
+++ b/libc/src/stdio/printf_core/float_inf_nan_converter.h
@@ -10,7 +10,6 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FLOAT_INF_NAN_CONVERTER_H
 
 #include "src/__support/FPUtil/FPBits.h"
-#include "src/__support/common.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
diff --git a/libc/src/stdio/printf_core/int_converter.h b/libc/src/stdio/printf_core/int_converter.h
index 13fcf3f1aa2eda..7744d801cbc189 100644
--- a/libc/src/stdio/printf_core/int_converter.h
+++ b/libc/src/stdio/printf_core/int_converter.h
@@ -11,7 +11,6 @@
 
 #include "src/__support/CPP/span.h"
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/common.h"
 #include "src/__support/integer_to_string.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
diff --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index f1994517e1ab1c..ab491655275fb9 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -10,9 +10,6 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PARSER_H
 
 #include "src/__support/CPP/optional.h"
-#include "src/__support/CPP/type_traits.h"
-#include "src/__support/arg_list.h"
-#include "src/__support/common.h"
 #include "src/__support/str_to_integer.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/printf_config.h"
diff --git a/libc/src/stdio/printf_core/ptr_converter.h b/libc/src/stdio/printf_core/ptr_converter.h
index 73c6e608a59a78..c5d4086647ec3c 100644
--- a/libc/src/stdio/printf_core/ptr_converter.h
+++ b/libc/src/stdio/printf_core/ptr_converter.h
@@ -9,9 +9,6 @@
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PTR_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PTR_CONVERTER_H
 
-#include "src/__support/CPP/string_view.h"
-#include "src/__support/common.h"
-#include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/int_converter.h"
 #include "src/stdio/printf_core/string_converter.h"
diff --git a/libc/src/stdio/printf_core/string_converter.h b/libc/src/stdio/printf_core/string_converter.h
index 158315311e9ead..04dc5a06da2226 100644
--- a/libc/src/stdio/printf_core/string_converter.h
+++ b/libc/src/stdio/printf_core/string_converter.h
@@ -10,7 +10,6 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_CONVERTER_H
 
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/common.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
diff --git a/libc/src/stdio/printf_core/write_int_converter.h b/libc/src/stdio/printf_core/write_int_converter.h
index 35cafacd5a8c1e..0310905f36f146 100644
--- a/libc/src/stdio/printf_core/write_int_converter.h
+++ b/libc/src/stdio/printf_core/write_int_converter.h
@@ -9,7 +9,6 @@
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_WRITE_INT_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_WRITE_INT_CONVERTER_H
 
-#include "src/__support/CPP/limits.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
 
diff --git a/libc/src/stdio/printf_core/writer.cpp b/libc/src/stdio/printf_core/writer.cpp
index c831ca14c9d917..f8ecd829af3a6d 100644
--- a/libc/src/stdio/printf_core/writer.cpp
+++ b/libc/src/stdio/printf_core/writer.cpp
@@ -8,9 +8,7 @@
 
 #include "writer.h"
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/macros/optimization.h"
 #include "src/stdio/printf_core/core_structs.h"
-#include "src/string/memory_utils/inline_memcpy.h"
 #include "src/string/memory_utils/inline_memset.h"
 #include <stddef.h>
 
diff --git a/libc/src/stdio/scanf_core/converter_utils.h b/libc/src/stdio/scanf_core/converter_utils.h
index a14f35796d27f2..a25e8a73e99a4e 100644
--- a/libc/src/stdio/scanf_core/converter_utils.h
+++ b/libc/src/stdio/scanf_core/converter_utils.h
@@ -9,11 +9,9 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_CONVERTER_UTILS_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_CONVERTER_UTILS_H
 
-#include "src/__support/common.h"
 #include "src/__support/ctype_utils.h"
 #include "src/__support/str_to_float.h"
 #include "src/stdio/scanf_core/core_structs.h"
-#include "src/stdio/scanf_core/reader.h"
 
 #include <stddef.h>
 
diff --git a/libc/src/stdio/scanf_core/core_structs.h b/libc/src/stdio/scanf_core/core_structs.h
index 5f13287ec41dd6..29e1bf2e47f393 100644
--- a/libc/src/stdio/scanf_core/core_structs.h
+++ b/libc/src/stdio/scanf_core/core_structs.h
@@ -11,7 +11,6 @@
 
 #include "src/__support/CPP/bitset.h"
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/FPUtil/FPBits.h"
 
 #include <inttypes.h>
 #include <stddef.h>
diff --git a/libc/src/stdio/scanf_core/current_pos_converter.h b/libc/src/stdio/scanf_core/current_pos_converter.h
index fd62383e394092..be25cefed151a9 100644
--- a/libc/src/stdio/scanf_core/current_pos_converter.h
+++ b/libc/src/stdio/scanf_core/current_pos_converter.h
@@ -9,7 +9,6 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_CURRENT_POS_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_CURRENT_POS_CONVERTER_H
 
-#include "src/__support/common.h"
 #include "src/stdio/scanf_core/converter_utils.h"
 #include "src/stdio/scanf_core/core_structs.h"
 #include "src/stdio/scanf_core/reader.h"
diff --git a/libc/src/stdio/scanf_core/parser.h b/libc/src/stdio/scanf_core/parser.h
index 7f3a53be357008..5ae9009bc4a231 100644
--- a/libc/src/stdio/scanf_core/parser.h
+++ b/libc/src/stdio/scanf_core/parser.h
@@ -10,7 +10,6 @@
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_PARSER_H
 
 #include "src/__support/arg_list.h"
-#include "src/__support/common.h"
 #include "src/__support/ctype_utils.h"
 #include "src/__support/str_to_integer.h"
 #include "src/stdio/scanf_core/core_structs.h"



More information about the libc-commits mailing list