[libc-commits] [libc] c25af9b - [libc][NFC] Use a function instead of templated static member functions for TYPE_DESC

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Thu Jan 5 02:03:15 PST 2023


Author: Guillaume Chatelet
Date: 2023-01-05T10:02:57Z
New Revision: c25af9bb1ea50353e6387d59a13e3c81fb116a74

URL: https://github.com/llvm/llvm-project/commit/c25af9bb1ea50353e6387d59a13e3c81fb116a74
DIFF: https://github.com/llvm/llvm-project/commit/c25af9bb1ea50353e6387d59a13e3c81fb116a74.diff

LOG: [libc][NFC] Use a function instead of templated static member functions for TYPE_DESC

I'm surprised that clang accepts the current code.
It seems odd to me to specialize templated static member variables.

GCC rejects them: https://godbolt.org/z/3ecE9Ps7T

This patch is in the context of https://github.com/llvm/llvm-project/issues/59368

Differential Revision: https://reviews.llvm.org/D140981

Added: 
    

Modified: 
    libc/src/stdio/printf_core/CMakeLists.txt
    libc/src/stdio/printf_core/parser.cpp
    libc/src/stdio/printf_core/parser.h

Removed: 
    


################################################################################
diff  --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 8cd6743a029ca..6d7a3be52bf28 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -21,6 +21,7 @@ add_object_library(
     libc.src.__support.str_to_integer
     libc.src.__support.CPP.bit
     libc.src.__support.CPP.string_view
+    libc.src.__support.CPP.type_traits
 )
 
 add_object_library(

diff  --git a/libc/src/stdio/printf_core/parser.cpp b/libc/src/stdio/printf_core/parser.cpp
index 6a2db74ecb6ed..0637eda28d31c 100644
--- a/libc/src/stdio/printf_core/parser.cpp
+++ b/libc/src/stdio/printf_core/parser.cpp
@@ -266,9 +266,9 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
         ++local_pos;
 
         size_t width_index = parse_index(&local_pos);
-        set_type_desc(width_index, TYPE_DESC<int>);
+        set_type_desc(width_index, get_type_desc<int>());
         if (width_index == index)
-          return TYPE_DESC<int>;
+          return get_type_desc<int>();
 
       } else if (internal::isdigit(str[local_pos])) {
         while (internal::isdigit(str[local_pos]))
@@ -282,9 +282,9 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
           ++local_pos;
 
           size_t precision_index = parse_index(&local_pos);
-          set_type_desc(precision_index, TYPE_DESC<int>);
+          set_type_desc(precision_index, get_type_desc<int>());
           if (precision_index == index)
-            return TYPE_DESC<int>;
+            return get_type_desc<int>();
 
         } else if (internal::isdigit(str[local_pos])) {
           while (internal::isdigit(str[local_pos]))
@@ -303,13 +303,13 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
         continue;
       }
 
-      TypeDesc conv_size = TYPE_DESC<void>;
+      TypeDesc conv_size = get_type_desc<void>();
       switch (str[local_pos]) {
       case ('%'):
-        conv_size = TYPE_DESC<void>;
+        conv_size = get_type_desc<void>();
         break;
       case ('c'):
-        conv_size = TYPE_DESC<int>;
+        conv_size = get_type_desc<int>();
         break;
       case ('d'):
       case ('i'):
@@ -321,24 +321,24 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
         case (LengthModifier::hh):
         case (LengthModifier::h):
         case (LengthModifier::none):
-          conv_size = TYPE_DESC<int>;
+          conv_size = get_type_desc<int>();
           break;
         case (LengthModifier::l):
-          conv_size = TYPE_DESC<long>;
+          conv_size = get_type_desc<long>();
           break;
         case (LengthModifier::ll):
         case (LengthModifier::L): // This isn't in the standard, but is in other
                                   // libc implementations.
-          conv_size = TYPE_DESC<long long>;
+          conv_size = get_type_desc<long long>();
           break;
         case (LengthModifier::j):
-          conv_size = TYPE_DESC<intmax_t>;
+          conv_size = get_type_desc<intmax_t>();
           break;
         case (LengthModifier::z):
-          conv_size = TYPE_DESC<size_t>;
+          conv_size = get_type_desc<size_t>();
           break;
         case (LengthModifier::t):
-          conv_size = TYPE_DESC<ptr
diff _t>;
+          conv_size = get_type_desc<ptr
diff _t>();
           break;
         }
         break;
@@ -352,9 +352,9 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
       case ('g'):
       case ('G'):
         if (lm != LengthModifier::L)
-          conv_size = TYPE_DESC<double>;
+          conv_size = get_type_desc<double>();
         else
-          conv_size = TYPE_DESC<long double>;
+          conv_size = get_type_desc<long double>();
         break;
 #endif // LLVM_LIBC_PRINTF_DISABLE_FLOAT
 #ifndef LLVM_LIBC_PRINTF_DISABLE_WRITE_INT
@@ -362,10 +362,10 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
 #endif // LLVM_LIBC_PRINTF_DISABLE_WRITE_INT
       case ('p'):
       case ('s'):
-        conv_size = TYPE_DESC<void *>;
+        conv_size = get_type_desc<void *>();
         break;
       default:
-        conv_size = TYPE_DESC<int>;
+        conv_size = get_type_desc<int>();
         break;
       }
 
@@ -381,7 +381,7 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
 
   // If there is no size for the requested index, then just guess that it's an
   // int.
-  return TYPE_DESC<int>;
+  return get_type_desc<int>();
 }
 
 void Parser::args_to_index(size_t index) {
@@ -391,26 +391,26 @@ void Parser::args_to_index(size_t index) {
   }
 
   while (args_index < index) {
-    Parser::TypeDesc cur_type_desc = TYPE_DESC<void>;
+    Parser::TypeDesc cur_type_desc = get_type_desc<void>();
     if (args_index <= DESC_ARR_LEN)
       cur_type_desc = desc_arr[args_index - 1];
 
-    if (cur_type_desc == TYPE_DESC<void>)
+    if (cur_type_desc == get_type_desc<void>())
       cur_type_desc = get_type_desc(args_index);
 
-    if (cur_type_desc == TYPE_DESC<uint32_t>)
+    if (cur_type_desc == get_type_desc<uint32_t>())
       args_cur.next_var<uint32_t>();
-    else if (cur_type_desc == TYPE_DESC<uint64_t>)
+    else if (cur_type_desc == get_type_desc<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 == TYPE_DESC<double>)
+    else if (cur_type_desc == get_type_desc<double>())
       args_cur.next_var<double>();
-    else if (cur_type_desc == TYPE_DESC<long double>)
+    else if (cur_type_desc == get_type_desc<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 == TYPE_DESC<void *>)
+    else if (cur_type_desc == get_type_desc<void *>())
       args_cur.next_var<void *>();
     else
       args_cur.next_var<uint32_t>();

diff  --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index 16cfd9ec3d45a..7829914a78aeb 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PARSER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PARSER_H
 
+#include "src/__support/CPP/type_traits.h"
 #include "src/__support/arg_list.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/printf_config.h"
@@ -103,19 +104,18 @@ class Parser {
   // local_pos.
   size_t parse_index(size_t *local_pos);
 
-  template <typename T>
-  static constexpr TypeDesc TYPE_DESC{sizeof(T), PrimaryType::Integer};
-  template <>
-  static constexpr TypeDesc TYPE_DESC<double>{sizeof(double),
-                                              PrimaryType::Float};
-  template <>
-  static constexpr TypeDesc TYPE_DESC<long double>{sizeof(long double),
-                                                   PrimaryType::Float};
-  template <>
-  static constexpr TypeDesc TYPE_DESC<void *>{sizeof(void *),
-                                              PrimaryType::Pointer};
-  template <>
-  static constexpr TypeDesc TYPE_DESC<void>{0, PrimaryType::Integer};
+  template <typename T> 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};
+    }
+  }
 
   void inline set_type_desc(size_t index, TypeDesc value) {
     if (index != 0 && index <= DESC_ARR_LEN)
@@ -129,7 +129,7 @@ class Parser {
     if (!(index == 0 || index == args_index))
       args_to_index(index);
 
-    set_type_desc(index, TYPE_DESC<T>);
+    set_type_desc(index, get_type_desc<T>());
 
     ++args_index;
     return get_next_arg_value<T>();


        


More information about the libc-commits mailing list