[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 24 01:38:58 PST 2017


ioeric added inline comments.


================
Comment at: clangd/JSONExpr.h:368
+    // Typed accessors return None/nullptr if the element has the wrong type.
+    llvm::Optional<std::nullptr_t> null(size_t I) const {
+      return (*this)[I].null();
----------------
Why is this needed? `v[x].null()` seems to be more intuitive than `v.null(x)`.


================
Comment at: unittests/clangd/JSONExprTests.cpp:203
+
+  EXPECT_FALSE(O->null("missing"));
+  EXPECT_FALSE(O->null("boolean"));
----------------
It's not very obvious that this accesses a KV and converts the value, by only reading this line. Would it make sense to make the APIs more explicit e.g. `get_as_xxx(...)`? 


https://reviews.llvm.org/D40399





More information about the cfe-commits mailing list