From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D125D38582A3 for ; Sat, 19 Aug 2023 10:05:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D125D38582A3 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692439528; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5G1TZo2BMgqoIL5cJsVO7dWrs72i/xzbDZeyjuYxtG4=; b=Jffld4mV2uU0YL6++mG3/pcxzZRL52rczio29FKYoHYt25SisaSavr07PMOis2WtWbBDal AcnW47MRL7mI3fN+XaExmZuz/jIBJtKIHuZ2LKARVs+yuA4A0Upuf38QHuCxWHf/sl8sHN iehXZHxyMkgE2wbc4nSt2D/SF5i1gN8= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-303-a5JfKeEqPsarSHUOLGpJoA-1; Sat, 19 Aug 2023 06:05:26 -0400 X-MC-Unique: a5JfKeEqPsarSHUOLGpJoA-1 Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-4fe0f0f6ec0so941992e87.1 for ; Sat, 19 Aug 2023 03:05:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692439525; x=1693044325; 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=5G1TZo2BMgqoIL5cJsVO7dWrs72i/xzbDZeyjuYxtG4=; b=X3uS7XfBSgOUSME/xyTjKVWnJ+Z6g8LZ/rZW3+B6lg5GGtsBYzygLGtj7CdPMwnHMd TYMROwnXG/UoOQCT6lxmElt5BDRfQLJxn0pvPA6qXBhcjdM7WcS75jXyf7kB7pjOWSov dtAZ8tuaDT1ETMO3supcziG2vi7O64SRs0uLrjYKWqjenZv0R3JUPIZG/A2t0tAup/d+ qoLkxMwcGBCeBsrZGPtKm8cpDyNvyM8T7tODY/sNEeu8CWnML0tn1UDpzH3NmtBX6NOR OpKO5jEUr5GZZN+2odfG6xNgEIgKSavY2BJdZx2x2Al/5YcuzMJwm3JGv37swU2SAoAm TzQw== X-Gm-Message-State: AOJu0YyUMD0vRYQ+0df5dyEew7Aj+EuLyQEbZ5kOfyQAH90+6KE/V2wg 7z//Rwrk+m/Dg/iorGJSUnEQ6HTqmWl/3bQtfWBHipRPZ/N7+IZA7OdxrNieGhVJuyH+T0m+CKU 33xLd6RgqXkc2EeDGbSdfc/w83FjXd5U= X-Received: by 2002:a05:6512:1294:b0:4f9:6adf:3981 with SMTP id u20-20020a056512129400b004f96adf3981mr478199lfs.33.1692439525339; Sat, 19 Aug 2023 03:05:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFCS9TTzedL/H/s7nckX3GLWK4mgtdobe+WMhbUtjsyyTmH8P/6dGl3eCVdvxuiSNzzQUcd6jfsINlg9K8y5ZQ= X-Received: by 2002:a05:6512:1294:b0:4f9:6adf:3981 with SMTP id u20-20020a056512129400b004f96adf3981mr478194lfs.33.1692439524911; Sat, 19 Aug 2023 03:05:24 -0700 (PDT) MIME-Version: 1.0 References: <20230817122226.966809-1-jwakely@redhat.com> In-Reply-To: <20230817122226.966809-1-jwakely@redhat.com> From: Jonathan Wakely Date: Sat, 19 Aug 2023 11:05:14 +0100 Message-ID: Subject: Re: [committed] libstdc++: Fix std::format("{:F}", inf) to use uppercase To: Jonathan Wakely Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP 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 Thu, 17 Aug 2023 at 13:22, Jonathan Wakely via Libstdc++ wrote: > > Tested x86_64-linux. Pushed to trunk. Backport to gcc-13 will follow. Re the backport, I forgot to say that this changes the order/values of the enumerators for _Pres_type. In theory that could cause incompatibilities between GCC 13.2 and 13.3, if one object uses the old definition of std::formatter::parse and another object uses the new definition of std::formatter::format, or vice versa. But given that 99.999% of uses of std::formatter are via std::format (not using the formatter class directly), I expect that the calls to parse and format will always be instantiated together at the same time, and so every object will contain both symbols. That will mean that the linker will always pick a "matching pair" of symbols, i.e. both symbols will use the new enumerator values, or both will use the old enumerator values, and so in practice there won't be a mismatch. I could have added the new _Pres_F enumerator at the end, so it would not alter the values of the other enumerators. But that wouldn't completely avoid the problem anyway, because a new object that uses _Pres_F in formatter::parse would be incompatible with an old object that didn't know about the new _Pres_F value in formatter::format. So I would prefer to keep the _Pres_F enumerator adjacent to _Pres_f and the other ones for floating-point presentation types. There have been so many other fixes to std::format that I think it will be reasonable to tell anybody using it that they should just use GCC 13.3 consistently anyway, and not mix code built with 13.2 and 13.3 if they're using the experimental C++20 std::format implementation. > > -- >8 -- > > std::format was treating {:f} and {:F} identically on the basis that for > the fixed 1.234567 format there are no alphabetical characters that need > to be in uppercase. But that's wrong for infinities and NaNs, which > should be formatted as "INF" and "NAN" for {:F}. > > libstdc++-v3/ChangeLog: > > * include/std/format (__format::_Pres_type): Add _Pres_F. > (__formatter_fp::parse): Use _Pres_F for 'F'. > (__formatter_fp::format): Set __upper for _Pres_F. > * testsuite/std/format/functions/format.cc: Check formatting of > infinity and NaN for each presentation type. > --- > libstdc++-v3/include/std/format | 10 ++++++++-- > .../testsuite/std/format/functions/format.cc | 12 ++++++++++++ > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format > index a8db10d6460..40c7d6128f6 100644 > --- a/libstdc++-v3/include/std/format > +++ b/libstdc++-v3/include/std/format > @@ -309,7 +309,7 @@ namespace __format > // Presentation types for integral types (including bool and charT). > _Pres_d = 1, _Pres_b, _Pres_B, _Pres_o, _Pres_x, _Pres_X, _Pres_c, > // Presentation types for floating-point types. > - _Pres_a = 1, _Pres_A, _Pres_e, _Pres_E, _Pres_f, _Pres_g, _Pres_G, > + _Pres_a = 1, _Pres_A, _Pres_e, _Pres_E, _Pres_f, _Pres_F, _Pres_g, _Pres_G, > _Pres_p = 0, _Pres_P, // For pointers. > _Pres_s = 0, // For strings and bool. > _Pres_esc = 0xf, // For strings and charT. > @@ -1382,10 +1382,13 @@ namespace __format > ++__first; > break; > case 'f': > - case 'F': > __spec._M_type = _Pres_f; > ++__first; > break; > + case 'F': > + __spec._M_type = _Pres_F; > + ++__first; > + break; > case 'g': > __spec._M_type = _Pres_g; > ++__first; > @@ -1442,6 +1445,9 @@ namespace __format > __use_prec = true; > __fmt = chars_format::scientific; > break; > + case _Pres_F: > + __upper = true; > + [[fallthrough]]; > case _Pres_f: > __use_prec = true; > __fmt = chars_format::fixed; > diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc > index 4db5202815d..59ed3be8baa 100644 > --- a/libstdc++-v3/testsuite/std/format/functions/format.cc > +++ b/libstdc++-v3/testsuite/std/format/functions/format.cc > @@ -159,6 +159,18 @@ test_alternate_forms() > VERIFY( s == "1.e+01 1.e+01 1.e+01" ); > } > > +void > +test_infnan() > +{ > + double inf = std::numeric_limits::infinity(); > + double nan = std::numeric_limits::quiet_NaN(); > + std::string s; > + s = std::format("{0} {0:e} {0:E} {0:f} {0:F} {0:g} {0:G} {0:a} {0:A}", inf); > + VERIFY( s == "inf inf INF inf INF inf INF inf INF" ); > + s = std::format("{0} {0:e} {0:E} {0:f} {0:F} {0:g} {0:G} {0:a} {0:A}", nan); > + VERIFY( s == "nan nan NAN nan NAN nan NAN nan NAN" ); > +} > + > struct euro_punc : std::numpunct > { > std::string do_grouping() const override { return "\3\3"; } > -- > 2.41.0 >