[libc-commits] [PATCH] D123424: [libc][NFC] add index mode to printf parser

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Few nits inline, but can you add tests so that I understand how the new API is to be used and then I might have more comments.



================
Comment at: libc/src/stdio/printf_core/parser.h:27
+#ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
   internal::ArgList args_start;
   size_t args_index = 1;
----------------
Nit: Add a comment explain the difference between `args_start` vs `args_cur`.  I think the naming is indicative enough, but would be nice if you can also add a note.


================
Comment at: libc/src/stdio/printf_core/parser.h:32
+  static constexpr size_t SIZE_ARR_LEN = 32;
+  static constexpr uint8_t FLOAT_MASK = 0x80;
+  // size_arr stores the sizes of the variables in the ArgList. This is used in
----------------
Nit: Move `FLOAT_MASK` below line 37 and add a comment explaining what it is.


================
Comment at: libc/src/stdio/printf_core/parser.h:47
+      : str(new_str), args_cur(args), args_start(args) {
+    for (size_t i = 0; i < SIZE_ARR_LEN; ++i)
+      size_arr[i] = 0;
----------------
Use `inline_memset` instead of this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123424