[libcxx-commits] [PATCH] D70631: Microsoft's floating-point to_chars powered by Ryu and Ryu Printf

Tue Feb 22 14:30:58 PST 2022

STL_MSFT added a comment.

In D70631#3337734 <https://reviews.llvm.org/D70631#3337734>, @bcain wrote:

> This test `libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp` fails downstream for us w/Hexagon SDK.  It's a subtle nit from the compiler regarding format specifiers.

This was my fault, sorry about that - I forgot that `uint32_t` is slightly platform-dependent.

> I can post a patch to change the test to use the `inttypes.h` `PRI___` specifiers instead.

I think that'd be the least invasive fix, yeah.

> But it occurs to me: since this is a lit test, are the printouts only used for debugging?

In general, they're needed to understand test failures - particularly the ones that say "this value failed, it's a randomized test, so report this to the maintainers and don't just rerun it".

> Does this test come from a third party repository that we want to stay in sync with?

Yes, https://github.com/microsoft/STL . We'd be happy to review and merge a PR there to keep it in sync with libcxx.

> The commit regards printf implementation -- are any of the calls to `printf` part of the intended scope of the test?  If not, could/should the test be redesigned to use C++ streams instead?

printf/fprintf isn't essential, we could also use iostreams, but iostreams manipulators (for the equivalent of `%016llX`) are a pain to remember (even for a Standard Library maintainer, heh) so I used printf for convenience.

(There are sprintf_s calls in the MSVC implementation that verify that to_chars prints floating-point values in the same way - those are essential since we don't want to compare against the iostreams implementation)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70631/new/

https://reviews.llvm.org/D70631