[Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb

[Zachary Turner via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D24369#538422, @aizatsky wrote:

> In https://reviews.llvm.org/D24369#538213, @zturner wrote:
>
> > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid the JSON parser that currently exists in LLDB won't do.  As Pavel mentioned, the code has a definite LLDB feel to it, but more importantly it depends on StringExtractor which I think probably doesn't belong in LLVM.  If you want to go down this route, I would start by deleting the StringExtractor changes from this CL.  90% of the stuff StringExtractor is used for, like hex string to number, string to integer, etc are covered by existing LLVM classes or methods.
>
>
> My rationale was that reusing parser from a child project is better than building anything new from scratch.


Agree that we always want to reuse code wherever possible, but LLVM has stricter requirements on code style and class design.  The JSON stuff itself can probably be a good starting point since clang-tidy doesn't have one (see my comment later), but I don't think `StringExtractor` belongs in LLVM.  Looking at the code, JSON really only uses 1 or 2 methods from `StringExtractor anyway, so I don't think it's a big thing to overcome.

> 

> 

> > I'm a bit surprised there's not already a json parser in LLVM.  Maybe there's a reason for it.  I think this CL needs a few more reviewers on the LLVM side, so I'm adding a few people.

> 

> 

> I can't actually find json parser within clang-tidy. Only references that I see lead eventually to YAML parsing:


I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at it, it seems to be pretty specialized and doesn't support arbitrary JSON.


https://reviews.llvm.org/D24369