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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 19 17:32:41 PDT 2020


MaskRay added a comment.

In D86045#2227367 <https://reviews.llvm.org/D86045#2227367>, @echristo wrote:

> In D86045#2227284 <https://reviews.llvm.org/D86045#2227284>, @MaskRay wrote:
>
>> In D86045#2227240 <https://reviews.llvm.org/D86045#2227240>, @echristo wrote:
>>
>>> I feel pretty OK approving this.
>>>
>>> (Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)
>>>
>>> -eric
>>
>> Do you mean removing the OS->J rename? That would be a bit ambiguous because people usually use `OS` to refer to raw_ostream and its derivatives. The JSON stream is special.
>
> Yep. And that's one of the reasons why I discarded that one. JS would probably make sense (JSON Stream) if you'd like. No strong opinions.
>
> -eric

Thanks! I misread the previous comment (keeping the "OS" name was contemplated but discarded). I'll commit this as is tomorrow if nobody expresses a preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86045



More information about the llvm-commits mailing list