On Mon, 17 Jun 2019 at 15:21, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On 13/06/19 22:41 +0200, Christophe Lyon wrote:
> >Hi,
> >
> >
> >On Wed, 12 Jun 2019 at 16:54, Jonathan Wakely <jwak...@redhat.com> wrote:
> >>
> >> The std::to_chars functions from C++17 can be used to implement
> >> std::to_string with much better performance than calling snprintf. Only
> >> the __detail::__to_chars_len and __detail::__to_chars_10 functions are
> >> needed for to_string, because it always outputs base 10 representations.
> >>
> >> The return type of __detail::__to_chars_10 should not be declared before
> >> C++17, so the function body is extracted into a new function that can be
> >> reused by to_string and __detail::__to_chars_10.
> >>
> >> The existing tests for to_chars rely on to_string to check for correct
> >> answers. Now that they use the same code that doesn't actually ensure
> >> correctness, so add new tests for std::to_string that compare against
> >> printf output.
> >>
> >> * include/Makefile.am: Add new <bits/charconv.h> header.
> >> * include/Makefile.in: Regenerate.
> >> * include/bits/basic_string.h (to_string(int), to_string(unsigned))
> >> (to_string(long), to_string(unsigned long), to_string(long long))
> >> (to_string(unsigned long long)): Rewrite to use __to_chars_10_impl.
> >> * include/bits/charconv.h: New header.
> >> (__detail::__to_chars_len): Move here from <charconv>.
> >> (__detail::__to_chars_10_impl): New function extracted from
> >> __detail::__to_chars_10.
> >> * include/std/charconv (__cpp_lib_to_chars): Add, but comment out.
> >> (__to_chars_unsigned_type): New class template that reuses
> >> __make_unsigned_selector_base::__select to pick a type.
> >> (__unsigned_least_t): Redefine as
> >> __to_chars_unsigned_type<T>::type.
> >> (__detail::__to_chars_len): Move to new header.
> >> (__detail::__to_chars_10): Add inline specifier. Move code doing
> >> the
> >> output to __detail::__to_chars_10_impl and call that.
> >> * include/std/version (__cpp_lib_to_chars): Add, but comment out.
> >> * testsuite/21_strings/basic_string/numeric_conversions/char/
> >> to_string.cc: Fix reference in comment. Remove unused variable.
> >> * testsuite/21_strings/basic_string/numeric_conversions/char/
> >> to_string_int.cc: New test.
> >>
> >> Tested x86_64-linux, committed to trunk.
> >>
> >
> >The new test to_string_int.cc fails on arm-none-eabi:
> >PASS: 21_strings/basic_string/numeric_conversions/char/to_string_int.cc
> >(test for excess errors)
> >spawn
> >/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/utils/bin/qemu-wrapper.sh
> >./to_string_int.exe
> >/libstdc++-v3/testsuite/21_strings/basic_string/numeric_conversions/char/to_string_int.cc:105:
> >void check_value(T) [with T = long long int]: Assertion 's ==
> >expected' failed.
> >FAIL: 21_strings/basic_string/numeric_conversions/char/to_string_int.cc
> >execution test
>
> Does the target support "%lld" in printf?
>
> Does the .sum file show UNSUPPORTED for the existing
> 21_strings/basic_string/numeric_conversions/char/to_string.cc test?
>
No, it PASSes.
> Does the attached patch make the test show what fails?
I didn't try because I realized that it might be a problem with how I
configure newlib in my validation system.
After I added the same flags I use when I build&test manually,
to_string_int.cc now passes.
I added:
--enable-newlib-io-pos-args \
--enable-newlib-io-c99-formats \
--enable-newlib-io-long-long \
--enable-newlib-io-long-double \
to newlib's configure flags.
However, this makes several other libstdc++ tests fail on aarch64:
22_locale/money_get/get/char/5.cc execution test
22_locale/money_get/get/wchar_t/5.cc execution test
22_locale/num_get/get/char/39168.cc execution test
22_locale/num_get/get/char/4.cc execution test
22_locale/num_get/get/wchar_t/39168.cc execution test
22_locale/num_get/get/wchar_t/4.cc execution test
26_numerics/complex/inserters_extractors/char/1.cc execution test
26_numerics/complex/inserters_extractors/wchar_t/1.cc execution test
27_io/basic_istream/extractors_arithmetic/char/01.cc execution test
27_io/basic_istream/extractors_arithmetic/char/12.cc execution test
27_io/basic_istream/extractors_arithmetic/wchar_t/01.cc execution test
27_io/basic_istream/extractors_arithmetic/wchar_t/12.cc execution test
but I guess that should be a separate bug report....
I'll add those flags to my automated tests.
Thanks,
Christophe
> I suspect the problem is that the test relies on snprintf to check the
> answers are correct, even though the actual library code doesn't need
> snprintf any longer. Previously the std::to_string functions were all
> guarded by _GLIBCXX_USE_C99_STDIO and so I'm guessing were not
> supported for arm-none-eabi. Now the overloads for integral types
> should work without any C library support, and so the new test is
> enabled unconditionally. But the test itself still uses the C library.
>
> Maybe I should have a simple test that doesn't use snprintf and just
> checks some hardcoded values produce the expected results, and a more
> involved test that compares the results to snprintf but uses
> { dg-require-string-conversions "" } like the existing test.
>
>
>
