[libcxx-commits] [libcxx] Formatting Ranges: Range "m" specifier do not pass through to underlying element (PR #70616)

[Yihe Li via libcxx-commits]

https://github.com/Mick235711 updated https://github.com/llvm/llvm-project/pull/70616

>From d590a6533652e50669accb2260b70ab25788270b Mon Sep 17 00:00:00 2001
From: Yihe Li <winmikedows at hotmail.com>
Date: Mon, 30 Oct 2023 12:19:51 +0800
Subject: [PATCH 1/2] Fix m specifier for range formatting: call
 set_bracket/separator on underlying too

---
 libcxx/include/__format/range_formatter.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libcxx/include/__format/range_formatter.h b/libcxx/include/__format/range_formatter.h
index d13278009fcf897..25d0f5697c49c7f 100644
--- a/libcxx/include/__format/range_formatter.h
+++ b/libcxx/include/__format/range_formatter.h
@@ -216,6 +216,8 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter {
       if constexpr (__fmt_pair_like<_Tp>) {
         set_brackets(_LIBCPP_STATICALLY_WIDEN(_CharT, "{"), _LIBCPP_STATICALLY_WIDEN(_CharT, "}"));
         set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ", "));
+        __underlying_.set_brackets({}, {});
+        __underlying_.set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": "));
         ++__begin;
       } else
         std::__throw_format_error("Type m requires a pair or a tuple with two elements");

>From 5bdf2c2c7961881ece04a1b943d3331c566f2ec4 Mon Sep 17 00:00:00 2001
From: Yihe Li <winmikedows at hotmail.com>
Date: Mon, 30 Oct 2023 13:49:03 +0800
Subject: [PATCH 2/2] Additional bugfix: only add m specifier when there is no
 underlying spec & change test suite

---
 libcxx/include/__format/range_formatter.h        | 16 ++++++++++++----
 .../format.functions.tests.h                     |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/libcxx/include/__format/range_formatter.h b/libcxx/include/__format/range_formatter.h
index 25d0f5697c49c7f..d58742332d09074 100644
--- a/libcxx/include/__format/range_formatter.h
+++ b/libcxx/include/__format/range_formatter.h
@@ -65,7 +65,7 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter {
 
     // The n field overrides a possible m type, therefore delay applying the
     // effect of n until the type has been procesed.
-    __parse_type(__begin, __end);
+    bool __has_m_specifier = __parse_type(__begin, __end);
     if (__parser_.__clear_brackets_)
       set_brackets({}, {});
     if (__begin == __end) [[unlikely]]
@@ -87,6 +87,14 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter {
     __ctx.advance_to(__begin);
     __begin = __underlying_.parse(__ctx);
 
+    // Add "m" specifier semantics if there are no underlying specifier
+    if (!__has_range_underlying_spec && __has_m_specifier) {
+        if constexpr (__fmt_pair_like<_Tp>) { // should always be true
+            __underlying_.set_brackets({}, {});
+            __underlying_.set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": "));
+        }
+    }
+
     // This test should not be required if __has_range_underlying_spec is false.
     // However this test makes sure the underlying formatter left the parser in
     // a valid state. (Note this is not a full protection against evil parsers.
@@ -210,15 +218,14 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter {
 
 private:
   template <contiguous_iterator _Iterator>
-  _LIBCPP_HIDE_FROM_ABI constexpr void __parse_type(_Iterator& __begin, _Iterator __end) {
+  _LIBCPP_HIDE_FROM_ABI constexpr bool __parse_type(_Iterator& __begin, _Iterator __end) {
     switch (*__begin) {
     case _CharT('m'):
       if constexpr (__fmt_pair_like<_Tp>) {
         set_brackets(_LIBCPP_STATICALLY_WIDEN(_CharT, "{"), _LIBCPP_STATICALLY_WIDEN(_CharT, "}"));
         set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ", "));
-        __underlying_.set_brackets({}, {});
-        __underlying_.set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": "));
         ++__begin;
+        return true;
       } else
         std::__throw_format_error("Type m requires a pair or a tuple with two elements");
       break;
@@ -241,6 +248,7 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter {
       } else
         std::__throw_format_error("Type ?s requires character type as formatting argument");
     }
+    return false;
   }
 
   template <class _ParseContext>
diff --git a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
index 1e6c75cc69fe449..2417efe4afe3d76 100644
--- a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
+++ b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
@@ -983,10 +983,10 @@ void test_pair_tuple(TestFunction check, ExceptionTest check_exception, auto&& i
 
   // *** n
   check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24n}"), input);
-  check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24nm}"), input); // m should have no effect
+  check(SV("____1: 'a', 42: '*'_____"), SV("{:_^24nm}"), input);
 
   // *** type ***
-  check(SV("__{(1, 'a'), (42, '*')}___"), SV("{:_^26m}"), input);
+  check(SV("____{1: 'a', 42: '*'}_____"), SV("{:_^26m}"), input);
   check_exception("Type s requires character type as formatting argument", SV("{:s}"), input);
   check_exception("Type ?s requires character type as formatting argument", SV("{:?s}"), input);
   for (std::basic_string_view<CharT> fmt : fmt_invalid_types<CharT>("s"))