This is an archive of the discontinued LLVM Phabricator instance.

Add JSONScopedPrinter as a subclass to ScopedPrinter with array output
AbandonedPublic

Authored by leonardchan on Nov 16 2021, 8:56 PM.

Details

Summary

This is an alternative approach to the following diff: D111658: Add JSON output skeleton to llvm-readelf

The major difference is that in this change JSONScoperPrinter inherits from ScopedPrinter rather
than creating a ScopedPrinterBase base class which both JSONScopedPrinter and ScopedPrinter
inherit from.

In comment {D111658#3121940} I cited 3 reasons for why I didn't think this was possible. I'll go over
how this change address each problem:

Problem 1: ScopedPrinter has template methods which are not able to be virtual.
Solution: Add private internal non-template methods which can be virtual. The
public template method then transforms all template parameters into concrete types
and then used to call the internal non-template method. This enables JSONScopedPrinter
to override the functionality of all of ScopedPrinter's methods.

Problem 2: ScopedPrinter provides a method startLine() which returns the underlying
raw_ostream. It would be impossible to ensure printing directly to raw_ostream results
in valid JSON.
Solution: There is still no easy solution to the problem. Though since this change is to
enable json output for llvm-readelf. The planned solution to this problem is to have a
JSONELFDumper class similar to D111658: Add JSON output skeleton to llvm-readelf and override
methods which call startLine() to use another interface.

Problem 3: A common usage pattern inside llvm-readelf (calling ListScope(W, "attr1")
and then DictScope(W, "attr2")) produces invalid json since you can't have attributes inside
of lists. (The example would product the following json output)

ListScope(W, "attr1");
DictScope(W, "attr2");

"attr1" : [
  "attr2" : { ... }
]

Solution: Adjust both ListScope(..., Attr) and DictScope(..., Attr) to always emit an enclosing []
and then output the Attr as an element to the array. Then update calls to printString(...), printNumber(...), etc
to output enclosing {}. This ensures all calls to the ScopedPrinter can unconditionally handle being inside
the context of an array and ensure the next call is always in the context of an array. The following are code
examples and their respective outputs:

ListScope(W, "attr1");
DictScope(W, "attr2");
W.printString("attr3",  "val");

[
  "attr1",
    [
       "attr2",
      {
        "attr3" : "val"
      }
    ]
]
DictScope(W, "attr1");
DictScope(W, "attr2");
W.printString("attr3",  "val");

[
  "attr1",
    [
       "attr2",
      {
        "attr3" : "val"
      }
    ]
]

Note: There is also another approach which handles Problem 3 differently D114052: Add JSONScopedPrinter as a subclass to ScopedPrinter with history stack

Diff Detail

Event Timeline

Jaysonyan created this revision.Nov 16 2021, 8:56 PM
Jaysonyan requested review of this revision.Nov 16 2021, 8:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2021, 8:56 PM

Can this be abandoned now?

leonardchan commandeered this revision.Feb 11 2022, 11:59 AM
leonardchan edited reviewers, added: Jaysonyan; removed: leonardchan.

Can this be abandoned now?

I believe so now that we've gone with the history stack implementation.

leonardchan abandoned this revision.Feb 11 2022, 11:59 AM