On 23/02/21 11:30 -0500, Patrick Palka via Libstdc++ wrote:
On Mon, 22 Feb 2021, Patrick Palka wrote:

This makes the hexadecimal section of the long double std::to_chars
testcase more robust by avoiding false-negative FAILs due to printf
using a different leading hex digit than us, and by additionally
verifying the correctness of the hexadecimal form via round-tripping
through std::from_chars.

Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64.  Does this
look OK for trunk?

The commit message could explain the issue better, so here's v2 with a
more detailed commit message.

-- >8 --

Subject: [PATCH] libstdc++: Robustify long double std::to_chars testcase
[PR98384]

The long double std::to_chars testcase currently verifies the
correctness of its output by comparing it to that of printf, so if
there's a mismatch between to_chars and printf, the test FAILs.  This
works well for the scientific, fixed and general formatting modes,
because the corresponding printf conversion specifiers (%e, %f and %g)
are rigidly specified.

But this doesn't work so well for the hex formatting mode because the
corresponding printf conversion specifier %a is more flexibly specified.
For instance, the hexadecimal forms 0x1p+0, 0x2p-1, 0x4p-2 and 0x8p-3
are all equivalent and valid outputs of the %a specifier for the number
1.  The apparent freedom here is the choice of leading hex digit -- the
standard just requires that the leading hex digit is nonzero for
normalized numbers.

Currently, our hexadecimal formatting implementation uses 0/1/2 as the
leading hex digit for floating point types that have an implicit leading
mantissa bit which in practice means all supported floating point types
except x86 long double.  The latter type has a 64 bit mantissa with an
explicit leading mantissa bit, and for this type our implementation uses
the most significant four bits of the mantissa as leading hex digit.
This seems to be consistent with most printf implementations, but not
all, as PR98384 illustrates.

In order to avoid false-positive FAILs due to arbitrary disagreement
between to_chars and printf about the choice of leading hex digit, this
patch makes the testcase's verification via printf conditional on the
leading hex digits first agreeing.  An additional verification step is
also added: round-tripping the output of to_chars through from_chars
should yield the original value.

Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64.  Does this
look OK for trunk?

@@ -50,6 +51,38 @@ namespace detail
void
test01()
{
+  // Verifies correctness of the hexadecimal form [BEGIN,END) for VALUE by
+  // round-tripping it through from_chars (if available).
+  auto verify_via_from_chars = [] (char *begin, char *end, long double value) {
+#if __cpp_lib_to_chars >= 201611L || _GLIBCXX_HAVE_USELOCALE

This is currently going to fail, because we don't actually define
__cpp_lib_to_chars yet (we should fix that!)

Is checking the feature test macro here useful? We know that
floating-point from_chars was committed before to_chars, so if this
test is running, we should have from_chars (modulo uselocale being
available, so that check is right). Is this to make the test usable
for other C++ std::lib implementations?

+    long double roundtrip;
+    auto result = from_chars(begin, end, roundtrip, chars_format::hex);
+    VERIFY( result.ec == errc{} );
+    VERIFY( result.ptr == end );
+    VERIFY( roundtrip == value );
+#endif

Reply via email to