[libcxx-commits] [PATCH] D128577: [libc++][chrono] Implements formatter day.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added reviewers: ldionne, vitaut.
Mordante added subscribers: vitaut, ldionne.
Mordante added inline comments.


================
Comment at: libcxx/docs/Status/Cxx20Issues.csv:170
 "`3230 <https://wg21.link/LWG3230>`__","Format specifier ``%y/%Y``\  is missing locale alternative versions","Belfast","","","|chrono| |format|"
 "`3232 <https://wg21.link/LWG3232>`__","Inconsistency in ``zoned_time``\  deduction guides","Belfast","","","|chrono|"
 "`3222 <https://wg21.link/LWG3222>`__","P0574R1 introduced preconditions on non-existent parameters","Belfast","",""
----------------
@ldionne, @vitaut this is the first chrono formatter. A rough draft with more formatters is available in D126592.

This patch adds the basic framework and one formatter specialization to keep the review manageable. I will chop the large patch in smaller reviewable patches.

Note there's an issue with Windows somehow the values for invalid days are odd and the CI doesn't provide good feedback. I'll test to see whether I can run the Windows test locally otherwise I'll ask assistance of people with access to Windows.


================
Comment at: libcxx/include/__chrono/formatter.h:104
+        break;
+
+      case _CharT('O'):
----------------
Note the number of "special cases" will grow in future patches. For example '%S' needs to show subsecond resolution. '%Y' requires 4 digits, which C doesn't do.


================
Comment at: libcxx/include/__chrono/formatter.h:140
+
+  // TODO FMT Use the stringstream's view after P0408R7 has been implemented.
+  basic_string<_CharT> __str = __sstr.str();
----------------
Basically the mentioned paper gives direct access to the buffer's data with a view. That avoids the need for the temporary string to create a view.


================
Comment at: libcxx/include/__chrono/parser_std_format_spec.h:50
+/// \ref format_error is thrown.
+enum class __flags {
+  __second = 0x1,
----------------
Note most of these `__flags` aren't used and the functions below also not yet, except for the unit tests. There have been used in the large patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128577/new/

https://reviews.llvm.org/D128577