[PATCH] D45753: Lift JSON library from clang-tools-extra/clangd to llvm/Support.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 23 04:08:24 PDT 2018


labath added a comment.

This looks good to me, but I do feel someone else should comment on the appropriateness of including this library in llvm/Support. Chandler is listed as the code owner of Support, and he used to have opinions on json parsers in the past, so maybe he would be a good candidate (?)



================
Comment at: include/llvm/Support/JSON.h:93-96
+//   - json::obj is a type like map<ObjectKey, Value>
+//   - json::ary is a type like vector<Value>
+// These can be list-initialized, or used to build up collections in a loop.
+// json::ary(Collection) converts all items in a collection to Values.
----------------
Leftover references to `obj` and `ary`


================
Comment at: include/llvm/Support/JSON.h:457
+  // Maps a property to a field, if it exists.
+  template <typename T> bool map(const char *Prop, T &Out) {
+    assert(*this && "Must check this is an object before calling map()");
----------------
Could we use `ObjectKey` as the property type here?


Repository:
  rL LLVM

https://reviews.llvm.org/D45753





More information about the llvm-commits mailing list