diff mbox series

[committed] libstdc++: Fix P2510R3 "Formatting pointers" [PR110149]

Message ID 20230609121025.294493-1-jwakely@redhat.com
State New
Headers show
Series [committed] libstdc++: Fix P2510R3 "Formatting pointers" [PR110149] | expand

Commit Message

Jonathan Wakely June 9, 2023, 12:10 p.m. UTC 
Tested powerpc64le-linux. Pushed to trunk.

I'll backport it to gcc-13 later.

-- >8 --

I had intended to support the P2510R3 proposal unconditionally in C++20
mode, but I left it half implemented. The parse function supported the
new extensions, but the format function didn't.

This adds the missing pieces, and makes it only enabled for C++26 and
non-strict modes.

libstdc++-v3/ChangeLog:

	PR libstdc++/110149
	* include/std/format (formatter<const void*, charT>::parse):
	Only alow 0 and P for C++26 and non-strict modes.
	(formatter<const void*, charT>::format): Use toupper for P
	type, and insert zero-fill characters for 0 option.
	* testsuite/std/format/functions/format.cc: Check pointer
	formatting. Only check P2510R3 extensions conditionally.
	* testsuite/std/format/parse_ctx.cc: Only check P2510R3
	extensions conditionally.
---
 libstdc++-v3/include/std/format               | 56 ++++++++++++++++---
 .../testsuite/std/format/functions/format.cc  | 42 ++++++++++++++
 .../testsuite/std/format/parse_ctx.cc         | 15 +++--
 3 files changed, 101 insertions(+), 12 deletions(-)

Comments

Prathamesh Kulkarni June 12, 2023, 11:07 a.m. UTC | #1 
On Fri, 9 Jun 2023 at 17:41, Jonathan Wakely via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Tested powerpc64le-linux. Pushed to trunk.
Hi Jonathan,
This patch causes following regression on armv8l-unknown-linux-gnueabihf:
FAIL: std/format/functions/format.cc execution test
/home/tcwg-buildslave/workspace/tcwg_gnu_3/abe/snapshots/gcc.git~master/libstdc++-v3/testsuite/std/format/functions/format.cc:368:
void test_pointer(): Assertion 's == (str_int + ' ' + str_int + "
0x0")' failed.
timeout: the monitored command dumped core

Full libstdc++.log:
https://people.linaro.org/~prathamesh.kulkarni/libstdc++.log.0.xz
Could you please check ?

Thanks,
Prathamesh



>
> I'll backport it to gcc-13 later.
>
> -- >8 --
>
> I had intended to support the P2510R3 proposal unconditionally in C++20
> mode, but I left it half implemented. The parse function supported the
> new extensions, but the format function didn't.
>
> This adds the missing pieces, and makes it only enabled for C++26 and
> non-strict modes.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/110149
>         * include/std/format (formatter<const void*, charT>::parse):
>         Only alow 0 and P for C++26 and non-strict modes.
>         (formatter<const void*, charT>::format): Use toupper for P
>         type, and insert zero-fill characters for 0 option.
>         * testsuite/std/format/functions/format.cc: Check pointer
>         formatting. Only check P2510R3 extensions conditionally.
>         * testsuite/std/format/parse_ctx.cc: Only check P2510R3
>         extensions conditionally.
> ---
>  libstdc++-v3/include/std/format               | 56 ++++++++++++++++---
>  .../testsuite/std/format/functions/format.cc  | 42 ++++++++++++++
>  .../testsuite/std/format/parse_ctx.cc         | 15 +++--
>  3 files changed, 101 insertions(+), 12 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
> index 6edc3208afa..96a1e62ccc8 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -830,7 +830,7 @@ namespace __format
>         {
>           if (_M_spec._M_type == _Pres_esc)
>             {
> -             // TODO: C++20 escaped string presentation
> +             // TODO: C++23 escaped string presentation
>             }
>
>           if (_M_spec._M_width_kind == _WP_none
> @@ -2081,19 +2081,31 @@ namespace __format
>         if (__finished())
>           return __first;
>
> -       // _GLIBCXX_RESOLVE_LIB_DEFECTS
> -       // P2519R3 Formatting pointers
> +// _GLIBCXX_RESOLVE_LIB_DEFECTS
> +// P2510R3 Formatting pointers
> +#define _GLIBCXX_P2518R3 (__cplusplus > 202302L || ! defined __STRICT_ANSI__)
> +
> +#if _GLIBCXX_P2518R3
>         __first = __spec._M_parse_zero_fill(__first, __last);
>         if (__finished())
>           return __first;
> +#endif
>
>         __first = __spec._M_parse_width(__first, __last, __pc);
>
> -       if (__first != __last && (*__first == 'p' || *__first == 'P'))
> +       if (__first != __last)
>           {
> -           if (*__first == 'P')
> +           if (*__first == 'p')
> +             ++__first;
> +#if _GLIBCXX_P2518R3
> +           else if (*__first == 'P')
> +           {
> +             // _GLIBCXX_RESOLVE_LIB_DEFECTS
> +             // P2510R3 Formatting pointers
>               __spec._M_type = __format::_Pres_P;
> -           ++__first;
> +             ++__first;
> +           }
> +#endif
>           }
>
>         if (__finished())
> @@ -2110,9 +2122,21 @@ namespace __format
>           char __buf[2 + sizeof(__v) * 2];
>           auto [__ptr, __ec] = std::to_chars(__buf + 2, std::end(__buf),
>                                              __u, 16);
> -         const int __n = __ptr - __buf;
> +         int __n = __ptr - __buf;
>           __buf[0] = '0';
>           __buf[1] = 'x';
> +#if _GLIBCXX_P2518R3
> +         if (_M_spec._M_type == __format::_Pres_P)
> +           {
> +             __buf[1] = 'X';
> +             for (auto __p = __buf + 2; __p != __ptr; ++__p)
> +#if __has_builtin(__builtin_toupper)
> +               *__p = __builtin_toupper(*__p);
> +#else
> +               *__p = std::toupper(*__p);
> +#endif
> +           }
> +#endif
>
>           basic_string_view<_CharT> __str;
>           if constexpr (is_same_v<_CharT, char>)
> @@ -2126,6 +2150,24 @@ namespace __format
>               __str = wstring_view(__p, __n);
>             }
>
> +#if _GLIBCXX_P2518R3
> +         if (_M_spec._M_zero_fill)
> +           {
> +             size_t __width = _M_spec._M_get_width(__fc);
> +             if (__width <= __str.size())
> +               return __format::__write(__fc.out(), __str);
> +
> +             auto __out = __fc.out();
> +             // Write "0x" or "0X" prefix before zero-filling.
> +             __out = __format::__write(std::move(__out), __str.substr(0, 2));
> +             __str.remove_prefix(2);
> +             size_t __nfill = __width - __n;
> +             return __format::__write_padded(std::move(__out), __str,
> +                                             __format::_Align_right,
> +                                             __nfill, _CharT('0'));
> +           }
> +#endif
> +
>           return __format::__write_padded_as_spec(__str, __n, __fc, _M_spec,
>                                                   __format::_Align_right);
>         }
> diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc
> index 2a1b1560394..3485535e3cb 100644
> --- a/libstdc++-v3/testsuite/std/format/functions/format.cc
> +++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
> @@ -206,6 +206,8 @@ test_width()
>    VERIFY( s == "    " );
>    s = std::format("{:{}}", "", 3);
>    VERIFY( s == "   " );
> +  s = std::format("{:{}}|{:{}}", 1, 2, 3, 4);
> +  VERIFY( s == " 1|   3" );
>    s = std::format("{1:{0}}", 2, "");
>    VERIFY( s == "  " );
>    s = std::format("{:03}", 9);
> @@ -342,6 +344,45 @@ test_float128()
>  #endif
>  }
>
> +void
> +test_pointer()
> +{
> +  void* p = nullptr;
> +  const void* pc = p;
> +  std::string s, str_int;
> +
> +  s = std::format("{} {} {}", p, pc, nullptr);
> +  VERIFY( s == "0x0 0x0 0x0" );
> +  s = std::format("{:p} {:p} {:p}", p, pc, nullptr);
> +  VERIFY( s == "0x0 0x0 0x0" );
> +  s = std::format("{:4},{:5},{:6}", p, pc, nullptr); // width
> +  VERIFY( s == " 0x0,  0x0,   0x0" );
> +  s = std::format("{:<4},{:>5},{:^7}", p, pc, nullptr); // align+width
> +  VERIFY( s == "0x0 ,  0x0,  0x0  " );
> +  s = std::format("{:o<4},{:o>5},{:o^7}", p, pc, nullptr); // fill+align+width
> +  VERIFY( s == "0x0o,oo0x0,oo0x0oo" );
> +
> +  pc = p = &s;
> +  str_int = std::format("{:#x}", reinterpret_cast<std::uintptr_t>(p));
> +  s = std::format("{} {} {}", p, pc, nullptr);
> +  VERIFY( s == (str_int + ' ' + str_int + " 0x0") );
> +  str_int = std::format("{:#20x}", reinterpret_cast<std::uintptr_t>(p));
> +  s = std::format("{:20} {:20p}", p, pc);
> +  VERIFY( s == (str_int + ' ' + str_int) );
> +
> +#if __cplusplus > 202302L || ! defined __STRICT_ANSI__
> +  // P2510R3 Formatting pointers
> +  s = std::format("{:06} {:07P} {:08p}", (void*)0, (const void*)0, nullptr);
> +  VERIFY( s == "0x0000 0X00000 0x000000" );
> +  str_int = std::format("{:#016x}", reinterpret_cast<std::uintptr_t>(p));
> +  s = std::format("{:016} {:016}", p, pc);
> +  VERIFY( s == (str_int + ' ' + str_int) );
> +  str_int = std::format("{:#016X}", reinterpret_cast<std::uintptr_t>(p));
> +  s = std::format("{:016P} {:016P}", p, pc);
> +  VERIFY( s == (str_int + ' ' + str_int) );
> +#endif
> +}
> +
>  int main()
>  {
>    test_no_args();
> @@ -354,4 +395,5 @@ int main()
>    test_minmax();
>    test_p1652r1();
>    test_float128();
> +  test_pointer();
>  }
> diff --git a/libstdc++-v3/testsuite/std/format/parse_ctx.cc b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
> index 069dfceced5..260caf123d0 100644
> --- a/libstdc++-v3/testsuite/std/format/parse_ctx.cc
> +++ b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
> @@ -244,10 +244,6 @@ test_pointer()
>    VERIFY( ! is_std_format_spec_for<void*>("-") );
>    VERIFY( ! is_std_format_spec_for<void*>(" ") );
>    VERIFY( ! is_std_format_spec_for<void*>("#") );
> -  VERIFY( is_std_format_spec_for<void*>("0p") ); // P2510
> -  VERIFY( is_std_format_spec_for<void*>("0") );
> -  VERIFY( ! is_std_format_spec_for<void*>("00p") );
> -  VERIFY( is_std_format_spec_for<void*>("01p") );
>    VERIFY( is_std_format_spec_for<void*>("1") );
>    VERIFY( ! is_std_format_spec_for<void*>("-1") );
>    VERIFY( ! is_std_format_spec_for<void*>("-1p") );
> @@ -263,7 +259,6 @@ test_pointer()
>    VERIFY( ! is_std_format_spec_for<void*>("s") );
>    VERIFY( ! is_std_format_spec_for<void*>("?") );
>    VERIFY( is_std_format_spec_for<void*>("p") );
> -  VERIFY( is_std_format_spec_for<void*>("P") );
>    VERIFY( ! is_std_format_spec_for<void*>("a") );
>    VERIFY( ! is_std_format_spec_for<void*>("A") );
>    VERIFY( ! is_std_format_spec_for<void*>("f") );
> @@ -271,6 +266,16 @@ test_pointer()
>    VERIFY( ! is_std_format_spec_for<void*>("g") );
>    VERIFY( ! is_std_format_spec_for<void*>("G") );
>    VERIFY( ! is_std_format_spec_for<void*>("+p") );
> +
> +#if __cplusplus > 202302L || ! defined __STRICT_ANSI__
> +  // As an extension, we support P2510R3 Formatting pointers
> +  VERIFY( is_std_format_spec_for<void*>("P") );
> +  VERIFY( is_std_format_spec_for<void*>("0p") );
> +  VERIFY( is_std_format_spec_for<void*>("0P") );
> +  VERIFY( is_std_format_spec_for<void*>("0") );
> +  VERIFY( is_std_format_spec_for<void*>("01p") );
> +  VERIFY( ! is_std_format_spec_for<void*>("00p") );
> +#endif
>  }
>
>  void
> --
> 2.40.1
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 6edc3208afa..96a1e62ccc8 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -830,7 +830,7 @@  namespace __format
 	{
 	  if (_M_spec._M_type == _Pres_esc)
 	    {
-	      // TODO: C++20 escaped string presentation
+	      // TODO: C++23 escaped string presentation
 	    }
 
 	  if (_M_spec._M_width_kind == _WP_none
@@ -2081,19 +2081,31 @@  namespace __format
 	if (__finished())
 	  return __first;
 
-	// _GLIBCXX_RESOLVE_LIB_DEFECTS
-	// P2519R3 Formatting pointers
+// _GLIBCXX_RESOLVE_LIB_DEFECTS
+// P2510R3 Formatting pointers
+#define _GLIBCXX_P2518R3 (__cplusplus > 202302L || ! defined __STRICT_ANSI__)
+
+#if _GLIBCXX_P2518R3
 	__first = __spec._M_parse_zero_fill(__first, __last);
 	if (__finished())
 	  return __first;
+#endif
 
 	__first = __spec._M_parse_width(__first, __last, __pc);
 
-	if (__first != __last && (*__first == 'p' || *__first == 'P'))
+	if (__first != __last)
 	  {
-	    if (*__first == 'P')
+	    if (*__first == 'p')
+	      ++__first;
+#if _GLIBCXX_P2518R3
+	    else if (*__first == 'P')
+	    {
+	      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+	      // P2510R3 Formatting pointers
 	      __spec._M_type = __format::_Pres_P;
-	    ++__first;
+	      ++__first;
+	    }
+#endif
 	  }
 
 	if (__finished())
@@ -2110,9 +2122,21 @@  namespace __format
 	  char __buf[2 + sizeof(__v) * 2];
 	  auto [__ptr, __ec] = std::to_chars(__buf + 2, std::end(__buf),
 					     __u, 16);
-	  const int __n = __ptr - __buf;
+	  int __n = __ptr - __buf;
 	  __buf[0] = '0';
 	  __buf[1] = 'x';
+#if _GLIBCXX_P2518R3
+	  if (_M_spec._M_type == __format::_Pres_P)
+	    {
+	      __buf[1] = 'X';
+	      for (auto __p = __buf + 2; __p != __ptr; ++__p)
+#if __has_builtin(__builtin_toupper)
+		*__p = __builtin_toupper(*__p);
+#else
+		*__p = std::toupper(*__p);
+#endif
+	    }
+#endif
 
 	  basic_string_view<_CharT> __str;
 	  if constexpr (is_same_v<_CharT, char>)
@@ -2126,6 +2150,24 @@  namespace __format
 	      __str = wstring_view(__p, __n);
 	    }
 
+#if _GLIBCXX_P2518R3
+	  if (_M_spec._M_zero_fill)
+	    {
+	      size_t __width = _M_spec._M_get_width(__fc);
+	      if (__width <= __str.size())
+		return __format::__write(__fc.out(), __str);
+
+	      auto __out = __fc.out();
+	      // Write "0x" or "0X" prefix before zero-filling.
+	      __out = __format::__write(std::move(__out), __str.substr(0, 2));
+	      __str.remove_prefix(2);
+	      size_t __nfill = __width - __n;
+	      return __format::__write_padded(std::move(__out), __str,
+					      __format::_Align_right,
+					      __nfill, _CharT('0'));
+	    }
+#endif
+
 	  return __format::__write_padded_as_spec(__str, __n, __fc, _M_spec,
 						  __format::_Align_right);
 	}
diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc
index 2a1b1560394..3485535e3cb 100644
--- a/libstdc++-v3/testsuite/std/format/functions/format.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
@@ -206,6 +206,8 @@  test_width()
   VERIFY( s == "    " );
   s = std::format("{:{}}", "", 3);
   VERIFY( s == "   " );
+  s = std::format("{:{}}|{:{}}", 1, 2, 3, 4);
+  VERIFY( s == " 1|   3" );
   s = std::format("{1:{0}}", 2, "");
   VERIFY( s == "  " );
   s = std::format("{:03}", 9);
@@ -342,6 +344,45 @@  test_float128()
 #endif
 }
 
+void
+test_pointer()
+{
+  void* p = nullptr;
+  const void* pc = p;
+  std::string s, str_int;
+
+  s = std::format("{} {} {}", p, pc, nullptr);
+  VERIFY( s == "0x0 0x0 0x0" );
+  s = std::format("{:p} {:p} {:p}", p, pc, nullptr);
+  VERIFY( s == "0x0 0x0 0x0" );
+  s = std::format("{:4},{:5},{:6}", p, pc, nullptr); // width
+  VERIFY( s == " 0x0,  0x0,   0x0" );
+  s = std::format("{:<4},{:>5},{:^7}", p, pc, nullptr); // align+width
+  VERIFY( s == "0x0 ,  0x0,  0x0  " );
+  s = std::format("{:o<4},{:o>5},{:o^7}", p, pc, nullptr); // fill+align+width
+  VERIFY( s == "0x0o,oo0x0,oo0x0oo" );
+
+  pc = p = &s;
+  str_int = std::format("{:#x}", reinterpret_cast<std::uintptr_t>(p));
+  s = std::format("{} {} {}", p, pc, nullptr);
+  VERIFY( s == (str_int + ' ' + str_int + " 0x0") );
+  str_int = std::format("{:#20x}", reinterpret_cast<std::uintptr_t>(p));
+  s = std::format("{:20} {:20p}", p, pc);
+  VERIFY( s == (str_int + ' ' + str_int) );
+
+#if __cplusplus > 202302L || ! defined __STRICT_ANSI__
+  // P2510R3 Formatting pointers
+  s = std::format("{:06} {:07P} {:08p}", (void*)0, (const void*)0, nullptr);
+  VERIFY( s == "0x0000 0X00000 0x000000" );
+  str_int = std::format("{:#016x}", reinterpret_cast<std::uintptr_t>(p));
+  s = std::format("{:016} {:016}", p, pc);
+  VERIFY( s == (str_int + ' ' + str_int) );
+  str_int = std::format("{:#016X}", reinterpret_cast<std::uintptr_t>(p));
+  s = std::format("{:016P} {:016P}", p, pc);
+  VERIFY( s == (str_int + ' ' + str_int) );
+#endif
+}
+
 int main()
 {
   test_no_args();
@@ -354,4 +395,5 @@  int main()
   test_minmax();
   test_p1652r1();
   test_float128();
+  test_pointer();
 }
diff --git a/libstdc++-v3/testsuite/std/format/parse_ctx.cc b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
index 069dfceced5..260caf123d0 100644
--- a/libstdc++-v3/testsuite/std/format/parse_ctx.cc
+++ b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
@@ -244,10 +244,6 @@  test_pointer()
   VERIFY( ! is_std_format_spec_for<void*>("-") );
   VERIFY( ! is_std_format_spec_for<void*>(" ") );
   VERIFY( ! is_std_format_spec_for<void*>("#") );
-  VERIFY( is_std_format_spec_for<void*>("0p") ); // P2510
-  VERIFY( is_std_format_spec_for<void*>("0") );
-  VERIFY( ! is_std_format_spec_for<void*>("00p") );
-  VERIFY( is_std_format_spec_for<void*>("01p") );
   VERIFY( is_std_format_spec_for<void*>("1") );
   VERIFY( ! is_std_format_spec_for<void*>("-1") );
   VERIFY( ! is_std_format_spec_for<void*>("-1p") );
@@ -263,7 +259,6 @@  test_pointer()
   VERIFY( ! is_std_format_spec_for<void*>("s") );
   VERIFY( ! is_std_format_spec_for<void*>("?") );
   VERIFY( is_std_format_spec_for<void*>("p") );
-  VERIFY( is_std_format_spec_for<void*>("P") );
   VERIFY( ! is_std_format_spec_for<void*>("a") );
   VERIFY( ! is_std_format_spec_for<void*>("A") );
   VERIFY( ! is_std_format_spec_for<void*>("f") );
@@ -271,6 +266,16 @@  test_pointer()
   VERIFY( ! is_std_format_spec_for<void*>("g") );
   VERIFY( ! is_std_format_spec_for<void*>("G") );
   VERIFY( ! is_std_format_spec_for<void*>("+p") );
+
+#if __cplusplus > 202302L || ! defined __STRICT_ANSI__
+  // As an extension, we support P2510R3 Formatting pointers
+  VERIFY( is_std_format_spec_for<void*>("P") );
+  VERIFY( is_std_format_spec_for<void*>("0p") );
+  VERIFY( is_std_format_spec_for<void*>("0P") );
+  VERIFY( is_std_format_spec_for<void*>("0") );
+  VERIFY( is_std_format_spec_for<void*>("01p") );
+  VERIFY( ! is_std_format_spec_for<void*>("00p") );
+#endif
 }
 
 void