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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/float_to_string.h:19
+
+namespace __llvm_libc {
+
----------------
Two suggestions:
1. Move the pieces internal to the implementation into an `__internal` namespace.
2. Add a comment about how client code is to use this utility.


================
Comment at: libc/src/stdio/printf_core/float_dec_converter.h:32
+
+// This implementation is based on the Ryu Printf algorithm by Ulf Adams:
+// Ulf Adams. 2019. Ryū revisited: printf floating point conversion.
----------------
Do these comment blocks belong here or next to the new utility?


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131023