[PATCH] D46054: [TableGen] Add a general-purpose JSON backend.

Simon Tatham via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 26 10:05:16 PDT 2018


simon_tatham added a comment.

> Well. The one thing I do have a strong opinion on is that the representation of very large integers should not be different from the representation of small integers, because consumers would almost certainly get that wrong.

A good point – now you put it that way, I suddenly agree strongly!

> So that leaves two options: string representations, or sending JavaScript to the hell it deserves and going with integers.

Well, since my personal use cases all involve Python, and Python copes fine with arbitrary integers, I'm happy with the latter if you are :-) And I'd definitely prefer not to have to put a decode-from-string operation in every single consumer of this JSON output.

I suppose a `cl::opt<bool>` to switch to a more cumbersome representation of integers could always be added later, if anyone turns out to really need one.

> Take a look at TGParser.cpp, checkConcrete. All valid final and needed records should pass that check. There are unfortunately still some exceptions, although it'd be nice if we could get rid of those actually.

Ah! Yes, that seems nice. And if I'm not dumping the class definitions, then perhaps it's not worth dumping the details of all the //types// either, for the same reason (a 'back end' consuming this format will already know what type to expect from any field it cares about), in which case I can simplify the output representation a great deal by removing the extra level of dereference where you have to suffix ['value'] all the time.

I agree that none of my own use cases will care about any of the things that this redesign throws away – and it makes the output JSON a great deal smaller and simpler. Thanks for the suggestions!


Repository:
  rL LLVM

https://reviews.llvm.org/D46054





More information about the llvm-commits mailing list