[PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 22 09:18:07 PDT 2020
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/cfe-commits/attachments/20200922/c040c7d8/attachment-0001.bin>
More information about the cfe-commits
mailing list