Add JSONScopedPrinter class

[Jaysonyan]

Summary

This change adds a JSONScopedPrinter as a subclass to ScopedPrinter.

This change is split off from D114052 and builds off of D114223.

Depends on D114684

[jhenderson]

I think JSONScopedPrinter (and possibly ScopedPrinter too, if it doesn't already have them) could do with gtest unit tests. It should be fairly straightforward to write them, and would help give confidence in the methods, especially as we may not use all of them up-front. The tests would also help with documenting what the code actually does, which will aid in future reviewing.

Will do! Do we want to test each method in isolation? or possibly in conjunction with each other to test things like the history stack? or possibly both?

Cross-posting this comment, since it's now applicable to this patch specifically: I think we probably need to do a bit of both: each method should be tested in isolation. I think we should ensure we have tests for the history stack, where it's applicable only. Basically, test the existing possible code paths, plus maybe one or two more interesting cases that test interaction between functions, if it seems sensible.

llvm/include/llvm/Support/ScopedPrinter.h

470

Nits:

1. I think we are normally explicit about public/private, so I'd add the explicit private: here, even though it's not strictly required.
2. I don't think we normally have blank lines at the start of class definitions.

474–486

I'd probably add some blank lines between these things.

490

I believe the explicit llvm is unnecessary.

496

How come we need to stringify the value here? That seems surprising to me, and may lead to undesirable behaviour in my mind (e.g. a field for an ELF struct might be a number for the 32-bit ELF format, but a string in the 64-bit ELF format (and not all numbers will be strings in the latter-case, where they are not 64-bits)).

I have this vague memory that the LLVM JSON format doesn't support 64-bit numbers. If so, I think this is a mistake and should be fixed.

528–530

Similar point to the above, but slightly different: I think we should compare this to std::numeric_limits<uint64_t>::max() and print as a number, if possible, rather than as a string.

543

Similar to my comments in D114223, I'd recomend adding helper functions to avoid some of the logic duplication in the following methods.

642

I was thinking about this, and I think we shouldn't actually print as hex in JSON, and should fallback to the standard number printing. Here's the reasoning: most of the time, we use printHex to print some number that represents an offset, or other value in a fixed-sized field, that is easier to read as hex. However, JSON format is not intended for human consumption, at least not in our use-case for this code. As such, readability is of lesser concern than parseability. If we were to store hex numbers as strings, rather than converting them to their decimal counterparts, we'd end up with some numeric values as strings, as others as integers, in a seemingly arbitrary manner to the end-user. I don't think that this is desirable.

652

I would actually print this as two separate attributes: a symbol name, and a numeric offset. That'll be easier for consumers to parse. Perhaps these should be in a separate object, to avoid surprise name clashes. Rather than:

"Label":"Symbol+0x100"

or

"Label":"Symbol",
"LabelOffset":256

do

"Label":{
  "SymName":"Symbol",
  "Offset":256
}

658

You should check through the existing usages, but at least in llvm-readelf, I think most labels use UpperCameCase style, so printing as MyValueRaw would look a little cleaner. Also, see my above comments re. symbol offset about a nested object/naming clash risk, i.e. I'd recommend:

"Label":{
  "Value":1234,
  "RawValue":4321
}

663

Why's this being left as a todo?

[Jaysonyan]

Add printBinaryImpl implementation.
Remove duplicated logic for json scope logic.
Update printHex to print numbers.
Clean up formatting.

[Jaysonyan]

In D114224#3142126, @jhenderson wrote:

Cross-posting this comment, since it's now applicable to this patch specifically: I think we probably need to do a bit of both: each method should be tested in isolation. I think we should ensure we have tests for the history stack, where it's applicable only. Basically, test the existing possible code paths, plus maybe one or two more interesting cases that test interaction between functions, if it seems sensible.

That seems reasonable to me! Will update with these tests soon.

llvm/include/llvm/Support/ScopedPrinter.h

496

I had believed LLVM JSON originally didn't support uint64_t types but since then I see that support was added here: https://github.com/llvm/llvm-project/commit/8c3adce81dc36306ba30cda0cdf458cfcf7d076c so I think I can just remove the to_string(...)

528–530

I updated this to just print the full APSInt unconditionally. Would it be preferable to cap printing numbers at std::numeric_limits<uint64_t>::max() and then fallback on strings otherwise? Since json doesn't have a max numerical value but arbitrarily long values may not always be supported. https://stackoverflow.com/a/39681707

642

That seems reasonable to me! Updated to output numeric values over hex strings.

663

My mistake, this was meant to be implemented before being put up for review.

[jhenderson]

llvm/include/llvm/Support/ScopedPrinter.h

528–530

Let's leave it without the cap, if we have a way of sensibly handling it. I think changing the type based on the value could get very confusing, and since as you say JSON has no firm limitations, printing out arbitrarily large numbers seems sensible.

An alternative (and probably better) approach would be to add support for this type to the JSON library directly (in a precursor patch).

640

Here and in the other loop below, I'd avoid the use of auto: it doesn't improve readability, and we tend to avoid the "always use auto" approach in LLVM, when the type isn't obvious from that line.

642

Perhaps worth a comment here elaborating on the reasoning, so that future users understand why.

661–665

[Jaysonyan]

Add JSONScopedPrinter tests.
Update some ScopedPrinter methods.
Fix some formatting.

[Jaysonyan]

llvm/include/llvm/Support/ScopedPrinter.h

642

I've added a comment at the top of the first method which outputs Hex. I wasn't sure if it was desirable to add a comment for every location that outputs Hex or not. Let me know if there's a preference of one over the other.

[jhenderson]

I've only skimmed the new tests, but as there are some structural issues there, I suggest you address those and then I'll confirm the test coverage and logic looks good.

llvm/include/llvm/Support/ScopedPrinter.h

183

Seems like the change in this and the printObject function below should be part of the ScopedPrinter virtualisation patch?

386

This check seems to be a behaviour change? Seems like we should keep that out of this patch, to keep this patch purely NFC for the regular ScopedPrinter

586–589

If I'm not mistaken, this is going to lead to an array of mixed types (some strings and some integers). I'm not convinced that's desirable, since it will mean needing to switch on type to determine how to handle the field. Perhaps we should do what we do elsewhere and have an array of objects, where the object has name (possibly optional) and value fields?

Alternatively, maybe the actual name isn't even needed, and the interesting thing is just the individual flag values. I guess that depends on how human readable we want this JSON output to be.

598

Same below.

599

I'm confused by this additional logic. It seems to me you're passing in a list of strings, but those strings might actually be numbers? That doesn't seem right to me with the calling code and/or our interface, rather than this function. For example, imagine our list happens to be a list of strings that are purely made up of digits, but the fact that they are digits is purely chance, and they should remain as strings. This function will convert to a list of integers. Further, a future new element in the same list without an all-digit input would cause the type of ALL the members of the list to change back to strings.

611

I'm inclined to say we should have the comment by each function. Perhaps better though, at the cost of a little more code than would otherwise be necessary, would be to have a small helper function hexNumberToInt or similar, which is then called in all places you currently do HexNumber.Value. You could then put the comment with that function, in one place.

654

Slightly surprised to see the new if here and a little further below. Could you explain them, please? Are you sure it's better to have attributes missing rather than as empty strings (I'm not entirely sure either way, and it probably depends on the context to some extent, but I'd focus on ease of parsing)?

Aside: reminder that for single line if and loop statements, remove the braces, i.e. here and in the for loop below.

llvm/unittests/Support/ScopedPrinterTest.cpp

16

I wouldn't bother with the anonymous namespace. There'd only be a problem if someone else started writing ScopedPrinterTests in another file, which I think we'd want to spot and possibly stop.

18

For new unit tests that are not to do with the JSONScopedPrinter class, I'd put them in their own patch, as they are independently useful.

34

This string isn't really scoped itself. It's more the buffer used by the stream, so I'd just call it Buffer or StreamBuffer.

[jhenderson]

llvm/unittests/Support/ScopedPrinterTest.cpp

19

What's the point of the lambda? It seems like it's adding unnecessary complexity to the test, when you could just do:

std::string ScopedString;
llvm::raw_string_ostream OS(ScopedString);
ScopedPrinter Writer(OS);

// <current body of the lambda>

const char *Out = /*...*/;
EXPECT_EQ(Out, ScopedString);

35

92–94

This setup logic might benefit from being pulled into a test fixture, to reduce the duplication between tests.

The JSON stuff could go in the same fixture.

503

Looking at this test seesm to make it clear to me that this is not hte optimal format of this output. I think you probably want it to be an array of numbers, rather than an object, with individual bytes labelled.

[Jaysonyan]

Add JSONScopedPrinter specific tests.
Update printBinaryImpl structure to include index.
Add hexNumberToInt method.
Revert JSONScopedPrinter::printListImpl to only print strings.
Add printRawFlagsImpl method to JSONScopedPrinter.

llvm/include/llvm/Support/ScopedPrinter.h

183

Moved to virtualization patch!

386

This was meant to accommodate the change I made to the making the extra printFlags method virtual. I was hoping both public printFlags(...) methods could utilize the same printFlagsImpl method but you're right this could be a change in behaviour. I've updated the virtualisation patch to keep this implementation the same and added an explicit printRawFlagsImpl method.

586–589

This was an attempt similar to the change in the ScopedPrinter implementation of printFlagsImpl to handle 2 types of public printFlags methods. I've added a printRawFlagsImpl to handle printing Flag.Value and updated this method to printing both Flag.Name and Flag.Value. This seems to best match the information that the ScopedPrinter implementation provides.

599

Yea this makes sense. The problem I was trying to solve was that the public method printList(...) takes a list of template type. But to pass to the printListImpl(...) method currently we are using the to_string method. So passing a list of numbers to printList(...) would result in a list of strings for the JSONScopedPrinter.

I've reverted this change to just unconditionally print as strings for now but maybe it would be preferable to replace the template param printList(...) with multiple overloaded printList(...) methods that can delegate to printStringListImpl(...)/printNumberListImpl(...)/printBooleanListImpl(...) methods. Interested if you have any thoughts on this.

654

This was to match closer with the public methods available. Some public methods of printBinary(...) don't provide a Str param and so it's just passed into printBinaryImpl as StringRef() and the existing ScopedPrinter::printBinaryImpl does a similar check for Str.empty(). Similarly, there are public printBinary(...) and printBinaryBlock(...) methods where the latter prints the accompanying characters and the former does not. I'm not opposed to unconditionally printing these values but since these checks are used to differentiate different public methods inside ScopedPrinter::printBinaryImpl I thought it made sense to do the same for JSONScopedPrinter::printBinaryImpl.

My mistake about the if statments, I'll try to be more careful about it in the future.

llvm/unittests/Support/ScopedPrinterTest.cpp

18

Moved ScopedPrinter-related tests to D114684 and rebased off that patch.

19

I haven't removed the lambda but it's now using a test fixture which takes a lambda. It doesn't need to use the test fixture since we're not testing the JSONScopedPrinter but I'd imagine it might be nice for the sake of consistency with other tests. Let me know if you feel otherwise.

503

I've updated the output to have the following format:

"Bytes": [
  {
    "Index": 0,
    "Value": 70,
    "Character": "F"
  },
  {
    "Index": 2,
    "Value": 111,
    "Character": "o"
  },
  ...
]

(Character attribute will be omitted for non printBinaryBlock(...) methods for now)

[jhenderson]

Could you rebase this patch on top of the other commits in the series, please, so that it's easier to see the current state?

llvm/include/llvm/Support/ScopedPrinter.h

580

I'd use the type name explicitly here. The concept of a "hex" number is purely a representation thing, so doesn't really make sense to refer to a number as a "hex number" until it's been written out somewhere. Numbers are just numbers.

I also suggest referring to the base explicitly in this comment for how they are outputted.

599

I think multiple functions is the way forward, although I'd just call them all printListImpl and let the ArrayRef type determine the version to call. Any particular reason we'd need multiple printList functions too? Seems like it's something that could be determined without it.

Some rough pseudo code:

template <typename T>
void printList(StringRef Label, const ArrayRef<T> List) {
  printListImpl(Label, List);
}

virtual void printListImpl(StringRef, ArrayRef<int> List) { /*...*/ }
virtual void printListImpl(StringRef, ArrayRef<bool> List) { /*...*/ }
virtual void printListImpl(StringRef Label, ArrayRef<std::string> List) { /*...*/ }

Actually, now that I type that out, I wonder if you could just get rid of printListImpl entirely, and make printList a set of virtual non-templated functions with multiple overloads for the different supported types? (Potentially you might still want a non-virtual printListImpl (that could be templated) to avoid code duplication of course).

654

Let's leave it as-is for now. We can always add attributes should we desire to at a future point.

That being said, I'm wondering if the Character block is actually useful for JSON output, since the character is just a human-readable version of the byte. If we're assuming people won't be directly reading JSON output and instead will be processing it themselves, the Character output wouldn't be useful at all, I think (they could reproduce it from the Bytes data, if needed). Thoughts?

[Jaysonyan]

Add JSON implementation for new printList(...) overloads.
Update comment wording.

[Jaysonyan]

Remove Character block from JSONScopedPrinter::printBinaryImpl(...)

[Jaysonyan]

llvm/include/llvm/Support/ScopedPrinter.h

599

I've added overloaded methods for printList(...) inside the virtualization diff and updated the ScopedPrinter test diff and this diff accordingly.

The implementation roughly follows the ideas you've laid out but the major difference is that the overloaded methods are public printList(...) methods rather than private printListImpl(...). This was needed because we still need to maintain a template printList(...) method to fall-back on for lists not comprised of strings, ints, or booleans.

654

You're right, adding the characters doesn't provide much value if we're analyzing the output through automated scripts. I think it makes sense to remove this block.

[jhenderson]

I've not yet looked at the JSONScopedPrinter tests again, but the body of the code looks pretty good.

llvm/include/llvm/Support/ScopedPrinter.h

620

This comment applies to all HexNumber instances, not just the one passed in, so it should be plural.

705

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], ...]
}

[Jaysonyan]

Update JSONScopedPrinter::printBinaryImpl to print initial offset rather than including a byte index in each byte entry.
Small comment change.

[Jaysonyan]

llvm/include/llvm/Support/ScopedPrinter.h

705

This makes a lot of sense and seems like a better format to me. Updated to match this format.

[jhenderson]

You should add tests (for both classes) for classof and getKind functions, since they're part of the public API.

One thought that's occurred to me: should the default JSON output really be formatted so verbosely (i.e. with all the new lines and spacing)? As the prime motivation of this is for machine readability, would it not make more sense for it to be compact, at least by default (with a possible option for pretty printing)? The amount of whitespace that's in the output could make the output data size be significantly larger than it needs to be, slowing down the parsing of said output.

llvm/include/llvm/Support/ScopedPrinter.h

96–108

All the new enums probably want to be enum class.

97

I'd avoid the use of the name Standard, as it could be confused with the C++ standard. I'd use Base, Plain or Basic for the enum value.

477

Similar comment to the above: don't use the name "Standard" for an enum value. Actually, in this case, without looking at the code, I have no clue what "Standard" even means. It should be renamed to reflect what it is used for.

489

At one point, there was some effort to set good defaults for SmallVector, allowing you to omit the second argument. It's less useful here, because we have a good guess about the amount of nesting. I think it's more likely to be a max of about 4 or 5, so 16 is probably way too high. Perhaps worth doing an audit of the usage within the LLVM code and add a few more points above the current maximum scoping (yes, I know JSONScopedPrinter isn't used really yet, but you can imagine how it would look, based on how ScopedPrinter is used already).

llvm/unittests/Support/ScopedPrinterTest.cpp

706–712

I think we need a JSONScopedPrinter version of this test, since we override startLine in that class.

706–712

Ditto.

[Jaysonyan]

Update enum naming.
Add APSInt implementation and test.
Add classof and kindOf test.

[Jaysonyan]

Update ScopeHistory SmallVector size to 8

llvm/include/llvm/Support/ScopedPrinter.h

97

Updated to use Base.

477

Updated to NoAttribute.

489

Looking through the usage of ScopedPrinter the most amount of nested scopes I came across was 6. This was a tie between a few methods in llmv-readobj (LLVMElfDumper::printVersionDependencySection, LLVMElfDumper::printBBAddrMaps, and COFFDumper::printCodeViweSymbolSection). Inside each of these methods there are 4 nested scopes and paired with the 2 scopes that would be added by JSONScopedPrinter (highest level [] and {} for each file) so I think the most nesting is 6. I'll update the SmallVector size to be 8.

llvm/unittests/Support/ScopedPrinterTest.cpp

706–712

My mistake, I actually mean to remove the overridden implementations of both startLine and getOStream. For the JSONScopedPrinter to provide these methods, it relies on json::OStream::rawValueBegin() which can only be used where values are used (elements of arrays or values to attributes). So if startLine or getOStream are called in any place which aren't these contexts (which is most of the time) then assertions inside json::OStream fail. So I think it might be more desirable to just rely on the ScopedPrinter implementation of both these methods.

[jhenderson]

llvm/include/llvm/Support/ScopedPrinter.h

96–108

This is marked as done but I don't see it here?

585–586

I'd pull this into a little helper function, shared by printNumber. It just helps avoid a (small) amount of duplication, but also helps label what the code does. (Ideally, I'd actually suggest enhancing the JSON output stream interface, but up to you whether you want to go that far)

llvm/unittests/Support/ScopedPrinterTest.cpp

36

This and the classof test don't rely on the fixture, so I'd a) change them to not use the fixture, and b) move them above it. Alternatively, see my comment below.

45

Either "nothing is" or "nothing's". Same below.

You might want to consider enhancing the current fixture, as an alternative to making these two tests not use it. In this case, I'd add the std::string, raw_string_ostream, ScopedPrinter and JSONScopedPrinter local variables used here and in the verify* functions into the base class, and then use the TearDown method to ensure JSONScopedPrinter has that empty string written.

Aside: it seems to me that this assertion is bogus - it's not that unreasonable to create a printer, but write nothing to it, to get an empty output.

706–712

Okay. I haven't looked into this, so I'll trust your judgement.

[Jaysonyan]

Update missing enum to enum class
Update test fixture to handle printing outer {} and handle nothing printed at top level.

llvm/include/llvm/Support/ScopedPrinter.h

96–108

My mistake, looks like I updated some but not all of the enums. Updated them all now.

585–586

I've added a helper function for now. I don't imagine it would take too much to enhance the JSON class to handle APSInt but I don't have the capacity to investigate right now.

llvm/unittests/Support/ScopedPrinterTest.cpp

45

Updated the test fixture to handle the teardown. I needed to add a check to ensure we're only printing an empty string if there hasn't been a call to verifyJSONScopedPrinter since printString can only be called under specific contexts. Alternatively I could call something like DictScope(W, "Label") which can be called under all contexts (since it uses the history stack) but I held off because calling printString("") with the empty string felt more appropriate. Let me know if you have any opinions on this.

[jhenderson]

llvm/include/llvm/Support/ScopedPrinter.h

496–497

I'm not a fan of having to have an enum for the scoping kind, having looked at this. I'd much rather a class that does RAII-style management of the printer be passed in and ownership taken by this printer. I was going to suggest the following interface, but I see that the DelimitedScope takes a ScopedPrinter, leading to a chicken and egg problem.

JSONScopedPrinter(raw_ostream &OS, bool PrettyPrint = false, std::unique_ptr<DelimitedScope> &&Scope = std::unique_ptr<DelimitedScope>());

The first argument is unchanged. The second argument means control of the details of indentation is left to the class, rather than needing to know what "2" or "0" means (especially as "0" is special, as it means no new lines either). The third argument allows a user to provide the appropriate scoping mechanism for their use-case, but leaves the class to take and manage ownership; if it's defaulted, then no scoping will be used.

NB: the functionality of the second and third arguments would need testing (both default and explicit values), if they aren't already.

I wonder if the way to resolve this circular issue, is to have the DelimitedScope subclasses have a default constructor, which delays assigning the ScopedPrinter to a separate function. The scope opening would then be performed when the class is assigned its printer (and closing would only happen if a printer has been assigned). What do you think of this?

llvm/unittests/Support/ScopedPrinterTest.cpp

45

The only other idea I had was to make JSONScopedPrinter an Optional in the fixture, initialised in the corresponding verify method (and optionally in other tests, if needed), but I don't think that works.

I'm happy going with your suggestion.

[Jaysonyan]

Add verifyAll method to test fixture.
Allow JSONScopedPrinter to be constructed with DelimitedScope.
Add DelimitedScope and pretty-print unit tests.

[Jaysonyan]

llvm/include/llvm/Support/ScopedPrinter.h

496–497

I agree that constructing the JSONScopedPrinter with an enum is less preferable than constructing with a DelimitedScope. I've implemented your suggestion to allow for DelimitedScope subclasses to be default constructed and later add the ScopedPrinter. This allowed us to construct JSONScopedPrinter with a DelimitedScope rather than using an enum.

Making this change also pointed out a weird piece of code which accesses the ScopedPrinter through the DelimitedScope member but it had access to the original ScopedPrinter. So I've updated that piece of code to directly access the ScopedPrinter.

I've also added tests for pretty printing and this delimitedScope ctor param. Since they require specific ctor calls they don't use the test fixture.

llvm/unittests/Support/ScopedPrinterTest.cpp

45

I'll leave it as is for now, possibly in the future that assertion will be removed and this extra work can just be deleted.

[Jaysonyan]

Fix DelimitedScopeCtor test.

[jhenderson]

llvm/lib/Support/ScopedPrinter.cpp

50 ↗

(On Diff #392679)

Consider adding a comment, as suggested inline, to "name" the pretty print/indentation parameter. The name should match the parameter's name.

52 ↗

(On Diff #392679)

I don't think you need the .get()?

llvm/unittests/Support/ScopedPrinterTest.cpp

20

I'd add a comment as suggested inline, to explain the boolean.

52

For symmetry, I might be inclined to print a string in each of the three cases, not just the "no scope" case.

[Jaysonyan]

Add param comments.
Update pretty-print tests.

[jhenderson]

Looks good to me!

llvm/include/llvm/Support/ScopedPrinter.h

 
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 namespace llvm {
 
 template <typename T> struct EnumEntry {
   StringRef Name;
   // While Name suffices in most of the cases, in certain cases
template <class T> std::string to_string(const T &Value) {
   std::string number;
   llvm::raw_string_ostream stream(number);
   stream << Value;
   return stream.str();
 }
 
 class ScopedPrinter {
 public:
-  ScopedPrinter(raw_ostream &OS) : OS(OS), IndentLevel(0) {}
+  enum ScopedPrinterKind {
+    Standard,
+    JSON,
+  };
+
+  ScopedPrinter(raw_ostream &OS, ScopedPrinterKind Kind = Standard)
+      : OS(OS), IndentLevel(0), Kind(Kind) {}
+
+  ScopedPrinterKind getKind() const { return Kind; }
+
+  static bool classof(const ScopedPrinter *SP) {
+    return SP->getKind() == Standard;
+  }
 
   virtual ~ScopedPrinter() {}
 
   void flush() { OS.flush(); }
 
   void indent(int Levels = 1) { IndentLevel += Levels; }
 
   void unindent(int Levels = 1) {
for (const auto &Flag : Flags) {
       }
     }
 
     llvm::sort(SetFlags, &flagName);
     printFlagsImpl(Label, hex(Value), SetFlags);
   }
 
   template <typename T> void printFlags(StringRef Label, T Value) {
     SmallVector<FlagEntry, 10> SetFlags;
     uint64_t Flag = 1;
     uint64_t Curr = Value;
     while (Curr > 0) {
       if (Curr & 1)
         SetFlags.emplace_back("", Flag);
       Curr >>= 1;
       Flag <<= 1;
     }
private:
 
   virtual void printBinaryImpl(StringRef Label, StringRef Str,
                                ArrayRef<uint8_t> Value, bool Block,
                                uint32_t StartOffset = 0);
 
   virtual void printFlagsImpl(StringRef Label, HexNumber Value,
                               ArrayRef<FlagEntry> Flags) {
     startLine() << Label << " [ (" << Value << ")\n";
-    for (const auto &Flag : Flags)
-      startLine() << "  " << Flag.Name << " (" << hex(Flag.Value) << ")\n";
+    for (const auto &Flag : Flags) {
+      raw_ostream &OS = startLine();
+      if (!Flag.Name.empty())
+        OS << "  " << Flag.Name << " (" << hex(Flag.Value) << ")\n";
+      else
+        OS << "  " << hex(Flag.Value) << '\n';
+    }
     startLine() << "]\n";
   }
 
   virtual void printRawFlagsImpl(StringRef Label, HexNumber Value,
                                  ArrayRef<FlagEntry> Flags) {
     startLine() << Label << " [ (" << Value << ")\n";
     for (const auto &Flag : Flags)
       startLine() << "  " << hex(Flag.Value) << '\n';
private:
   void scopedEnd(char Symbol) {
     unindent();
     startLine() << Symbol << '\n';
   }
 
   raw_ostream &OS;
   int IndentLevel;
   StringRef Prefix;
+  ScopedPrinterKind Kind;
 };
 
 template <>
 inline void
 ScopedPrinter::printHex<support::ulittle16_t>(StringRef Label,
                                               support::ulittle16_t Value) {
   startLine() << Label << ": " << hex(Value) << "\n";
 }
 
+class JSONScopedPrinter : public ScopedPrinter {
+private:
+  enum Scope {
+    Array,
+    Object,
+  };
+
+  enum ScopeKind {
+    Standard,
+    Attribute,
+    NestedAttribute,
+  };
+
+  struct ScopeContext {
+    Scope Context;
+    ScopeKind Kind;
+    ScopeContext(Scope Context, ScopeKind Kind = ScopeKind::Standard)
+        : Context(Context), Kind(Kind) {}
+  };
+
+  SmallVector<ScopeContext, 16> ScopeHistory;
+  json::OStream JOS;
+
+public:
+  JSONScopedPrinter(raw_ostream &OS)
+      : ScopedPrinter(OS, ScopedPrinter::ScopedPrinterKind::JSON), JOS(OS, 2) {}
+
+  static bool classof(const ScopedPrinter *SP) { return SP->getKind() == JSON; }
+
+  void printNumber(StringRef Label, uint64_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, uint32_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, uint16_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, uint8_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, int64_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, int32_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, int16_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, int8_t Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printNumber(StringRef Label, const APSInt &Value) override {
+    JOS.attributeBegin(Label);
+    JOS.rawValueBegin() << Value;
+    JOS.rawValueEnd();
+    JOS.attributeEnd();
+  }
+
+  void printBoolean(StringRef Label, bool Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void printString(StringRef Value) override { JOS.value(Value); }
+
+  void printString(StringRef Label, StringRef Value) override {
+    JOS.attribute(Label, Value);
+  }
+
+  void objectBegin() override {
+    scopedBegin({Scope::Object, ScopeKind::Standard});
+  }
+
+  void objectBegin(StringRef Label) override {
+    scopedBegin(Label, Scope::Object);
+  }
+
+  void objectEnd() override { scopedEnd(); }
+
+  void arrayBegin() override {
+    scopedBegin({Scope::Array, ScopeKind::Standard});
+  }
+
+  void arrayBegin(StringRef Label) override {
+    scopedBegin(Label, Scope::Array);
+  }
+
+  void arrayEnd() override { scopedEnd(); }
+
+  raw_ostream &startLine() override {
+    raw_ostream &OS = JOS.rawValueBegin();
+    JOS.rawValueEnd();
+    return OS;
+  }
+
+  raw_ostream &getOStream() override {
+    raw_ostream &OS = JOS.rawValueBegin();
+    JOS.rawValueEnd();
+    return OS;
+  }
+
+private:
+  // Output hex values as JSON numbers so that they're easier to parse.
+  uint64_t hexNumberToInt(HexNumber Hex) { return Hex.Value; }
+
+  void printFlagsImpl(StringRef Label, HexNumber Value,
+                      ArrayRef<FlagEntry> Flags) override {
+    JOS.attributeObject(Label, [&]() {
+      JOS.attribute("RawFlags", hexNumberToInt(Value));
+      JOS.attributeArray("Flags", [&]() {
+        for (const FlagEntry &Flag : Flags) {
+          JOS.objectBegin();
+          JOS.attribute("Name", Flag.Name);
+          JOS.attribute("Value", Flag.Value);
+          JOS.objectEnd();
+        }
+      });
+    });
+  }
+
+  void printRawFlagsImpl(StringRef Label, HexNumber Value,
+                         ArrayRef<FlagEntry> Flags) override {
+    JOS.attributeObject(Label, [&]() {
+      JOS.attribute("RawFlags", hexNumberToInt(Value));
+      JOS.attributeArray("Flags", [&]() {
+        for (const FlagEntry &Flag : Flags) {
+          JOS.value(Flag.Value);
+        }
+      });
+    });
+  }
+
+  void printListImpl(StringRef Label,
+                     const ArrayRef<std::string> List) override {
+    JOS.attributeArray(Label, [&]() {
+      for (StringRef Item : List)
+        JOS.value(Item);
+    });
+  }
+
+  void printHexListImpl(StringRef Label,
+                        const ArrayRef<HexNumber> List) override {
+    JOS.attributeArray(Label, [&]() {
+      for (const HexNumber &Item : List) {
+        JOS.value(hexNumberToInt(Item));
+      }
+    });
+  }
+
+  void printHexImpl(StringRef Label, HexNumber Value) override {
+    JOS.attribute(Label, hexNumberToInt(Value));
+  }
+
+  void printHexImpl(StringRef Label, StringRef Str, HexNumber Value) override {
+    JOS.attributeObject(Label, [&]() {
+      JOS.attribute("Value", Str);
+      JOS.attribute("RawValue", hexNumberToInt(Value));
+    });
+  }
+
+  void printSymbolOffsetImpl(StringRef Label, StringRef Symbol,
+                             HexNumber Value) override {
+    JOS.attributeObject(Label, [&]() {
+      JOS.attribute("SymName", Symbol);
+      JOS.attribute("Offset", hexNumberToInt(Value));
+    });
+  }
+
+  void printNumberImpl(StringRef Label, StringRef Str,
+                       StringRef Value) override {
+    JOS.attributeObject(Label, [&]() {
+      JOS.attribute("Value", Str);
+      JOS.attributeBegin("RawValue");
+      JOS.rawValueBegin() << Value;
+      JOS.rawValueEnd();
+      JOS.attributeEnd();
+    });
+  }
+
+  void printBinaryImpl(StringRef Label, StringRef Str, ArrayRef<uint8_t> Value,
+                       bool Block, uint32_t StartOffset = 0) override {
+    JOS.attributeObject(Label, [&]() {
+      if (!Str.empty())
+        JOS.attribute("Value", Str);
+      JOS.attributeArray("Bytes", [&]() {
+        uint32_t CurrentOffset = StartOffset;
+        for (uint8_t Val : Value) {
+          JOS.objectBegin();
+          JOS.attribute("Index", CurrentOffset++);
+          JOS.attribute("Value", Val);
+          if (Block) {
+            if (isPrint(Val)) {
+              JOS.attributeBegin("Character");
+              JOS.rawValueBegin() << '"' << static_cast<char>(Val) << '"';
+              JOS.rawValueEnd();
+              JOS.attributeEnd();
+            } else {
+              JOS.attribute("Character", ".");
+            }
+          }
+          JOS.objectEnd();
+        }
+      });
+    });
+  }
+
+  void scopedBegin(ScopeContext ScopeCtx) {
+    if (ScopeCtx.Context == Scope::Object)
+      JOS.objectBegin();
+    else if (ScopeCtx.Context == Scope::Array)
+      JOS.arrayBegin();
+    ScopeHistory.push_back(ScopeCtx);
+  }
+
+  void scopedBegin(StringRef Label, Scope Ctx) {
+    ScopeKind Kind = ScopeKind::Attribute;
+    if (ScopeHistory.empty() || ScopeHistory.back().Context != Scope::Object) {
+      JOS.objectBegin();
+      Kind = ScopeKind::NestedAttribute;
+    }
+    JOS.attributeBegin(Label);
+    scopedBegin({Ctx, Kind});
+  }
+
+  void scopedEnd() {
+    ScopeContext ScopeCtx = ScopeHistory.back();
+    if (ScopeCtx.Context == Scope::Object)
+      JOS.objectEnd();
+    else if (ScopeCtx.Context == Scope::Array)
+      JOS.arrayEnd();
+    if (ScopeCtx.Kind == ScopeKind::Attribute ||
+        ScopeCtx.Kind == ScopeKind::NestedAttribute)
+      JOS.attributeEnd();
+    if (ScopeCtx.Kind == ScopeKind::NestedAttribute)
+      JOS.objectEnd();
+    ScopeHistory.pop_back();
+  }
+};
+
 struct DelimitedScope {
   DelimitedScope(ScopedPrinter &W) : W(W) {}
   virtual ~DelimitedScope(){};
   ScopedPrinter &W;
 };
 
 struct DictScope : DelimitedScope {
   explicit DictScope(ScopedPrinter &W) : DelimitedScope(W) { W.objectBegin(); }

llvm/unittests/Support/ScopedPrinterTest.cpp

 //===- llvm/unittest/Support/ScopedPrinterTest.cpp - ScopedPrinter tests --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/ADT/APSInt.h"
 #include "gtest/gtest.h"
 #include <vector>
 
 using namespace llvm;
 
 class ScopedPrinterTest : public ::testing::Test {
 protected:
   using PrintFcn =
       function_ref<std::string(raw_string_ostream &, ScopedPrinter &)>;
 
   void verifyScopedPrinter(StringRef Expected, PrintFcn Fcn) {
     std::string StreamBuffer;
     raw_string_ostream OS(StreamBuffer);
     ScopedPrinter Writer(OS);
     EXPECT_EQ(Expected.str(), Fcn(OS, Writer));
   }
+
+  void verifyJSONScopedPrinter(StringRef Expected, PrintFcn Fcn) {
+    std::string StreamBuffer;
+    raw_string_ostream OS(StreamBuffer);
+    JSONScopedPrinter Writer(OS);
+    EXPECT_EQ(Expected.str(), Fcn(OS, Writer));
+  }
 };
 
 TEST_F(ScopedPrinterTest, PrintIndent) {
   auto IndentFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     W.indent();
     W.printString("|");
     W.indent();
     W.printString("|");
     W.unindent();
     W.printString("|");
     W.flush();
     return OS.str();
   };
 
   const char *Out = R"(  |
     |
   |
 )";
   verifyScopedPrinter(Out, IndentFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintNumber) {
   auto NumberFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       uint64_t Unsigned64Max = std::numeric_limits<uint64_t>::max();
       uint64_t Unsigned64Min = std::numeric_limits<uint64_t>::min();
auto NumberFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
       int32_t Signed32Max = std::numeric_limits<int32_t>::max();
       int32_t Signed32Min = std::numeric_limits<int32_t>::min();
       W.printNumber("int32_t-max", Signed32Max);
       W.printNumber("int32_t-min", Signed32Min);
 
       int16_t Signed16Max = std::numeric_limits<int16_t>::max();
       int16_t Signed16Min = std::numeric_limits<int16_t>::min();
       W.printNumber("int16_t-max", Signed16Max);
       W.printNumber("int16_t-min", Signed16Min);
 
       int8_t Signed8Max = std::numeric_limits<int8_t>::max();
       int8_t Signed8Min = std::numeric_limits<int8_t>::min();
       W.printNumber("int8_t-max", Signed8Max);
       W.printNumber("int8_t-min", Signed8Min);
 
       APSInt LargeNum("9999999999999999999999");
       W.printNumber("apsint", LargeNum);
 
       W.printNumber("label", "value", 0);
TEST_F(ScopedPrinterTest, PrintNumber) {
   int16_t-min: -32768
   int8_t-max: 127
   int8_t-min: -128
   apsint: 9999999999999999999999
   label: value (0)
 }
 )";
   verifyScopedPrinter(Out, NumberFcn);
+
+  const char *JSONOut = R"({
+  "uint64_t-max": 18446744073709551615,
+  "uint64_t-min": 0,
+  "uint32_t-max": 4294967295,
+  "uint32_t-min": 0,
+  "uint16_t-max": 65535,
+  "uint16_t-min": 0,
+  "uint8_t-max": 255,
+  "uint8_t-min": 0,
+  "int64_t-max": 9223372036854775807,
+  "int64_t-min": -9223372036854775808,
+  "int32_t-max": 2147483647,
+  "int32_t-min": -2147483648,
+  "int16_t-max": 32767,
+  "int16_t-min": -32768,
+  "int8_t-max": 127,
+  "int8_t-min": -128,
+  "apsint": 9999999999999999999999,
+  "label": {
+    "Value": "value",
+    "RawValue": 0
+  }
+})";
+  verifyJSONScopedPrinter(JSONOut, NumberFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintEnum) {
   auto EnumFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     const EnumEntry<int> EnumList[] = {{"Name1", "AltName1", 1},
                                        {"Name2", "AltName2", 2},
                                        {"Name3", "AltName3", 3}};
     EnumEntry<int> OtherEnum{"Name4", "AltName4", 4};
     {
       DictScope D(W);
       W.printEnum("Exists", EnumList[0].Value, makeArrayRef(EnumList));
       W.printEnum("DoesNotExist", OtherEnum.Value, makeArrayRef(EnumList));
     }
     return OS.str();
   };
 
   const char *Out = R"({
   Exists: Name1 (0x1)
   DoesNotExist: 0x4
 }
 )";
   verifyScopedPrinter(Out, EnumFcn);
+
+  const char *JSONOut = R"({
+  "Exists": {
+    "Value": "Name1",
+    "RawValue": 1
+  },
+  "DoesNotExist": 4
+})";
+  verifyJSONScopedPrinter(JSONOut, EnumFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintFlag) {
   auto FlagFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     const EnumEntry<int> FlagList[] = {{"Name1", "AltName1", 1},
                                        {"Name2", "AltName2", 1 << 1},
                                        {"Name3", "AltName3", 1 << 2}};
     {
TEST_F(ScopedPrinterTest, PrintFlag) {
   ]
   Flag1&3Raw [ (0x5)
     0x1
     0x4
   ]
 }
 )";
   verifyScopedPrinter(Out, FlagFcn);
+
+  const char *JSONOut = R"({
+  "NoFlag": {
+    "RawFlags": 8,
+    "Flags": []
+  },
+  "Flag1": {
+    "RawFlags": 1,
+    "Flags": [
+      {
+        "Name": "Name1",
+        "Value": 1
+      }
+    ]
+  },
+  "Flag1&3": {
+    "RawFlags": 5,
+    "Flags": [
+      {
+        "Name": "Name1",
+        "Value": 1
+      },
+      {
+        "Name": "Name3",
+        "Value": 4
+      }
+    ]
+  },
+  "NoFlagRaw": {
+    "RawFlags": 8,
+    "Flags": [
+      8
+    ]
+  },
+  "Flag1Raw": {
+    "RawFlags": 1,
+    "Flags": [
+      1
+    ]
+  },
+  "Flag1&3Raw": {
+    "RawFlags": 5,
+    "Flags": [
+      1,
+      4
+    ]
+  }
+})";
+  verifyJSONScopedPrinter(JSONOut, FlagFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintBoolean) {
   auto BooleanFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       W.printBoolean("True", true);
       W.printBoolean("False", false);
     }
     return OS.str();
   };
 
   const char *Out = R"({
   True: Yes
   False: No
 }
 )";
   verifyScopedPrinter(Out, BooleanFcn);
+
+  const char *JSONOut = R"({
+  "True": true,
+  "False": false
+})";
+  verifyJSONScopedPrinter(JSONOut, BooleanFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintVersion) {
   auto VersionFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       W.printVersion("Version", "123", "456", "789");
     }
     return OS.str();
   };
+
   const char *Out = R"({
   Version: 123.456.789
 }
 )";
   verifyScopedPrinter(Out, VersionFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintList) {
TEST_F(ScopedPrinterTest, PrintList) {
 
   const char *Out = R"({
   EmptyList: []
   NumberList: [1, 2, 3]
   StringList: [foo, bar, baz]
 }
 )";
   verifyScopedPrinter(Out, ListFcn);
+
+  const char *JSONOut = R"({
+  "EmptyList": [],
+  "NumberList": [
+    "1",
+    "2",
+    "3"
+  ],
+  "StringList": [
+    "foo",
+    "bar",
+    "baz"
+  ]
+})";
+  verifyJSONScopedPrinter(JSONOut, ListFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintHex) {
   auto HexFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       const uint64_t HexList[] = {0x1, 0x10, 0x100};
       W.printHexList("HexList", HexList);
       W.printHex("HexNumber", 0x10);
       W.printHex("HexLabel", "Name", 0x10);
       W.printSymbolOffset("SymbolOffset", "SymbolName", 0x10);
     }
     return OS.str();
   };
 
   const char *Out = R"({
   HexList: [0x1, 0x10, 0x100]
   HexNumber: 0x10
   HexLabel: Name (0x10)
   SymbolOffset: SymbolName+0x10
 }
 )";
   verifyScopedPrinter(Out, HexFcn);
+
+  const char *JSONOut = R"({
+  "HexList": [
+    1,
+    16,
+    256
+  ],
+  "HexNumber": 16,
+  "HexLabel": {
+    "Value": "Name",
+    "RawValue": 16
+  },
+  "SymbolOffset": {
+    "SymName": "SymbolName",
+    "Offset": 16
+  }
+})";
+  verifyJSONScopedPrinter(JSONOut, HexFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintString) {
   auto StringFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       const StringRef StringRefValue("Value");
       const std::string StringValue = "Value";
TEST_F(ScopedPrinterTest, PrintString) {
   CharArray: Value
   StringList [
     Value
     Value
   ]
 }
 )";
   verifyScopedPrinter(Out, StringFcn);
+
+  const char *JSONOut = R"({
+  "StringRef": "Value",
+  "String": "Value",
+  "CharArray": "Value",
+  "StringList": [
+    "Value",
+    "Value"
+  ]
+})";
+  verifyJSONScopedPrinter(JSONOut, StringFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintBinary) {
   auto BinaryFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       const uint8_t IntArray[] = {70, 111, 111, 66, 97, 114};
       const char CharArray[] = {'F', 'o', 'o', 'B', 'a', 'r'};
Binary6 (
     0000: 466F6F42 6172                        |FooBar|
   )
   Binary7 (
     0000: 466F6F42 6172                        |FooBar|
   )
 }
 )";
   verifyScopedPrinter(Out, BinaryFcn);
+
+  const char *JSONOut = R"({
+  "Binary1": {
+    "Value": "FooBar",
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70
+      },
+      {
+        "Index": 1,
+        "Value": 111
+      },
+      {
+        "Index": 2,
+        "Value": 111
+      },
+      {
+        "Index": 3,
+        "Value": 66
+      },
+      {
+        "Index": 4,
+        "Value": 97
+      },
+      {
+        "Index": 5,
+        "Value": 114
+      }
+    ]
+  },
+  "Binary2": {
+    "Value": "FooBar",
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70
+      },
+      {
+        "Index": 1,
+        "Value": 111
+      },
+      {
+        "Index": 2,
+        "Value": 111
+      },
+      {
+        "Index": 3,
+        "Value": 66
+      },
+      {
+        "Index": 4,
+        "Value": 97
+      },
+      {
+        "Index": 5,
+        "Value": 114
+      }
+    ]
+  },
+  "Binary3": {
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70
+      },
+      {
+        "Index": 1,
+        "Value": 111
+      },
+      {
+        "Index": 2,
+        "Value": 111
+      },
+      {
+        "Index": 3,
+        "Value": 66
+      },
+      {
+        "Index": 4,
+        "Value": 97
+      },
+      {
+        "Index": 5,
+        "Value": 114
+      }
+    ]
+  },
+  "Binary4": {
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70
+      },
+      {
+        "Index": 1,
+        "Value": 111
+      },
+      {
+        "Index": 2,
+        "Value": 111
+      },
+      {
+        "Index": 3,
+        "Value": 66
+      },
+      {
+        "Index": 4,
+        "Value": 97
+      },
+      {
+        "Index": 5,
+        "Value": 114
+      }
+    ]
+  },
+  "Binary5": {
+    "Bytes": [
+      {
+        "Index": 20,
+        "Value": 70,
+        "Character": "F"
+      },
+      {
+        "Index": 21,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 22,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 23,
+        "Value": 66,
+        "Character": "B"
+      },
+      {
+        "Index": 24,
+        "Value": 97,
+        "Character": "a"
+      },
+      {
+        "Index": 25,
+        "Value": 114,
+        "Character": "r"
+      }
+    ]
+  },
+  "Binary6": {
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70,
+        "Character": "F"
+      },
+      {
+        "Index": 1,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 2,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 3,
+        "Value": 66,
+        "Character": "B"
+      },
+      {
+        "Index": 4,
+        "Value": 97,
+        "Character": "a"
+      },
+      {
+        "Index": 5,
+        "Value": 114,
+        "Character": "r"
+      }
+    ]
+  },
+  "Binary7": {
+    "Bytes": [
+      {
+        "Index": 0,
+        "Value": 70,
+        "Character": "F"
+      },
+      {
+        "Index": 1,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 2,
+        "Value": 111,
+        "Character": "o"
+      },
+      {
+        "Index": 3,
+        "Value": 66,
+        "Character": "B"
+      },
+      {
+        "Index": 4,
+        "Value": 97,
+        "Character": "a"
+      },
+      {
+        "Index": 5,
+        "Value": 114,
+        "Character": "r"
+      }
+    ]
+  }
+})";
+  verifyJSONScopedPrinter(JSONOut, BinaryFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintObject) {
   auto ObjectFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       W.printObject("Object", "Value");
     }
     return OS.str();
   };
 
   const char *Out = R"({
   Object: Value
 }
 )";
   verifyScopedPrinter(Out, ObjectFcn);
+
+  const char *JSONOut = R"({
+  "Object": "Value"
+})";
+  verifyJSONScopedPrinter(JSONOut, ObjectFcn);
 }
 
 TEST_F(ScopedPrinterTest, PrintScope) {
   auto ScopeFcn = [](raw_string_ostream &OS, ScopedPrinter &W) {
     {
       DictScope D(W);
       {
         DictScope O(W, "Object");
List [
     ObjectInList {
     }
     ListInList [
     ]
   ]
 }
 )";
   verifyScopedPrinter(Out, ScopeFcn);
+
+  const char *JSONOut = R"({
+  "Object": {
+    "ObjectInObject": {},
+    "ListInObject": []
+  },
+  "List": [
+    {
+      "ObjectInList": {}
+    },
+    {
+      "ListInList": []
+    }
+  ]
+})";
+  verifyJSONScopedPrinter(JSONOut, ScopeFcn);
 }