[PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.

[Sam McCall via Phabricator via llvm-commits]

sammccall created this revision.
sammccall added reviewers: clayborg, kadircet, wallace.
Herald added subscribers: llvm-commits, cfe-commits, usaxena95, arphaman, mgrang, hiraditya.
Herald added projects: clang, LLVM.
sammccall requested review of this revision.
Herald added a subscriber: ilya-biryukov.

Translating between JSON objects and C++ strutctures is common.
>From experience in clangd, fromJSON/ObjectMapper work well and save a lot of
code, but aren't adopted elsewhere at least partly due to total lack of error
reporting beyond "ok"/"bad".

This new error model should be rich enough for most applications. It comprises:

- a name for the root object, so the user knows what we're parsing
- a path from the root object to the JSON node most associated with the error
- a local error message

These can be presented in two ways:

- An llvm::Error like "expected string at ConfigFile.credentials[0].username"
- A truncated dump of the original object with the error marked, like: { "credentials": [ { "username": /* error: expected string */ 42, "password": "secret" }, { ... } ] "backups": { ... } }

To track the locations, we exploit the fact that the call graph of recursive
parse functions mirror the structure of the JSON itself.
The current path is represented as a linked list of segments, each of which is
on the stack as a parameter. Concretely, fromJSON now looks like:

  bool fromJSON(const Value&, T&, Path);

The heavy parts mostly stay out of the way:

- building path segments is mostly handled in library code for common cases (arrays mapped as std::vector, objects mapped using ObjectMapper)
- no heap allocation at all unless an error is encountered, then one small one
- llvm::Error and error-in-context are created only if needed

The general top-level interface (Path::Root) is independent of fromJSON etc, and
ends up being a bit clunky.
I've added high-level parse<T>(StringRef) -> Expected<T>, but it's not general
enough to be the primary interface I think (at least, not usable in clangd).

---

This can be split into several separate patches if desired:

- the comment support in json::OStream (used to show errors in context)
- the Path type, including llvm::Error representation
- the fromJSON/ObjectMapper changes (and clangd usages - hard to separate)
- support for printing errors in context

I wanted to show it all together first, though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88103

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Support/JSON.cpp
  llvm/unittests/Support/JSONTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88103.293486.patch
Type: text/x-patch
Size: 72519 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200922/c040c7d8/attachment.bin>