[PATCH] D114224: Add JSONScopedPrinter class
Jayson Yan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 2 12:52:13 PST 2021
Jaysonyan marked 10 inline comments as done.
Jaysonyan added inline comments.
================
Comment at: llvm/include/llvm/Support/ScopedPrinter.h:737
+ JOS.objectBegin();
+ JOS.attribute("Index", CurrentOffset++);
+ JOS.attribute("Value", Val);
----------------
jhenderson wrote:
> I'm now looking at this `Index` field and wondering if it's useful at this position? We're in an array already, so the index should match the array position. It doesn't (so should be called Offset), but then why have an entry per byte, when one per array would be less verbose, and equally as informative? It would also allow the values to be written directly in the array rather than in a nested object. I.e.
> ```
> {
> "Value" : <Str>,
> "Offset" : <StartOffset>,
> "Bytes" : [Value[0], Value[1], ...]
> }
> ```
This makes a lot of sense and seems like a better format to me. Updated to match this format.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114224/new/
https://reviews.llvm.org/D114224
More information about the llvm-commits
mailing list