[libc-commits] [PATCH] D131023: [libc] add printf decimal float conversion

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Few high level comments about organization but logic LGTM.

1. The `ryu_constants.h` should be moved to a `.cpp` file.
2. There are two aspects wrt float to string conversion - one is the printf specific code, for example padding and spaces. Other is the actual conversion algorithm. I think we should separate out the actual conversion algorithm into a separately utility of it own and use it to implement the printf converter. If I am reading this patch correctly, the two aspects are being mixed up or co-located. I happy to be convinced that it is not possible, but I want  learn the reasons why if that is so.



================
Comment at: libc/src/stdio/printf_core/float_dec_converter.h:1
+//===-- Decimal Float Converter for printf ----------------------*- C++ -*-===//
+//
----------------
Why should this live entirely in a header file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131023