[libc][NFC] reorganize structs in printf

[michaelrj]

Summary

Previously the type description structs were defined in the parser. For
the fuzzing targets we'll need to use those, so I've moved them into
core_structs.h. Additionally I've renamed the function for determining
the TypeDesc from a given type. Previously it shared its name with
get_type_desc which is a related but separate function that is a part of
the parser.

[lntue]

libc/src/stdio/printf_core/core_structs.h

93

You can drop static here.

97

Use is_pointer_v from https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/type_traits.h#L80

98

Use cpp::is_floating_point_v from https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/type_traits.h#L92

[michaelrj]

clean up type_desc_from_type

libc/src/stdio/printf_core/core_structs.h

if (has_conv) {
         return (conv_val_ptr == other.conv_val_ptr);
       else if (conv_name != '%')
         return (conv_val_raw == other.conv_val_raw);
     }
     return true;
   }
 };
 
+enum PrimaryType : uint8_t { Integer = 0, Float = 1, Pointer = 2 };
+
+// TypeDesc stores the information about a type that is relevant to printf in
+// a relatively compact manner.
+struct TypeDesc {
+  uint8_t size;
+  PrimaryType primary_type;
+  LIBC_INLINE constexpr bool operator==(const TypeDesc &other) const {
+    return (size == other.size) && (primary_type == other.primary_type);
+  }
+};
+
+template <typename T> LIBC_INLINE constexpr TypeDesc type_desc_from_type() {
+  if constexpr (cpp::is_same_v<T, void>) {
+    return TypeDesc{0, PrimaryType::Integer};
+  } 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};
+  }
+}
+
 // This is the value to be returned by conversions when no error has occurred.
 constexpr int WRITE_OK = 0;
 // These are the printf return values for when an error has occurred. They are
 // all negative, and should be distinct.
 constexpr int FILE_WRITE_ERROR = -1;
 constexpr int FILE_STATUS_ERROR = -2;
 constexpr int NULLPTR_WRITE_ERROR = -3;
 constexpr int INT_CONVERSION_ERROR = -4;
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CORE_STRUCTS_H

libc/src/stdio/printf_core/parser.h

 
 #ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
   // args_start stores the start of the va_args, which is allows getting the
   // value of arguments that have already been passed. args_index is tracked so
   // that we know which argument args_cur is on.
   internal::ArgList args_start;
   size_t args_index = 1;
 
-  enum PrimaryType : uint8_t { Integer = 0, Float = 1, Pointer = 2 };
-
-  // TypeDesc stores the information about a type that is relevant to printf in
-  // a relatively compact manner.
-  struct TypeDesc {
-    uint8_t size;
-    PrimaryType primary_type;
-    LIBC_INLINE constexpr bool operator==(const TypeDesc &other) const {
-      return (size == other.size) && (primary_type == other.primary_type);
-    }
-  };
-
   // Defined in printf_config.h
   static constexpr size_t DESC_ARR_LEN = LLVM_LIBC_PRINTF_INDEX_ARR_LEN;
 
   // desc_arr stores the sizes of the variables in the ArgList. This is used in
   // index mode to reduce repeated string parsing. The sizes are stored as
   // TypeDesc objects, which store the size as well as minimal type information.
   // This is necessary because some systems separate the floating point and
   // integer values in va_args.
#ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
 
   // parse_index parses the index of a value inside a format string. It
   // assumes that str[*local_pos] points to character after a '%' or '*', and
   // returns 0 if there is no closing $, or if it finds no number. If it finds a
   // number, it will move local_pos past the end of the $, else it will not move
   // local_pos.
   size_t parse_index(size_t *local_pos);
 
-  template <typename T> LIBC_INLINE static constexpr TypeDesc get_type_desc() {
-    if constexpr (cpp::is_same_v<T, void>) {
-      return TypeDesc{0, PrimaryType::Integer};
-    } else {
-      constexpr bool isPointer = cpp::is_same_v<T, void *>;
-      constexpr bool isFloat =
-          cpp::is_same_v<T, double> || cpp::is_same_v<T, long double>;
-      return TypeDesc{sizeof(T), isPointer ? PrimaryType::Pointer
-                                 : isFloat ? PrimaryType::Float
-                                           : PrimaryType::Integer};
-    }
-  }
-
   LIBC_INLINE void set_type_desc(size_t index, TypeDesc value) {
     if (index != 0 && index <= DESC_ARR_LEN)
       desc_arr[index - 1] = value;
   }
 
   // get_arg_value gets the value from the arg list at index (starting at 1).
   // This may require parsing the format string. An index of 0 is interpreted as
   // the next value.
   template <class T> LIBC_INLINE T get_arg_value(size_t index) {
     if (!(index == 0 || index == args_index))
       args_to_index(index);
 
-    set_type_desc(index, get_type_desc<T>());
+    set_type_desc(index, type_desc_from_type<T>());
 
     ++args_index;
     return get_next_arg_value<T>();
   }
 
   // the ArgList can only return the next item in the list. This function is
   // used in index mode when the item that needs to be read is not the next one.
   // It moves cur_args to the index requested so the the appropriate value may

libc/src/stdio/printf_core/parser.cpp

 
 #include "src/__support/arg_list.h"
 
 #include "src/__support/CPP/bit.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/FPUtil/FPBits.h"
 #include "src/__support/ctype_utils.h"
 #include "src/__support/str_to_integer.h"
+#include "src/stdio/printf_core/core_structs.h"
 
 namespace __llvm_libc {
 namespace printf_core {
 
 #ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
 #define GET_ARG_VAL_SIMPLEST(arg_type, index) get_arg_value<arg_type>(index)
 #else
 #define GET_ARG_VAL_SIMPLEST(arg_type, _) get_next_arg_value<arg_type>()
if (internal::isdigit(str[*local_pos])) {
     if (str[*local_pos + result.parsed_len] != '$')
       return 0;
     *local_pos = 1 + result.parsed_len + *local_pos;
     return index;
   }
   return 0;
 }
 
-Parser::TypeDesc Parser::get_type_desc(size_t index) {
+TypeDesc Parser::get_type_desc(size_t index) {
   // index mode is assumed, and the indicies start at 1, so an index
   // of 0 is invalid.
   size_t local_pos = 0;
 
   while (str[local_pos]) {
     if (str[local_pos] == '%') {
       ++local_pos;
 
       size_t conv_index = parse_index(&local_pos);
 
       // the flags aren't relevant for this situation, but I need to skip past
       // them so they're parsed but the result is discarded.
       parse_flags(&local_pos);
 
       // handle width
       if (str[local_pos] == '*') {
         ++local_pos;
 
         size_t width_index = parse_index(&local_pos);
-        set_type_desc(width_index, get_type_desc<int>());
+        set_type_desc(width_index, type_desc_from_type<int>());
         if (width_index == index)
-          return get_type_desc<int>();
+          return type_desc_from_type<int>();
 
       } else if (internal::isdigit(str[local_pos])) {
         while (internal::isdigit(str[local_pos]))
           ++local_pos;
       }
 
       // handle precision
       if (str[local_pos] == '.') {
         ++local_pos;
         if (str[local_pos] == '*') {
           ++local_pos;
 
           size_t precision_index = parse_index(&local_pos);
-          set_type_desc(precision_index, get_type_desc<int>());
+          set_type_desc(precision_index, type_desc_from_type<int>());
           if (precision_index == index)
-            return get_type_desc<int>();
+            return type_desc_from_type<int>();
 
         } else if (internal::isdigit(str[local_pos])) {
           while (internal::isdigit(str[local_pos]))
             ++local_pos;
         }
       }
 
       LengthModifier lm = parse_length_modifier(&local_pos);
 
       // if we don't have an index for this conversion, then its position is
       // unknown and all this information is irrelevant. The rest of this logic
       // has been for skipping past this conversion properly to avoid
       // weirdness with %%.
       if (conv_index == 0) {
         ++local_pos;
         continue;
       }
 
-      TypeDesc conv_size = get_type_desc<void>();
+      TypeDesc conv_size = type_desc_from_type<void>();
       switch (str[local_pos]) {
       case ('%'):
-        conv_size = get_type_desc<void>();
+        conv_size = type_desc_from_type<void>();
         break;
       case ('c'):
-        conv_size = get_type_desc<int>();
+        conv_size = type_desc_from_type<int>();
         break;
       case ('d'):
       case ('i'):
       case ('o'):
       case ('x'):
       case ('X'):
       case ('u'):
         switch (lm) {
         case (LengthModifier::hh):
         case (LengthModifier::h):
         case (LengthModifier::none):
-          conv_size = get_type_desc<int>();
+          conv_size = type_desc_from_type<int>();
           break;
         case (LengthModifier::l):
-          conv_size = get_type_desc<long>();
+          conv_size = type_desc_from_type<long>();
           break;
         case (LengthModifier::ll):
         case (LengthModifier::L): // This isn't in the standard, but is in other
                                   // libc implementations.
-          conv_size = get_type_desc<long long>();
+          conv_size = type_desc_from_type<long long>();
           break;
         case (LengthModifier::j):
-          conv_size = get_type_desc<intmax_t>();
+          conv_size = type_desc_from_type<intmax_t>();
           break;
         case (LengthModifier::z):
-          conv_size = get_type_desc<size_t>();
+          conv_size = type_desc_from_type<size_t>();
           break;
         case (LengthModifier::t):
-          conv_size = get_type_desc<ptrdiff_t>();
+          conv_size = type_desc_from_type<ptrdiff_t>();
           break;
         }
         break;
 #ifndef LLVM_LIBC_PRINTF_DISABLE_FLOAT
       case ('f'):
       case ('F'):
       case ('e'):
       case ('E'):
       case ('a'):
       case ('A'):
       case ('g'):
       case ('G'):
         if (lm != LengthModifier::L)
-          conv_size = get_type_desc<double>();
+          conv_size = type_desc_from_type<double>();
         else
-          conv_size = get_type_desc<long double>();
+          conv_size = type_desc_from_type<long double>();
         break;
 #endif // LLVM_LIBC_PRINTF_DISABLE_FLOAT
 #ifndef LLVM_LIBC_PRINTF_DISABLE_WRITE_INT
       case ('n'):
 #endif // LLVM_LIBC_PRINTF_DISABLE_WRITE_INT
       case ('p'):
       case ('s'):
-        conv_size = get_type_desc<void *>();
+        conv_size = type_desc_from_type<void *>();
         break;
       default:
-        conv_size = get_type_desc<int>();
+        conv_size = type_desc_from_type<int>();
         break;
       }
 
       set_type_desc(conv_index, conv_size);
       if (conv_index == index)
         return conv_size;
     }
     // If the end of the format section is on the '\0'. This means we need to
     // not advance the local_pos.
     if (str[local_pos] != '\0')
       ++local_pos;
   }
 
   // If there is no size for the requested index, then just guess that it's an
   // int.
-  return get_type_desc<int>();
+  return type_desc_from_type<int>();
 }
 
 void Parser::args_to_index(size_t index) {
   if (args_index > index) {
     args_index = 1;
     args_cur = args_start;
   }
 
   while (args_index < index) {
-    Parser::TypeDesc cur_type_desc = get_type_desc<void>();
+    TypeDesc cur_type_desc = type_desc_from_type<void>();
     if (args_index <= DESC_ARR_LEN)
       cur_type_desc = desc_arr[args_index - 1];
 
-    if (cur_type_desc == get_type_desc<void>())
+    if (cur_type_desc == type_desc_from_type<void>())
       cur_type_desc = get_type_desc(args_index);
 
-    if (cur_type_desc == get_type_desc<uint32_t>())
+    if (cur_type_desc == type_desc_from_type<uint32_t>())
       args_cur.next_var<uint32_t>();
-    else if (cur_type_desc == get_type_desc<uint64_t>())
+    else if (cur_type_desc == type_desc_from_type<uint64_t>())
       args_cur.next_var<uint64_t>();
 #ifndef LLVM_LIBC_PRINTF_DISABLE_FLOAT
     // Floating point numbers are stored separately from the other arguments.
-    else if (cur_type_desc == get_type_desc<double>())
+    else if (cur_type_desc == type_desc_from_type<double>())
       args_cur.next_var<double>();
-    else if (cur_type_desc == get_type_desc<long double>())
+    else if (cur_type_desc == type_desc_from_type<long double>())
       args_cur.next_var<long double>();
 #endif // LLVM_LIBC_PRINTF_DISABLE_FLOAT
     // pointers may be stored separately from normal values.
-    else if (cur_type_desc == get_type_desc<void *>())
+    else if (cur_type_desc == type_desc_from_type<void *>())
       args_cur.next_var<void *>();
     else
       args_cur.next_var<uint32_t>();
 
     ++args_index;
   }
 }
 
 #endif // LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
 
 } // namespace printf_core
 } // namespace __llvm_libc