Re: [committed] libstdc++: Fix std::format("{:F}", inf) to use uppercase

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 17 Aug 2023 at 13:22, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> 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<double>::infinity();
> +  double nan = std::numeric_limits<double>::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<char>
>  {
>    std::string do_grouping() const override { return "\3\3"; }
> --
> 2.41.0
>