From: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
To: Jonathan Wakely <jwakely@redhat.com>
Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: [committed] libstdc++: Fix P2510R3 "Formatting pointers" [PR110149]
Date: Mon, 12 Jun 2023 16:37:18 +0530 [thread overview]
Message-ID: <CAAgBjMmww16GQSs1rwJoXs8BiV3RtREm57uLo=Aev-+7oQ_pbA@mail.gmail.com> (raw)
In-Reply-To: <20230609121025.294493-1-jwakely@redhat.com>
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
>
prev parent reply other threads:[~2023-06-12 11:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 12:10 Jonathan Wakely
2023-06-12 11:07 ` Prathamesh Kulkarni [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAgBjMmww16GQSs1rwJoXs8BiV3RtREm57uLo=Aev-+7oQ_pbA@mail.gmail.com' \
--to=prathamesh.kulkarni@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jwakely@redhat.com \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).