[PATCH] D61912: [analyzer] print() JSONify: Store implementation

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 29 09:22:59 PDT 2019


lebedev.ri added a comment.

In D61912#1521352 <https://reviews.llvm.org/D61912#1521352>, @Charusso wrote:

> Hey @lebedev.ri, thanks for the review!
>
> In D61912#1521306 <https://reviews.llvm.org/D61912#1521306>, @lebedev.ri wrote:
>
> > All these patches bypassed cfe-commits.
>
>
> Bypassed? It is only added when you push your stuff, which is happened as expected.


That is pretty much the opposite from what is exected, e.g.:
https://llvm.org/docs/Phabricator.html

> While Phabricator is a useful tool for some, the relevant -commits mailing list
>  is the system of record for all LLVM code review. The mailing list should be added
>  as a subscriber on all reviews, and Phabricator users should be prepared to respond
>  to free-form comments in mail sent to the commits list.



>> Why does this invent a yet another json formatter instead of using
>>  `"llvm/Support/JSON.h"`, in particular it's lightweight `json::OStream` ?
> 
> We are doing our own JSON representation which is not really the job of `json` namespace. Also it is my personal feeling to avoid write::like::that for perfectly no reason.

That was precisely the question. Why is this not using the abstractions, but does everything on it's own?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61912





More information about the cfe-commits mailing list