[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