[PATCH] D87335: [json] Create some llvm::Expected-based accessors

Sam McCall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 18 09:03:23 PDT 2020


sammccall added a comment.

This is a sensible thing to want and very nice code.
I'm not sure about putting it in JSON.h - there are a few unfortunate aspects that I just don't know how to fix, but limit the benefit which has to be measured against the cost of adding more complexity to the API.

- the error messages aren't *quite* good enough to be directly user-visible in the general case. e.g. "JSON object missing the 'id' field" - some hint to which (sub)object is often needed. U
- because the getAsX() method names are taken, the new accessors are stuck with awkward names
- Expected<pointer> (for object/array) is an awkward type, because -> doesn't really work and the meaning/impossibility of nullptr isn't obvious.
- this is useful when mapping objects with many fields, but it's not clear how to extend it to ObjectMapper or fromJSON (which maybe themselves don't have the best design, but...)




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87335/new/

https://reviews.llvm.org/D87335



More information about the llvm-commits mailing list