[llvm-dev] Adding a third-party dependency in clang-tools-extra

[Sam McCall via llvm-dev]

On Oct 19, 2017 8:07 PM, "Manuel Klimek" <klimek at google.com> wrote:

On Thu, Oct 19, 2017 at 9:50 AM Chandler Carruth <chandlerc at google.com>
wrote:

> On Thu, Oct 19, 2017 at 3:48 AM Sam McCall <sammccall at google.com> wrote:
>
>> clangd communicates with an editor via JSON-RPC. It parses JSON with
>> YAMLParser, which is awkward, and generates JSON with printf and friends,
>> which is miserable. Much of LLVM does things this way, but clangd does it a
>> lot.
>>
>> I'd like to try replacing this with a JSON library. nlohmann/json[1]
>> seems like a reasonable fit: C++11 with exceptions optional, simple build,
>> MIT license.
>>
>> I'd propose vendoring it under tools/clang/tools/extra/clangd/nlohmann-json
>> so there's no question of it "leaking" into runtimes as described in this
>> thread[2].
>> This also means it wouldn't solve llvm's general JSON-parser problem :-)
>>
>> Any LLVM-level objections or concerns? (Whether that library is the right
>> technical choice for clangd can be discussed elsewhere, I think)
>> If anyone wants to argue that we *shouldn't* bury it in
>> clang/tools/extra/clangd, that's fine too!
>>
>
> Generally, I feel like we have continued to find JSON and YAML uses in
> LLVM. I think it would be a shame to have more code in that space in the
> repository.
>
> But that brings us to another problem: outside of tests, we really
> shouldn't add more third-party code with different licenses to LLVM. As a
> compiler, LLVM has rather unique licensing requirements. So while this
> might be "fine" inside of clangd, if it moves elsewhere (and in many ways
> it should!) it would become a problem. Worse, it might be easily missed, or
> accidentally end up being used elsewhere.
>
> I would personally be fairly reluctant to take this on unless there is a
> pretty huge reason why it is needed. For example, if we had a absolute need
> to do proper XML parsing and manipulation, the amount of code required for
> that would be untenable without using one of the existing XML libraries.
> LLDB for example actually does use an XML library IIRC.
>
> I'm hoping that the problem domain here is substantially simpler and it is
> tenable (if never really appealing) to just roll our own....
>

We already had a JSON parser at some point, but that was deleted as YAML
was considered a replacement (https://reviews.llvm.org/rL146735).



Regarding writing out JSON with a library, I'd be curious in the design - I
do agree with Chandler that all designs I'd come up with are rather simple.

That said, I'd be curious to see how code that is written against nlohmann/json
would look compared to code that's currently written.

https://reviews.llvm.org/D39098 includes some conversion of serialization
code.
Mostly it's not shorter, just a lot easier and safer.

I'll convert some parsing code tomorrow, I expect bigger simplifications
there.

I do think it's well-designed. If writing something for LLVM from scratch,
it'd likely be similar, just better integrated with ADT, SourceMgr etc.

If compelling, could an alternative be to make it a build time dep for
folks wanting to build clangd, as opposed to putting it in svn?

Maybe - what problem does that solve?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171019/a0bb8190/attachment-0001.html>