[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