aizatsky added a comment. In https://reviews.llvm.org/D24369#538472, @zturner wrote:
> 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. I got rid of StringExtractor and also renamed all methods to follow the guidelines. Let me know which other style inconsistencies you see there. https://reviews.llvm.org/D24369 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
