[libc-commits] [PATCH] D123339: [libc][NFC] implement printf parser

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/stdio/printf_files/CMakeLists.txt:1
+
+add_header_library(
----------------
It seems to me like directory should be named `printf` without the `_files` suffix. Normally, the `_files` kind of prefixes/suffixes indicate data files and not source files.


================
Comment at: libc/src/stdio/printf_files/parser.cpp:46
+
+      section.min_width = GET_ARG_VAL_SIMPLEST(int, parse_index(&cur_pos));
+    } else if (internal::isdigit(str[cur_pos])) {
----------------
I am very likely missing something obvious here: where is `parse_index` defined?


================
Comment at: libc/src/stdio/printf_files/parser.h:35
   // TODO: Make this an optional piece.
-  VariableType type_arr[TYPE_ARR_SIZE];
+  uint8_t size_arr[SIZE_ARR_LEN];
 
----------------
I think this array is storing the sizes of the var-args. Add a comment saying so. Also, what can be optional and why?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123339