piscisaureus added a comment.

In D66850#1648776 <https://reviews.llvm.org/D66850#1648776>, @aaron.ballman 
wrote:

> LGTM, but missing a test case.
>
> In D66850#1648557 <https://reviews.llvm.org/D66850#1648557>, @sidorovd wrote:
>
> > LGTM. I'm not an expert in JSON, but may be it makes sense to move the 
> > change a line earlier before creation of pointer representation?
>
>
> I would prefer it remains where it is -- having the `0x0` in the output for a 
> null pointer is a good thing because it conveys more information than a 
> totally empty `Type` object. We're accidentally inconsistent about this 
> currently (Decl prints 0x0 but Stmt gives an empty object).


I don't disagree, but I would argue that "0x0" is a rather poor choice, since 
now the consumer has to treat the "id" field as an opaque value, **except** 
when it's "0x0". A better choice would be to use JavaScript `null` to represent 
null pointers.
That said, I've simply followed the pattern established in `visit(Decl*)` here; 
switching to `null` would warrant a separate patch IMO.


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

https://reviews.llvm.org/D66850



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to