[PATCH] D40182: [clangd] Add parsing and value inspection to JSONExpr.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/JSONExpr.h:62
+// Array and Object also have typed indexing accessors for easy traversal:
+//   if (json::obj* Opts = O.array("options"))
+//     if (Optional<StringRef> Font = Opts->string("font"))
----------------
ioeric wrote:
> It's not obvious what `O.array("options")` does. Does it convert `O.at("options")` to an array?
Yes. Added a bit more context here.


================
Comment at: clangd/JSONExpr.h:78
 public:
-  class Object;
+  enum Kind {
+    Null,
----------------
ioeric wrote:
> I wonder if we could merge `Kind` and `ExprType`.
As discussed offline, the conceptual difference is public API vs internal implementation. If there was no difference in practice, YAGNI, but we have String->{T_String, T_StringRef}.

So either we need two enums or some hidden internal state which helps us distinguish. The latter seems more error prone to me (e.g. no help from -Wswitch).

So I prefer the current way overall, though it's not a really big deal.


https://reviews.llvm.org/D40182