From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id 447363858D20 for ; Mon, 12 Jun 2023 11:07:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 447363858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-wr1-x433.google.com with SMTP id ffacd0b85a97d-30fc2b44281so687358f8f.2 for ; Mon, 12 Jun 2023 04:07:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1686568075; x=1689160075; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=lKukftVjUGFyJkJweyYBYUhQCNWzXHQlebyW9tQid3g=; b=fVZxqjAKp+GpuZqFeJprVKf03kGqpdmawNPawMvVPBFzyafrEP0gXAYDl0cLbNPkMO T8Ju+MFQX9uehN6TcokSyjQ4ul2RcobFL5uQB8PD/dM9f3xd1NPBYYX3JIqdc2vZmPUg 5CQFpSTIiQbjVGdZMLkcnImYaFuq38Z1SceltGsb+o5RNFRFpBlkASO5DRt6ApAFueJS Kzuj780hU+fpoVyRr4w8Y+UJInXi4uk541Ym6mln/RsDolgsiOsjTYsbbwkC0qHVuWxe CskztAZx83CnU8MvUXNDqr4KJb5r638fCtVgqIfkOo+vdie2tOKo/Hli5kKt6iPFXhWG JsaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686568075; x=1689160075; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=lKukftVjUGFyJkJweyYBYUhQCNWzXHQlebyW9tQid3g=; b=MW1L4cnUrFs1GOYih4R2hqLPyXgF1rDVG4pvqlWBxnajtuJrQgVxj1kfzsqP5jKnZC K0o7B2NDAVRXNSP8+UhU9EeRMqFbt5ERyItuO7J5CZpbqsabd6rjYsn0Jia8i/XFQVJO LZEKu2GUaoPjMrR/Bz6lohkHT0Le0IYd2Os1Fj2S7oH8YEJwQ65F94e/zGVn/xBWmSH+ hosCzLeyTp8ua1pbzIG2FacpaWKtaNWD+EBF73BPUyYHrKzaeo/dlIHU13lUvV38Ytaw EePAYn+aeHidK01QucjF9uRyyKWlPGUJveGn62Ya7aU+A7vx40wdB90fRSrrFv58DbL/ kvvg== X-Gm-Message-State: AC+VfDwDSRCcf9vMbwsLQuWP2T/RO8swrv3CGJ63wBogYi5kGHEnqpVa 9/ThSja/+p78Km1wQAhi7onLrdZjlJ12i8XgieRbij9u84j7oG+vsSg= X-Google-Smtp-Source: ACHHUZ54B4O5A5CqGQlFcPVcQbDFLRzHfyU5YLyk5koQUtiKSKX+pGoiWWdS7WLGsqr2Erhxf/O6OA0DMzL/ifKCEaI= X-Received: by 2002:adf:dec5:0:b0:30f:bf71:501b with SMTP id i5-20020adfdec5000000b0030fbf71501bmr2290715wrn.61.1686568074837; Mon, 12 Jun 2023 04:07:54 -0700 (PDT) MIME-Version: 1.0 References: <20230609121025.294493-1-jwakely@redhat.com> In-Reply-To: <20230609121025.294493-1-jwakely@redhat.com> From: Prathamesh Kulkarni Date: Mon, 12 Jun 2023 16:37:18 +0530 Message-ID: Subject: Re: [committed] libstdc++: Fix P2510R3 "Formatting pointers" [PR110149] To: Jonathan Wakely Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 9 Jun 2023 at 17:41, Jonathan Wakely via Gcc-patches 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::parse): > Only alow 0 and P for C++26 and non-strict modes. > (formatter::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(p)); > + s = std::format("{} {} {}", p, pc, nullptr); > + VERIFY( s == (str_int + ' ' + str_int + " 0x0") ); > + str_int = std::format("{:#20x}", reinterpret_cast(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(p)); > + s = std::format("{:016} {:016}", p, pc); > + VERIFY( s == (str_int + ' ' + str_int) ); > + str_int = std::format("{:#016X}", reinterpret_cast(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("-") ); > VERIFY( ! is_std_format_spec_for(" ") ); > VERIFY( ! is_std_format_spec_for("#") ); > - VERIFY( is_std_format_spec_for("0p") ); // P2510 > - VERIFY( is_std_format_spec_for("0") ); > - VERIFY( ! is_std_format_spec_for("00p") ); > - VERIFY( is_std_format_spec_for("01p") ); > VERIFY( is_std_format_spec_for("1") ); > VERIFY( ! is_std_format_spec_for("-1") ); > VERIFY( ! is_std_format_spec_for("-1p") ); > @@ -263,7 +259,6 @@ test_pointer() > VERIFY( ! is_std_format_spec_for("s") ); > VERIFY( ! is_std_format_spec_for("?") ); > VERIFY( is_std_format_spec_for("p") ); > - VERIFY( is_std_format_spec_for("P") ); > VERIFY( ! is_std_format_spec_for("a") ); > VERIFY( ! is_std_format_spec_for("A") ); > VERIFY( ! is_std_format_spec_for("f") ); > @@ -271,6 +266,16 @@ test_pointer() > VERIFY( ! is_std_format_spec_for("g") ); > VERIFY( ! is_std_format_spec_for("G") ); > VERIFY( ! is_std_format_spec_for("+p") ); > + > +#if __cplusplus > 202302L || ! defined __STRICT_ANSI__ > + // As an extension, we support P2510R3 Formatting pointers > + VERIFY( is_std_format_spec_for("P") ); > + VERIFY( is_std_format_spec_for("0p") ); > + VERIFY( is_std_format_spec_for("0P") ); > + VERIFY( is_std_format_spec_for("0") ); > + VERIFY( is_std_format_spec_for("01p") ); > + VERIFY( ! is_std_format_spec_for("00p") ); > +#endif > } > > void > -- > 2.40.1 >