<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Oct 19, 2017 8:07 PM, "Manuel Klimek" <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div class="elided-text"><div dir="ltr">On Thu, Oct 19, 2017 at 9:50 AM Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Oct 19, 2017 at 3:48 AM Sam McCall <<a href="mailto:sammccall@google.com" target="_blank">sammccall@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>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.</div><div><br></div><div>I'd propose vendoring it under tools/clang/tools/extra/<wbr>clangd/nlohmann-json so there's no question of it "leaking" into runtimes as described in this thread[2].</div><div>This also means it wouldn't solve llvm's general JSON-parser problem :-)</div><div> </div><div>Any LLVM-level objections or concerns? (Whether that library is the right technical choice for clangd can be discussed elsewhere, I think)</div><div>If anyone wants to argue that we *shouldn't* bury it in clang/tools/extra/clangd, that's fine too!</div></div></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>I'm hoping that the problem domain here is substantially simpler and it is tenable (if never really appealing) to just roll our own....</div></div></div></blockquote><div><br></div></div><div>We already had a JSON parser at some point, but that was deleted as YAML was considered a replacement (<a href="https://reviews.llvm.org/rL146735" target="_blank">https://reviews.llvm.org/<wbr>rL146735</a>).</div></div></div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>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.</div><div><br></div><div>That said, I'd be curious to see how code that is written against <span style="color:rgb(33,33,33);font-size:13px">nlohmann/json would look </span><span style="color:rgb(33,33,33)">compared to code that's currently written.</span></div></div></div></blockquote></div></div></div><div dir="auto"><a href="https://reviews.llvm.org/D39098">https://reviews.llvm.org/D39098</a> includes some conversion of serialization code.<br></div><div dir="auto">Mostly it's not shorter, just a lot easier and safer.</div><div dir="auto"><br></div><div dir="auto">I'll convert some parsing code tomorrow, I expect bigger simplifications there.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><font color="#212121">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?</font></div></div></div></blockquote></div></div></div><div dir="auto">Maybe - what problem does that solve?</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote></div><br></div></div></div>