[libc-commits] [PATCH] D125929: [libc] add printf base 10 integer conversion

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/test/src/stdio/printf_core/converter_test.cpp:27
       __llvm_libc::printf_core::write_to_string);
+};
 
----------------
sivachandra wrote:
> I am probably missing something - how are these changes to string conversion tests related to integer conversion? Also, do we really want a separate string converter and writer for every test? If not, they can be static globals.
> 
> Another point, which can be addressed separately - we should probably not be testing against internal API for a converter, writer etc. You can have a test for verifying that the plumbing works, but everything else should probably be tested using `sprintf`.
I forgot that `StringWriter` is stateful - ignore that part of the comment about needing a new string writer for every test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125929