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

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Apr 18 14:31:43 PDT 2022

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
   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?

  rG LLVM Github Monorepo



More information about the libc-commits mailing list