[PATCH] D86045: [llvm-dwarfdump] --statistics: switch to json::OStream. NFC

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:482
+      LocationStats[0]);
   LLVM_DEBUG(
       llvm::dbgs() << Key
----------------
aprantl wrote:
> Orlando wrote:
> > jhenderson wrote:
> > > MaskRay wrote:
> > > > I am not sure repeating the message in LLVM_DEBUG is useful.
> > > > 
> > > > I can delete them in a separate change if people think the same as me.
> > > Not sure I've got any reasonable opinion here, since I don't really use this code or LLVM_DEBUG output ever. I'd prefer of one of the other typical users chime in here (e.g. @aprantl, @dblaikie, @Orlando)
> > I'm not sure how much weight my opinion carries here but I agree with removing the prints. I have never used these particular debug prints. When I want to inspect the output I usually just pipe it through `python -m json.tool` to format it.
> > I am not sure repeating the message in LLVM_DEBUG is useful.
> 
> The original idea was to have human-readable output on stderr and JSON output on stdout. But if this is no longer useful, I'm fine with removing it.
With
```
- json::OStream J(OS);
+ json::OStream J(OS, 2);
```

the JSON will be broken into multiple lines with an indentation of 2. I think that is quite readable, no need for stderr output.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86045/new/

https://reviews.llvm.org/D86045