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

Csaba Dabis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 29 09:34:48 PDT 2019


Charusso added a comment.

>> 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.

Hm, in my mind it was in the proper order, as you could see that. Thanks for the clarification! I will add it.

>>> 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?

There is that much information per node of the ExplodedGraph that cannot fit into a 8K monitor screen but you have to see the whole node and its predecessor to understand it. We are trying to compact and reduce it as small as possible. At the moment our representation is a little bit wider than it should be. Here is an ugly WIP example: D62553 <https://reviews.llvm.org/D62553>


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