This patch is a followup to the previous patch which implemented the
main printf parsing logic as well as sequential mode. This patch adds
index mode.
Details
- Reviewers
sivachandra lntue - Commits
- rG945fa672c60d: [libc][NFC] add index mode to printf parser
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
| libc/src/stdio/printf_core/parser.h | ||
|---|---|---|
| 31–32 | 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. | |
| 36 | Nit: Move FLOAT_MASK below line 37 and add a comment explaining what it is. | |
| 62 | Use inline_memset instead of this? | |
I have not yet gone through the tests.
| libc/src/stdio/printf_core/parser.cpp | ||
|---|---|---|
| 380 | Nit: s/cur_size/cur_type_desc ? | |
| libc/src/stdio/printf_core/parser.h | ||
| 34 | Nit: Call it PrimaryType may be? The word super can be misleading because of other concepts like super classes. | |
| 38 | Nit: Normally, things like this are named TypeDesc as a short for "type descriptor". | |
| 52 | Nit: This is not an array of sizes anymore. | |
| 109 | Looks like you can use a constexpr variable template instead of these functions? | |
| 140 | Nit: s/set_size_arr/set_type_desc/ ? | |
| 167 | Update the comment appropriately? | |
| 168 | Nit: s/size_of_index/get_type_desc/ ? | |
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.