On 13/06/19 22:41 +0200, Christophe Lyon wrote: >Hi, > > >On Wed, 12 Jun 2019 at 16:54, Jonathan Wakely 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 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 . >> (__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::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? Does the attached patch make the test show what fails? 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.