- Do you think the library should be accepted as a Boost library? Yes, my vote is to ACCEPT this library. - What is your evaluation of the design? Much has been debated recently on the merits of push parsers vs. pull parsers. Boost.JSON provides both a push parser (SAX-like interface) and a DOM parser (which is somewhat pull-like but I don't think is technically either kind). It does not provide a "real" pull parser, which are sometimes more efficient for deserialisation into custom types; the SAX parser can somewhat do that but is not composable, which makes it harder to use in practice, unless I'm missing something (this is usually why I end up never using SAX parsers). If so, I'd like to see a more complete example where the handler delegates part of the parsing to a class that in turn delegates part to another class (as with reading typed sub-values). I don't think this is currently possible. So I think I can understand some of the debate around this point; while I don't think it's strictly necessary that this particular library implement that, if it doesn't then another library will end up having potentially competing types and the natural Boost.Json name will already be taken. Personally, however, I don't think this is sufficient reason to block acceptance of this library. Direct pull parsing doesn't mesh well with JSON, due to the requirement to tolerate reordered keys, so any non-defective parser would have to have something DOM-like under the hood anyway. (Some kind of cache, if not necessarily a traditional DOM.) As such it should be sufficient to use json::value itself as part of a pull-like parsing interface. Regarding the SAX parser specifically, I dislike the *_part methods in the callback interface. I feel that these should be removed and only the final form retained, called only when the complete text of the value has been determined. I don't feel it is ever sensible to call back with a partially-parsed value (unless I am misunderstanding what these are for, but there is little explanation in the docs, and I wasn't able to get them called at all in my test). I also don't see the point of supplying a size argument to on_key* and on_string, as this can be read from the string_view argument instead. The various number methods don't pass this separately. Similarly, having a bool return as well as an error_code output argument seems peculiar; one or the other should be sufficient, as is it just invites inconsistency. parser.write() is incremental but basic_parser.write() is not, which seems odd. I assume this means that parser is storing an incrementally populated buffer of the full document rather than _really_ incrementally parsing. parse/parser do not appear to support consuming only part of the supplied input as a complete JSON value (returning how much was consumed), which is useful for streams where multiple objects are concatenated without explicit separation. Regarding other aspects of design, I haven't drilled down in great detail but the basic interface seems reasonably sound, other than the concerns raised above. I have mixed feelings on the initializer-list construction method -- I like that there is some method of doing this, and it seems like idiomatic C++ (particularly regarding how key-value pairs appear), but also very non-JSON -- an array of objects ends up having three nested braces. But I don't have any specific suggestion for improvement. - What is your evaluation of the implementation? It seems unfortunate (though understandable, due to heap allocation) that you cannot declare a json::value / json::object as constexpr, for cases where you do want some kind of constant reference object (e.g. a schema description). You can somewhat work around this by declaring the JSON as a constexpr string_view and later parsing it to a non-constexpr json::value, but that seems a bit awkward (and potentially bad-perf-prone if someone parses it multiple times). Otherwise I didn't really look into the implementation at all. - What is your evaluation of the documentation? Overall, good, if perhaps a bit sparse on some aspects. One pet peeve however is that most of the examples appear to be written as if "using namespace boost::json" is in effect, which I feel both encourages bad practice and makes it harder to distinguish which types are provided by the library vs. exist only for the example. This is especially confusing in the examples for tag_invoke, but not only there -- another example of confusion are the aliases for types present in both boost and std namespaces. I strongly recommend going through and changing these to explicitly use a json:: namespace prefix on all library types (including the boost/std aliases). Brevity is useful, but not at the expense of clarity. The "Using Numbers" section of the docs appears to be entirely blank. "serializer can be used to incrementally serialize a value incrementally" (redundant words are redundant) (On a related note, an example use case for serializer is listed as "ensure a fixed amount of work is performed in each cycle", but the only means of control is the length of the provided buffer. I guess this is equating work to number of characters output, which seems dubious. But I have no specific objections.) - What is your evaluation of the potential usefulness of the library? The library seems well suited for JSON as inter-process/network data interchange, other than perhaps the omission of partial input parsing. It seems less suited for JSON as configuration file -- while it can parse such files (via common extensions such as ignoring comments and trailing commas) it lacks means to pretty-print and to round-trip the comments, which would be essential if writing back a config file intended for human use. However while supporting this use case would be nice, I don't consider it mandatory. - Did you try to use the library? With what compiler? Did you have any problems? I tried it out with MSVC 2017 in standalone header-only mode, though only with a few small examples. The basic_parser examples do not compile out of the box -- with the suggested json.hpp and src.hpp includes there are link errors with basic_parser symbols. Additionally including basic_parser.hpp resolves these but starts complaining about missing max_{key,string,array,object}_size members of the handler type, which are not documented. After these are supplied then it works as expected. I don't see why these should be needed, however. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Roughly four hours over a few days reviewing docs and trying some small experiments. - Are you knowledgeable about the problem domain? I have some apps that use JSON as both interchange and configuration storage formats, although most of these do not use C++. So, "somewhat".