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

Pavel Labath via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 9 02:26:04 PDT 2016


labath added subscribers: zturner, lldb-commits, clayborg.
labath added a comment.

I am cc'ing a bunch of people to make sure we don't do this behind anyone's back. I don't know whether you discussed this with anyone on lldb side (it's certainly new to me), but I think it deserves a wider audience, as this is the first case (I can recall) of moving code from lldb back to llvm.

That said, I think it's a great idea. However, you should sync up with @zturner, as he has imminent plans to do a StringExtractor refactor (https://reviews.llvm.org/D24013) within lldb and you're making a copy of it here.

Some other high-level questions (not directly related to this change, but I'd like to know where you're going with this):

- despite the renaming, the code still has a distinct lldb feel to it. I presume this will be followed by a bunch of CL's to bring it more in line with the rest of llvm (?)
- you are copying StringExtractor to llvm as well. Will you switch lldb to use that copy as well, or is the plan to bypass it completely and use llvm parsing primitives for the job?
- were you able to get lldb test suite running? If not, I can help you get that working.
- mostly out of curiosity: what is the motivation for this? I guess you have a need for a JSON parser in llvm ?

cheers.


https://reviews.llvm.org/D24369





More information about the llvm-commits mailing list