clean up compoisite conversions and address comments

Author: michaelrj-google

libc/include/time.yaml

@@ -100,16 +100,6 @@ functions:
       - type: size_t
       - type: const char *__restrict
       - type: const struct tm *__restrict
-  - name: strftime_l
-    standard:
-      - stdc
-    return_type: size_t
-    arguments:
-      - type: char *__restrict
-      - type: size_t
-      - type: const char *__restrict
-      - type: const struct tm *__restrict
-      - type: locale_t
   - name: time
     standard:
       - stdc

libc/src/time/strftime.cpp

@@ -18,7 +18,7 @@ namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(size_t, strftime,
                    (char *__restrict buffer, size_t buffsz,
-                    const char *__restrict format, const struct tm *timeptr)) {
+                    const char *__restrict format, const tm *timeptr)) {
 
   printf_core::WriteBuffer wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
   printf_core::Writer writer(&wb);

libc/src/time/strftime.h

@@ -16,7 +16,7 @@
 namespace LIBC_NAMESPACE_DECL {
 
 size_t strftime(char *__restrict, size_t max, const char *__restrict format,
-                const struct tm *timeptr);
+                const tm *timeptr);
 
 } // namespace LIBC_NAMESPACE_DECL
 

libc/src/time/strftime_core/composite_converter.h

@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_COMPOSITE_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_COMPOSITE_CONVERTER_H
 
+#include "hdr/types/struct_tm.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/macros/config.h"
 #include "src/stdio/printf_core/writer.h"
@@ -21,46 +22,40 @@
 namespace LIBC_NAMESPACE_DECL {
 namespace strftime_core {
 
+LIBC_INLINE IntFormatSection
+get_specific_int_format(const tm *timeptr, const FormatSection &base_to_conv,
+                        char new_conv_name, int TRAILING_CONV_LEN = -1) {
+  // a negative padding will be treated as the default
+  const int NEW_MIN_WIDTH =
+      TRAILING_CONV_LEN > 0 ? base_to_conv.min_width - TRAILING_CONV_LEN : 0;
+  FormatSection new_conv = base_to_conv;
+  new_conv.conv_name = new_conv_name;
+  new_conv.min_width = NEW_MIN_WIDTH;
+
+  IntFormatSection result = get_int_format(new_conv, timeptr);
+
+  // If the user set the padding, but it's below the width of the trailing
+  // conversions, then there should be no padding.
+  if (base_to_conv.min_width > 0 && NEW_MIN_WIDTH < 0)
+    result.pad_to_len = 0;
+
+  return result;
+}
+
 LIBC_INLINE int convert_date_us(printf_core::Writer *writer,
                                 const FormatSection &to_conv,
                                 const tm *timeptr) {
   // format is %m/%d/%y (month/day/year)
   // we only pad the first conversion, and we assume all the other values are in
   // their valid ranges.
-  const size_t trailing_conv_len = 1 + 2 + 1 + 2; // sizeof("/01/02")
+  constexpr int TRAILING_CONV_LEN = 1 + 2 + 1 + 2; // sizeof("/01/02")
   IntFormatSection year_conv;
   IntFormatSection mon_conv;
   IntFormatSection mday_conv;
 
-  {
-    FormatSection raw_mon_conv = to_conv;
-    raw_mon_conv.conv_name = 'm';
-
-    const int requested_padding = to_conv.min_width - trailing_conv_len;
-    // a negative padding will be treated as the default
-    raw_mon_conv.min_width = requested_padding;
-
-    mon_conv = get_int_format(raw_mon_conv, timeptr);
-
-    // If the user set the padding, but it's below the width of the trailing
-    // conversions, then there should be no padding.
-    if (to_conv.min_width > 0 && requested_padding < 0)
-      mon_conv.pad_to_len = 0;
-  }
-  {
-    FormatSection raw_mday_conv = to_conv;
-    raw_mday_conv.conv_name = 'd';
-    raw_mday_conv.min_width = 0;
-
-    mday_conv = get_int_format(raw_mday_conv, timeptr);
-  }
-  {
-    FormatSection raw_year_conv = to_conv;
-    raw_year_conv.conv_name = 'y';
-    raw_year_conv.min_width = 0;
-
-    year_conv = get_int_format(raw_year_conv, timeptr);
-  }
+  mon_conv = get_specific_int_format(timeptr, to_conv, 'm', TRAILING_CONV_LEN);
+  mday_conv = get_specific_int_format(timeptr, to_conv, 'd');
+  year_conv = get_specific_int_format(timeptr, to_conv, 'y');
 
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, mon_conv));
   RET_IF_RESULT_NEGATIVE(writer->write('/'));
@@ -77,40 +72,14 @@ LIBC_INLINE int convert_date_iso(printf_core::Writer *writer,
   // format is "%Y-%m-%d" (year-month-day)
   // we only pad the first conversion, and we assume all the other values are in
   // their valid ranges.
-  const size_t trailing_conv_len = 1 + 2 + 1 + 2; // sizeof("-01-02")
+  constexpr int TRAILING_CONV_LEN = 1 + 2 + 1 + 2; // sizeof("-01-02")
   IntFormatSection year_conv;
   IntFormatSection mon_conv;
   IntFormatSection mday_conv;
 
-  {
-    FormatSection raw_year_conv = to_conv;
-    raw_year_conv.conv_name = 'Y';
-
-    const int requested_padding = to_conv.min_width - trailing_conv_len;
-    // a negative padding will be treated as the default
-    raw_year_conv.min_width = requested_padding;
-
-    year_conv = get_int_format(raw_year_conv, timeptr);
-
-    // If the user set the padding, but it's below the width of the trailing
-    // conversions, then there should be no padding.
-    if (to_conv.min_width > 0 && requested_padding < 0)
-      year_conv.pad_to_len = 0;
-  }
-  {
-    FormatSection raw_mon_conv = to_conv;
-    raw_mon_conv.conv_name = 'm';
-    raw_mon_conv.min_width = 0;
-
-    mon_conv = get_int_format(raw_mon_conv, timeptr);
-  }
-  {
-    FormatSection raw_mday_conv = to_conv;
-    raw_mday_conv.conv_name = 'd';
-    raw_mday_conv.min_width = 0;
-
-    mday_conv = get_int_format(raw_mday_conv, timeptr);
-  }
+  year_conv = get_specific_int_format(timeptr, to_conv, 'Y', TRAILING_CONV_LEN);
+  mon_conv = get_specific_int_format(timeptr, to_conv, 'm');
+  mday_conv = get_specific_int_format(timeptr, to_conv, 'd');
 
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, year_conv));
   RET_IF_RESULT_NEGATIVE(writer->write('-'));
@@ -127,42 +96,17 @@ LIBC_INLINE int convert_time_am_pm(printf_core::Writer *writer,
   // format is "%I:%M:%S %p" (hour:minute:second AM/PM)
   // we only pad the first conversion, and we assume all the other values are in
   // their valid ranges.
-  const size_t trailing_conv_len = 1 + 2 + 1 + 2 + 1 + 2; // sizeof(":01:02 AM")
+  constexpr int TRAILING_CONV_LEN =
+      1 + 2 + 1 + 2 + 1 + 2; // sizeof(":01:02 AM")
   IntFormatSection hour_conv;
   IntFormatSection min_conv;
   IntFormatSection sec_conv;
 
   const time_utils::TMReader time_reader(timeptr);
 
-  {
-    FormatSection raw_hour_conv = to_conv;
-    raw_hour_conv.conv_name = 'I';
-
-    const int requested_padding = to_conv.min_width - trailing_conv_len;
-    // a negative padding will be treated as the default
-    raw_hour_conv.min_width = requested_padding;
-
-    hour_conv = get_int_format(raw_hour_conv, timeptr);
-
-    // If the user set the padding, but it's below the width of the trailing
-    // conversions, then there should be no padding.
-    if (to_conv.min_width > 0 && requested_padding < 0)
-      hour_conv.pad_to_len = 0;
-  }
-  {
-    FormatSection raw_min_conv = to_conv;
-    raw_min_conv.conv_name = 'M';
-    raw_min_conv.min_width = 0;
-
-    min_conv = get_int_format(raw_min_conv, timeptr);
-  }
-  {
-    FormatSection raw_sec_conv = to_conv;
-    raw_sec_conv.conv_name = 'S';
-    raw_sec_conv.min_width = 0;
-
-    sec_conv = get_int_format(raw_sec_conv, timeptr);
-  }
+  hour_conv = get_specific_int_format(timeptr, to_conv, 'I', TRAILING_CONV_LEN);
+  min_conv = get_specific_int_format(timeptr, to_conv, 'M');
+  sec_conv = get_specific_int_format(timeptr, to_conv, 'S');
 
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, hour_conv));
   RET_IF_RESULT_NEGATIVE(writer->write(':'));
@@ -181,32 +125,13 @@ LIBC_INLINE int convert_time_minute(printf_core::Writer *writer,
   // format is "%H:%M" (hour:minute)
   // we only pad the first conversion, and we assume all the other values are in
   // their valid ranges.
-  const size_t trailing_conv_len = 1 + 2; // sizeof(":01")
+  constexpr int TRAILING_CONV_LEN = 1 + 2; // sizeof(":01")
   IntFormatSection hour_conv;
   IntFormatSection min_conv;
 
-  {
-    FormatSection raw_hour_conv = to_conv;
-    raw_hour_conv.conv_name = 'H';
-
-    const int requested_padding = to_conv.min_width - trailing_conv_len;
-    // a negative padding will be treated as the default
-    raw_hour_conv.min_width = requested_padding;
+  hour_conv = get_specific_int_format(timeptr, to_conv, 'H', TRAILING_CONV_LEN);
+  min_conv = get_specific_int_format(timeptr, to_conv, 'M');
 
-    hour_conv = get_int_format(raw_hour_conv, timeptr);
-
-    // If the user set the padding, but it's below the width of the trailing
-    // conversions, then there should be no padding.
-    if (to_conv.min_width > 0 && requested_padding < 0)
-      hour_conv.pad_to_len = 0;
-  }
-  {
-    FormatSection raw_min_conv = to_conv;
-    raw_min_conv.conv_name = 'M';
-    raw_min_conv.min_width = 0;
-
-    min_conv = get_int_format(raw_min_conv, timeptr);
-  }
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, hour_conv));
   RET_IF_RESULT_NEGATIVE(writer->write(':'));
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, min_conv));
@@ -220,40 +145,14 @@ LIBC_INLINE int convert_time_second(printf_core::Writer *writer,
   // format is "%H:%M:%S" (hour:minute:second)
   // we only pad the first conversion, and we assume all the other values are in
   // their valid ranges.
-  const size_t trailing_conv_len = 1 + 2 + 1 + 2; // sizeof(":01:02")
+  constexpr int TRAILING_CONV_LEN = 1 + 2 + 1 + 2; // sizeof(":01:02")
   IntFormatSection hour_conv;
   IntFormatSection min_conv;
   IntFormatSection sec_conv;
 
-  {
-    FormatSection raw_hour_conv = to_conv;
-    raw_hour_conv.conv_name = 'H';
-
-    const int requested_padding = to_conv.min_width - trailing_conv_len;
-    // a negative padding will be treated as the default
-    raw_hour_conv.min_width = requested_padding;
-
-    hour_conv = get_int_format(raw_hour_conv, timeptr);
-
-    // If the user set the padding, but it's below the width of the trailing
-    // conversions, then there should be no padding.
-    if (to_conv.min_width > 0 && requested_padding < 0)
-      hour_conv.pad_to_len = 0;
-  }
-  {
-    FormatSection raw_min_conv = to_conv;
-    raw_min_conv.conv_name = 'M';
-    raw_min_conv.min_width = 0;
-
-    min_conv = get_int_format(raw_min_conv, timeptr);
-  }
-  {
-    FormatSection raw_sec_conv = to_conv;
-    raw_sec_conv.conv_name = 'S';
-    raw_sec_conv.min_width = 0;
-
-    sec_conv = get_int_format(raw_sec_conv, timeptr);
-  }
+  hour_conv = get_specific_int_format(timeptr, to_conv, 'H', TRAILING_CONV_LEN);
+  min_conv = get_specific_int_format(timeptr, to_conv, 'M');
+  sec_conv = get_specific_int_format(timeptr, to_conv, 'S');
 
   RET_IF_RESULT_NEGATIVE(write_padded_int(writer, hour_conv));
   RET_IF_RESULT_NEGATIVE(writer->write(':'));
@@ -273,33 +172,22 @@ LIBC_INLINE int convert_full_date_time(printf_core::Writer *writer,
   // their valid ranges.
   // sizeof("Sun Jan 12 03:45:06 2025")
   const size_t full_conv_len = 3 + 1 + 3 + 1 + 2 + 1 + 8 + 1 + 4;
-  // use the full conv lent because
+  // use the full conv len because this isn't being passed to a proper converter
+  // that will handle the width of the leading conversion. Instead it has to be
+  // handled below.
   const int requested_padding = to_conv.min_width - full_conv_len;
 
   cpp::string_view wday_str = unwrap_opt(time_reader.get_weekday_short_name());
   cpp::string_view month_str = unwrap_opt(time_reader.get_month_short_name());
   IntFormatSection mday_conv;
-  FormatSection raw_time_conv = to_conv;
   IntFormatSection year_conv;
 
-  {
-    FormatSection raw_mday_conv = to_conv;
-    raw_mday_conv.conv_name = 'e';
-    raw_mday_conv.min_width = 0;
-
-    mday_conv = get_int_format(raw_mday_conv, timeptr);
-  }
-  {
-    FormatSection raw_year_conv = to_conv;
-    raw_year_conv.conv_name = 'Y';
-    raw_year_conv.min_width = 0;
+  mday_conv = get_specific_int_format(timeptr, to_conv, 'e');
+  year_conv = get_specific_int_format(timeptr, to_conv, 'Y');
 
-    year_conv = get_int_format(raw_year_conv, timeptr);
-  }
-  {
-    raw_time_conv.conv_name = 'T';
-    raw_time_conv.min_width = 0;
-  }
+  FormatSection raw_time_conv = to_conv;
+  raw_time_conv.conv_name = 'T';
+  raw_time_conv.min_width = 0;
 
   if (requested_padding > 0)
     RET_IF_RESULT_NEGATIVE(writer->write(' ', requested_padding));

libc/src/time/strftime_core/converter.cpp

@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "hdr/types/struct_tm.h"
 #include "src/__support/macros/config.h"
 #include "src/stdio/printf_core/writer.h"
 #include "src/time/strftime_core/core_structs.h"

libc/src/time/strftime_core/converter.h

@@ -9,11 +9,10 @@
 #ifndef LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_CONVERTER_H
 
+#include "hdr/types/struct_tm.h"
 #include "src/stdio/printf_core/writer.h"
 #include "src/time/strftime_core/core_structs.h"
 
-#include <stddef.h>
-
 namespace LIBC_NAMESPACE_DECL {
 namespace strftime_core {
 

libc/src/time/strftime_core/core_structs.h

@@ -12,7 +12,7 @@
 #include "hdr/types/struct_tm.h"
 #include "src/__support/CPP/string_view.h"
 
-#include <inttypes.h>
+#include <stdint.h>
 
 namespace LIBC_NAMESPACE_DECL {
 namespace strftime_core {
@@ -28,12 +28,12 @@ enum FormatFlags : uint8_t {
 };
 
 struct FormatSection {
-  bool has_conv;
-  cpp::string_view raw_string;
+  bool has_conv = false;
+  cpp::string_view raw_string = {};
 
   FormatFlags flags = FormatFlags(0);
-  ConvModifier modifier;
-  char conv_name;
+  ConvModifier modifier = ConvModifier::none;
+  char conv_name = '\0';
   int min_width = 0;
 };
 

libc/src/time/strftime_core/num_converter.h

@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_NUM_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_NUM_CONVERTER_H
 
+#include "hdr/types/struct_tm.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/integer_to_string.h"
 #include "src/__support/macros/config.h"
@@ -23,9 +24,9 @@ namespace strftime_core {
 using DecFmt = IntegerToString<uintmax_t>;
 
 struct IntFormatSection {
-  uintmax_t num;
-  char sign_char;
-  size_t pad_to_len;
+  uintmax_t num = 0;
+  char sign_char = '\0';
+  size_t pad_to_len = 0;
   char padding_char = '0';
 };