[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:08:00 PDT 2020


sammccall added a comment.

Argh, I hit enter too soon. One thing I'd planned to add was I'm sorry about the delay in reviewing this, too.

I think the first concern is the most important - providing an error-bearing API whose error messages aren't actually good enough for a lot of purposes is a bit of a hazard.
To give good errors you really need to pass around some context, but this cuts down on the simplicity of the API.

Happy to discuss this further if we want to try to come up with a general solution (I know clangd currently reports no errors other than "this is invalid", so I have some interest!)
Otherwise it might be best to carry these as non-member functions in the local project that can make the tradeoffs around APIs. Since they're implemented entirely on top of the public API this seems manageable, just a little ugly :-\


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