- Boost - lists.boost.org

[json][review] Scott Determan review
by Scott Determan 23 Sep '20

23 Sep '20

I recommend ACCEPTING the proposed json library. - What is your evaluation of the design? Design is very good. The parser interfaces and object building interfaces meet my needs. I especially want to call out the pmr support and it's support for either sharing or transferring ownership of resources. It's unfortunate that this required a non-standard string class, but I think supporting shared ownership is worth it. I also like the use of tag_invoke for customization points. Apart from libunifex, this is the first library I've seen that uses this mechanism. I think this is the right way to do customization points. - What is your evaluation of the implementation? I looked at the value, storage_ptr, and object classes. It appears to be a very high quality implementation. However, I did this only as a sanity check, I did not spend much time looking at the implementation. The benchmarking numbers also point to a high quality implementation. - What is your evaluation of the documentation? Documentation is very good. Message for Vinnie: One minor nit caught my eye: My understanding is when implementing a tag_invoke customization point it should be a hidden friend because it improves compile times by keeping overload sets smaller. If that is best practice (and it may not be; I'm not a tag_invoke expert), then the examples in the documentation should follow this. - What is your evaluation of the potential usefulness of the library? It covers a widely useful range of json functionality and I expect this library to be widely used. If accepted I intend on using it immediately. - Did you try to use the library? With which compiler(s)? Did you have any problems? Yes: gcc 10.2 and clang 10.0.1; Linux; No problems. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Read the documentation, browsed the code, wrote some toy programs. Spent about half a day on it. - Are you knowledgeable about the problem domain? As a library consumer, I am familiar with nlohmann_json as well as the custom json library used in my current work project. I am not familiar with the problem domain from a library writer point of view.

2 1

[review][json] Boost.JSON review
by Ruben Perez 23 Sep '20

23 Sep '20

Hi all, My recommendation is to ACCEPT Boost.JSON into Boost. Parsing and serializing JSON using a DOM representation is a common enough task to have a specialized library for it in Boost. Most other languages provide built-in support for this task. I see Boost as the repository where I look at when the standard doesn't provide me something it should, so having a DOM JSON library in Boost is a must for me. - What is your evaluation of the design? Generally sound. I especially value its simplicity. Concretely: 1. The memory management model is extremely simple. I wonder if storage_ptr could be moved somewhere else long term so other libraries can use it. 2. Using concrete types for numbers (uint64_t/int64_t/double) may not fit everyone's needs but will definitely fit most and makes things easy. I miss some way of accessing json::value from generic code, but I understand visitation is currently in progress. As a user I would also be satisfied with generic versions of get_xxx, is_xxx and so on: ```cpp json::value val = /* ... */ val.get<json::object>() // or json::get<json::object>(val) ``` I also miss some way of determining the location (line and column) of parse errors. I don't know how this feature would interact with performance though. Nice to have but not required for me. I would have also liked `json::kind` be streamable, as it makes logging/debugging easier in some contexts. Nothing I can't live without, though. Finally, I would also appreciate `json::string` have a straightforward way to be converted to `std::string` (e.g. by having a `to_std_string()` member function). - What is your evaluation of the implementation? I have just looked at the interface. - What is your evaluation of the documentation? The documentation is very good and detailed, nice work. I would say the main point of improvement would be `basic_parser`, as I had a hard time understanding when some of the callbacks get called, especially the `on_xxx_part` ones. It would be also good to know if accessing the `basic_parser` state (e.g. `depth()`) is allowed from within the handler. And if it is, maybe passing the parser instance at the beginning of the parse could be useful (e.g. in the `on_document_begin`, or in a separate hook). Ideally I would like to see an example of `basic_parser` used to parse a custom struct, if it's not excessively complex. Also note that `basic_parser` docs say that it's defined in `<boost/json/detail/basic_parser.hpp>` and that the convenience header is `<boost/json.hpp>`. Instead, it would be good to note that the user should include `<boost/json/basic_parser.hpp>` and that `<boost/json.hpp>` is not enough (I understand for build speed reasons). I've noted that streaming a `json::string` outputs the string within double quotes. It surprised me at first, as I was working with the 'json::string equals std::string' mentality. I guess this behavior makes sense, but I would make a note in the 'Using strings' section. 'See also' section for https://master.json.cpp.al/json/ref/boost__json__value_from_tag.html renders functions without separation, looks strange. I would make a note in the Value conversion mechanism referencing P1895 on `tag_invoke` so users who don't know this paper don't stare at the name wondering 'why?' As a final note, googling 'Boost json' renders this link to an old version of the docs: http://vinniefalco.github.io/doc/json/index.html Consider removing it so users don't get confused (I did at the very beginning). - What is your evaluation of the potential usefulness of the library? I think it will be incredibly useful. Parsing to DOM and serializing from DOM are very widespread tasks, and having a simple, well designed `json::value` container makes things much better. I would have used it for my use cases if it had been available. - Did you try to use the library? With which compiler(s)? Did you have any problems? I used the library in the Boost version and separate compilation model. I built and installed the library using CMake 3.16.3 and g++ 9.3.0 on Ubuntu 20.04, with no problems. I then built several toy examples with the library's CMake `find_package` interface. No problems. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent around 10h reading the documentation and building examples. - Are you knowledgeable about the problem domain? I have used JSON as a data exchange format in the embedded, financial and web worlds, in C++ and other languages (JavaScript, Python and PHP). In C++ I have mainly used rapidJSON, and I would replace it with Boost.JSON for future developments. I have no experience writing JSON libraries myself. Thanks, Ruben.

3 3

Re: [boost] [review][json] My review
by Gavin Lambert 23 Sep '20

23 Sep '20

At 00:22 24/09/2020, Vinnie Falco wrote: >On Wed, Sep 23, 2020 at 1:40 AM Gavin Lambert wrote: >> I've walked back on that one a little bit; I now think that it's >ok for >> basic_parser to do that (to retain a zero-allocation guarantee) >but in >> that case it should treat numbers the same way (which it >currently >> doesn't), and ideally there should be a standard >slightly-higher-level >> parser that has a similar interface but does preassemble the >values, >> eliminating the _part callbacks (for people who prefer ease of >use over >> raw performance). > >I had that originally, it was called number_parser, it was a >public >interface, and it was one of the first things to go during our >3-month >journey of optimization. You can fish it out of the commit log if >you >care to see it. I didn't fish out the code, but given the class name, that sounds like the complete opposite of what I was suggesting. To restate again, my suggestion is: 1. Remove on_int64/uint64/double from basic_parser's handler type. 2. Add on_number(string_view, size_t) to basic_parser's handler type -- having exactly the same semantics as on_key/string and their _part companions. 3. Add custom_parser, which wraps a basic_parser but has internal (dynamically allocated if needed) storage for partial keys/strings/numbers so it never calls _part methods on its own handler type. It still uses on_number(string_view) but note that this (and on_key/string) have no reason to pass a size_t any more, because it can never be any different from the size() of the string_view. 4. The int64/uint64/double parsing code that used to be in basic_parser should be moved to a dedicated helper class, in turn used by parser and *optionally* used by the user's handler implementation for basic_parser/custom_parser. This design has the proper separation of concerns between the parser layers (not dictating a numeric storage format, and keeping number parsing separate from JSON structural parsing). It should not negatively impact performance of parser, and should positively affect performance of basic/custom_parser consumers who didn't want the default conversion anyway.

2 1

[review][JSON] RJanson's review
by Janson R. 22 Sep '20

22 Sep '20

This is a review from a library user standpoint. Thanks to Vinnie, Krystian, and the other contributors for their work. > Do you think the library should be accepted as a Boost library? I recommend ACCEPTING this library into Boost. > - What is your evaluation of the design? The design takes some time to get used to, but it fits well to the problem being tackled (a JSON-DOM library). Switching my codebases using RapidJSON to this library made the code more readable and it will also allow me to get rid of a significant amount of boilerplate. > - What is your evaluation of the implementation? Looks solid. Boost-quality I find. > - What is your evaluation of the documentation? Solid. A month ago I still had to read the source code to find some details, but now all I need for my use-cases is in the docs, well explained. > - What is your evaluation of the potential usefulness of the library? Very useful; to me it takes the same place as Nlohmann.JSON and RapidJSON, both very popular and widely used libraries, but brings better performance and a better API. Regardless of the outcome of this review, I will use this library for my future JSON-DOM needs in C++. > - Did you try to use the library? With which compiler(s)? All in standalone mode on Debian: GCC 9, 10 Clang 9, 10, 11 [trunk of 03 Jul 2020] > - How much effort did you put into your evaluation? Converted two of my codebases (which have different use-cases for a JSON-DOM), read through the implementation, read the docs at multiple stages of its maturation over the last couple months. Amounts to about 40h. > - Are you knowledgeable about the problem domain? I do not have much experience designing nor writing libraries, however I do use a significant number of them, both Boost and non-Boost. JSON is my primary format for configuration files and web communication; In the past I have used Nlohmann.JSON and then primarily RapidJSON for its speed, despite its (in my eyes) terribly user-unfriendly interface. I have also used JSON a lot in other languages (TypeScript, PureScript, Java). -- RJanson

2 1

[JSON[review] Andrzej's review
by Andrzej Krzemienski 22 Sep '20

22 Sep '20

Hi Everyone, I recommend to *ACCEPT* the library into Boost. It meets the standard for BI am definitely going to use it. I tried to use the library in two configurations in header-only mode: g++ 9.2.0 with -std=c++2a on MinGW on Windows, g++ 10.2.1 on Fedora with -DBOOST_JSON_STANDALONE and -std=c++20 All worked fine. I studied library docs and played with examples for about 30 hours. I am not particularly knowledgeable about the problem domain. But my programs have to consume and produce JSON files. DOCUMENTATION It is very good, and meets the highest Boost standards. I mean both the tutorial and reference sections. While doing this review I had to look at the documentation of the competing libraries, and they are really bad. This is why I can appreciate the big effort you put in making these docs. There is a section "Comparison" that compares Boost.JSON to other JSON libraries. I personally think it should also mention the documentation. Some observations: 1. Add reference to RFC 7159 (https://www.rfc-editor.org/info/rfc7159) It allows implementations to set limits on the range and precision of numbers accepted. 2. The documentation for array object and index mention "Satisfies the requirements of ContiguousContainer, ReversibleContainer, and SequenceContainer" These requirements are never defined or referenced. The user will not even know if you defined them or if they are "commonly known". Maybe add link to cppreference.com: https://en.cppreference.com/w/cpp/named_req/ReversibleContainer https://en.cppreference.com/w/cpp/named_req/ContiguousContainer https://en.cppreference.com/w/cpp/named_req/SequenceContainer 3. String has two sections "Member Functions": https://master.json.cpp.al/json/ref/boost__json__string.html 4. Documentation for value::operator= seems corrupt: https://master.json.cpp.al/json/ref/boost__json__value/operator_eq_/overloa… * it has sections with no content * It is an unconstrained template, it look like it can accept any type 5. Examples of input JSON-s contain a lot of escape characters in string literals, like in https://master.json.cpp.al/json/usage/conversion.html This makes it difficult to read. Consider using raw string literals. Instead of assert( serialize( jv ) == "{\"x\":4,\"y\":1,\"z\":4}" ); We would have assert( serialize( jv ) == R"({"x":4,"y":1,"z":4})" ); 6. Some functions (like operator== or swap(x, y)) are naturally part of class interface, even though they are not member functions. It is annoying and confusing that rather than being listed along with the class they belong to they are instead listed separately as "free functions" The docs are not uniform about this issue. For `json::value` operator== is listed as friends along with other member functions, but not 2-argument swap(). 7. In https://master.json.cpp.al/json/usage/conversion.html we read: " Given the following definition of customer::customer( const value& ) [...] Objects of type customer can be converted *to* and from value". This "to" is surprising here. How can a constructor converting from `value` to `customer` could be used to convert `customer` to `value`? 8. As discussed in another thread, documentation would benefit from the info on implementation-limits-related design choices. a. Any json::value that you can build can be serialized and then deserialized, and you are guaranteed that the resulting json::value will be equal to the original. b. JSON inputs where number values cannot be represented losslessly in uint64_t, int64_t and double, may render different values when parsed and then serialized back, and for extremely big number values can even fail to parse. c. Whatever JSON output you can produce with this library, we guarantee it can be passed by any common JSON implementation (probably also based on uint64_t+int64_t+double implementation. DESIGN Clear, sound and practical. The design decisions correspond with my expectations of a library: fast, easy to use and learn, with value semantics. The interface is very intuitive, I practically do not have to consult the documentation: every functionality (serialization, value alteration) has the interface that I would expect. I appreciate that it targets embedded environments. Some observations: 1. The handling of infinities and NaN's needs to be addressed. Currently, I can do: `json::serialize(std::numeric_limits<double>::infinity())` and t produces an invalid JSON. You cannot easily make a precondition on json::value assignment/construction because you expose the double as a mutable member: void fun(double x) // precondition x is not infinity or nan { json::value v = x; // ok v.as_double() *= 2.0; // this may produce infinity } So maybe you need a precondition in the serializer. 2. Boost.JSON provides its own string type. This begs a lot of design questions for `json::string`. Is it a better replacement for std::string? std::string provides the following capability: ``` void set(std::string& s, char v, char x, char z, char k) { if (cond) s = {v, k}; else s = {v, x, z, k}; } ``` json::string does not. How do I do a similar thing with json::string? 3. string::swap() has a strange precondition: you cannot swap an object with itself https://master.json.cpp.al/json/ref/boost__json__string/swap.html This is a really unexpected and potentially dangerous, and at minimum requires a very strong rationale. This would be the first swap function in std or Boost that has a precondition. While discussing a somewhat related issue (on the guarantees of the moved-from state) in the LEWG, someone remarked that there was at least one STL implementation that did a self-swap for convenience of implementation. Is it really necessary? IMPLEMENTATION I didn't look at it much. Just some quick points I observed. 1. Values of type `double` representing Infinity and NaN can be stored in json::value and when later serialized, they render a malformed JSON output. json::value should either validate special values of `double` or indicate as precondition that they should not be assigned. Alternatively, the serializer should either validate this or indicate as precondition. 2. tag_invoke() takes tag arguments by reference, does it have to? Passing by reference requires the constexpr variables to be instantiated and their address assigned. 3. Customization through ADL-discovered overloads can cause ODR violations. There is no superior alternative though. But maybe put a warning in the docs that the user must make sure that all translation units must see the same set of overloads. 4. I do not like the 2-state-switch mechanics of BOOST_JSON_STANDALONE. It assumes that a user either wants a no-Boost version or that she wants a package: all Boost components. But what if I am using C++14 and I want to use std::error_code and Boost-emulated memoty_resource and string_view? What If I use Boost version and C++17 and want to use std::string_view? Maybe you can additionally provide macros for turning Boost-emulation libraries one-by-one? I mean, one for string_view, one for error_code, and one for memory_resource. 5. The header only mode -- I really appreciate it. It is good for initial experiments. Finally, I would like to thank Vinnie and Krystian for writing and sharing this library. Regards, &rzej;

3 4

[FPR] FPR review starts Sept 28
by Benedek Thaler 22 Sep '20

22 Sep '20

The Boost formal review of the FPR (Flat Precise Reflection, ex Magic Get, ex PODs Flat Reflection) will take place from September 28, 2020 to October 7, 2020. The library is meant for accessing structure elements by index and providing other std::tuple like methods for user defined types without any macro or boilerplate code. The library is authored by Antony Polukhin, author of Boost.DLL, Stacktrace, Type Index libraries. Documentation: http://apolukhin.github.io/magic_get/ Source: https://github.com/apolukhin/magic_get If the description isn't immediately obvious for you, here's a motivating example: // requires: C++14 #include <iostream> #include <string> #include "boost/pfr/precise.hpp" struct some_person { std::string name; unsigned birth_year; }; int main() { some_person val{"Edgar Allan Poe", 1809}; std::cout << boost::pfr::get<0>(val) // No macro! << " was born in " << boost::pfr::get<1>(val); // Works with any aggregate initializables! } See more at: https://github.com/apolukhin/magic_get/blob/develop/README.md Benedek

1 0

[JSON][Review] Another Boost.JSON review
by Hadriel Kaplan 22 Sep '20

22 Sep '20

Howdy, I vote to ACCEPT Boost.JSON into boost. I think it’s a fine implementation for what it’s intended for. I don’t understand some of the other reviews that reject it for not being something else - they got an apple and are complaining it’s the wrong color for an orange. It’s fine to prefer oranges over apples, but it’s also ok to offer apples for sale in your store even if you wouldn’t buy it yourself. A lot of people like apples - they’re simple and convenient, without having to peel away the layers of complexity involved in something more pulpy. (Ok, I’ve squeezed as much juice out of that metaphor as possible...) - What is your evaluation of the design? Good. It took me a bit to get used to it, because I was used to something else (Facebook’s folly::dynamic mostly); but that would be true of any new library. - What is your evaluation of the implementation? I didn’t study the internal details. I used it as a consumer of the library. I thought the user-facing API was reasonable. The one change I made to the code was to force it to use std::string_view instead of boost::string_view, even though I used it in non-standalone mode. It would be nice if there were a separate compile flag for that, but it’s not a big deal. Having yet another custom/purpose-built string implementation was surprising. It’s not an issue, but just surprising that yet another string type is being created. It was also a bit surprising there wasn’t a function to pretty-print the serialized output (e..g, via an option arg for `serialize()`). It’s fairly common to need such a thing, for example to write to user-viewable files (e.g., configuration or property files), and potentially to debug logs, etc. Obviously the user can write a pretty-printer of their own, but this is one of those things that I think should come out-of-the-box eventually. - What is your evaluation of the documentation? Very good. As a consumer, it had most everything I needed. - What is your evaluation of the potential usefulness of the library? I think it’s very useful. Whether it will become _popular_ I don’t know. There’s no debate there is high demand for a JSON-based variant-structure library similar to Boost.JSON, but there are several popular choices already. Boost.JSON has better performance and memory usage than some of the ones I know of (nlohman and folly::dynamic), so that should give it a leg up. But those libraries also have a lot of bells and whistles, and it’s not clear how much that will matter to people. While my company also has a custom implementation for some specific uses, we do still use folly::dynamic fairly heavily, and I would seriously consider migrating to Boost.JSON in our code-base instead. - Did you try to use the library? Yes, I used it in non-standalone mode, and ran it through some tests we have in my employer’s code base. - With which compiler(s)? Gcc 7.3, C++17 mode. - Did you have any problems? Just a couple bugs that were quickly fixed by the authors. One test failure I got was with the usage of scientific notation in serialized output, as has been noted by other reviewers. It’s not _wrong_, of course, but it’s unusual - and unusual is frequently bad for interoperability. Also, I first tried it in standalone and was surprised it didn’t work even though we use C++17 - sadly, gcc 7.3 doesn’t have the pmr::memory_resource implementation. Gcc doesn’t have pmr until version 9+, which is a shame because incorporating it as standalone would be ideal; we do use boost heavily, but we almost always have issues when upgrading boost versions, so we don’t upgrade often. I think being usable as standalone would increase the uptake for Boost.JSON… but requiring gcc 9+ negates that somewhat, perhaps. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent about a day testing it. I’m sorry I didn’t have more time this past week. Life happens. :( - Are you knowledgeable about the problem domain? I’ve been using `folly::dynamic` from Facebook’s folly library since 2014, and have made changes to it for my company’s code base. It provides a similar variant-style structure based on JSON-ish types, with a parser and serializer for JSON encoding. I’ve also written a custom JSON-based variant-structure for some of my company’s particular needs, with built-in schema support, JsonPointer and XPath-style query support, etc. -hadriel

2 1

Re: [boost] [review] [json] My JSON review
by Vinnie Falco 21 Sep '20

21 Sep '20

On Mon, Sep 21, 2020 at 10:29 AM Mathias Gaunard <mathias.gaunard(a)ens-lyon.org> wrote: > > > In both cases, I'd like to read/write my data from/to JSON with the > > > same framework. > > > > Why? What specifically, is the requirement here? > > What I'd like is a way to describe how my C++ types map to a key-value > structure with normalized types so that I can easily convert my > objects back and forth through a structured self-describing and > human-readable interchange format. Right, but what I'm asking you is: *specifically* in what way would a framework that offers both JSON DOM and JSON Serialization be "consistent?" Can you show in terms of declarations, what that would look like? In other words I'm asking you to show using example code how putting these two separate concerns in a single library would offer benefits over having them in separate libraries. Thanks

3 3

[review] [json] My JSON review
by Mathias Gaunard 21 Sep '20

21 Sep '20

Hi, This is my review of Boost.JSON by Vinnie Falco and Krystian Stasiowski. I believe that the library should be REJECTED, though it would me a much better candidate with just a few minor tweaks here and there. The library provides the following components: - a parser framework - a DOM to represent/construct/edit trees of JSON-like values (json::value) - a serializer for that type Unfortunately, all three of these components are not quite satisfactory when it comes to addressing the problem domain of JSON as an interchange format. JSON is ubiquitous technology, and I feel like this library is not deserving of a name such as Boost.JSON, which would suggest "the right way to do JSON as seen by the C++ community". Parser: -------- Parsing is arguably the most important feature of any such library. However with Boost.JSON it really feels like it is an afterthought, and only its coupling with json::value was considered. These are the main issues: - The lack of proper number support from the DOM apparently propagates to the parser as well, even though there is no cost for the parser to properly support them. - The interface is closely tied to a text format and doesn't support JSON-inspired binary formats. This kind of forward thinking is what would really elevate those parsers. - The fact that only a push parser is provided. Parsing into own types usually requires a pull parser in order for nested member's parsers to combine into the outer parser. The way the parser interfaces handles numbers is not only inconsistent with the rest of its interface but also limiting. bool on_number_part( string_view, error_code& ) { return true; } bool on_int64( std::int64_t, string_view, error_code& ) { return true; } bool on_uint64( std::uint64_t, string_view, error_code& ) { return true; } bool on_double( double, string_view, error_code& ) { return true; } I would expect an interface that looks like bool on_number_part( string_view, std::size_t n, error_code& ) { return true; } bool on_number( string_view, std::size_t n, error_code& ) { return true; } bool on_int64( std::int64_t, error_code& ) { return true; } bool on_uint64( std::uint64_t, error_code& ) { return true; } bool on_double( double, error_code& ) { return true; } I see the push parser nicely integrates with beast/asio segmented buffering and incremental parsing, which is nice. DOM: ------ The DOM provides a canonical or vocabulary type to represent any JSON document, which is useful in itself. The DOM does not support numbers that are neither 64-bit integral numbers nor double-precision floating-point. This is a common limitation, but still quite disappointing, since the proposal of it being a canonical type is quite tainted by the fact it can't represent JSON data losslessly. I think it would be better if another type was provided for numbers that don't fit in the previous two categories. It uses a region allocator model based on polymorphic value, which is what the new polymorphic allocator model from Lakos et al. was specifically intended for. It puts some kind of reference-counting for life-extension semantics on top, which I'm not sure I'm a big fan of, but that seems optional. Querying capabilities for that type are pretty limited and quite un-C++ like. Examples use a C-style switch/extractor paradigm instead of a much safer variant-like visitor approach. There is no direct nested xpath-style querying like with boost property tree or other similar libraries. It feels like the interface is only a marginal improvement on that of rapidjson. Serializer: ----------- There is no serializer framework to speak of: just a serializer for the DOM type json::value, i.e. the only way to generate a JSON document is to generate a json::value and have it be serialized. A serializer interface symmetric to that of the parser could be provided instead, allowing arbitrary types to be serialized to JSON (or any other similar format) without conversion to json::value. When it comes to serializing floating-point numbers, it seems it always uses scientific notation. Most libraries have an option so that you get to choose a digit threshold at which to switch. Integers always use a fixed representation, which makes this all quite inconsistent. Review questions: ---------------------- > - What is your evaluation of the design? I can see the design is mostly concerned with receiving arbitrary JSON documents over the network, incrementally building a DOM as the data is received, and then doing the opposite on the sending path. It is good at doing that I suppose, but however is otherwise quite limited as it is unsuitable for any use of JSON as a serialization format for otherwise known messages. > - What is your evaluation of the implementation? I only took a quick glance, but it seemed ok. Can't say I'm a fan of this header-only idea, but some people like it. > - What is your evaluation of the documentation? It is quite lacking when it comes to usage of the parser beyond the DOM. It also doesn't really say how numbers are formatted in the serializer. > - What is your evaluation of the potential usefulness of the library? Limited, given the scope of the library and the quality of its interface, I don't see any reason to use it instead of rapidjson, which is an established library for which I already have converters and Asio stream adapters. > - Did you try to use the library? With which compiler(s)? Did you have any problems? I didn't. > - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Maybe half a day reading through the documentation and the mailing list. > - Are you knowledgeable about the problem domain? I have used a lot of different JSON libraries and I have written several serialization/deserialization frameworks using JSON, some of which are deeply tied into everything I do everyday.

5 14

[JSON][Review] Boost Json Review
by Maximilian Riemensberger 21 Sep '20

21 Sep '20

My review is primarily from the perspective of a library consumer. I use boost libraries and JSON on a daily basis. But neither did I ever write a boost library nor a JSON library. # Boost JSON Review > Please provide in your review information you think is valuable to > understand your choice to ACCEPT or REJECT including JSON as a > Boost library. Please be explicit about your decision (ACCEPT or REJECT). I recommend ACCEPTing the library into boost. It's well designed, professionally implemented and fills an important use case. The library is well maintained and I trust the authors to keep doing so. The documentation is good, but I the reference documentation could use some improvements in a few spots. Individual reference sections appear copy-pasted a bit too aggressively, which is clearly good for the doc writer but not so much for the doc reader. I consider none of the issues below show-stoppers that would block acceptance into boost. This is also based in the experience that the authors are responsive in maintaining the library (and other libraries) and I trust them that they will use their best judgement and whether and how to address any issues. Brief side note: I'm a user of boost libraries and I consider boost a very important part of the C++ library ecosystem, in particular, for building reliable and long lasting software components and systems. It sets a high bar for acceptance. When using individual boost libraries, an important consideration is whether the library has aged well with the evolution of the C++ standard, whether it is currently well maintained and whether it will remain well maintained in the future. Here boost sometimes appears to be a bit of a mixed bag. So I consider the maintenance promise to be quite important and in particular more important than any of the issues raise below. I want to thank all the people involved in boost for keeping it alive, useful and high quality. > Some other questions you might want to consider answering: > > - What is your evaluation of the design? I evaluated primarily the DOM interface, that is the value container, but didn't look into the parser and serializer in detail. The design is solid and clear. Smaller issues raised on slack during the review have been addressed. I particularly like that it provides an easy to use, automatic ownership allocator interface. It relieves me from the pain of manually maintaining the life-time of non-global memory pools etc. Minor comments: - The `value::is_xxx` functions inconsistently return bools or pointers. This can have unexpected consequences if they are use from a context that does not implicitly convert to bool. Also I really can't remember which one returns what. For example, `is_null`, `is_bool`, `is_number`, `is_string`, `is_structured` returns a really nice mix. (Fixed on develop) - The library provides `initializer_list` construction for `value`. I personally would prefer it wouldn't do that. Too many pitfalls with a missing pair of braces causing the meaning to change quite dramatically. Still I think many expect them in modern JSON libs. So it's up to the authors. - The library is inconsistent about `shrink_to_fit`. `object does not have it, `array` has it. Also the documentation does not state what `shrink_to_fit` does, basically repeating the errors of `std::vector`. I firmly believe that the function should either be removed or clear and precise semantics should be documented. In it's current state the following question comes to mind: Why would I ever want to call it? > - What is your evaluation of the implementation? I looked at the implementation of the `value` container. I did not look into the parser and serializer. The implementation appears clean and professional. The authors responded quickly to issues raised while studying the source code. > - What is your evaluation of the documentation? The documentation is generally good and well structured. However, the reference is occasionally pretty terse. Some parts of it are written with the assumption that any user going to the reference section has recently read the entire documentation including examples upfront. Some comments (mostly against master and develop from early Sep 20): * `value::storage()` should say: Return a reference to the `storage_ptr` to the `memory_resource` associated with the value. The same thing applies to a few other classes. * The reference for the `initializer_list` constructor could use some improvements: e.g. "Construct an object or array from an `initializer_list`". I find this hugely important since just the mere presence of an `initializer_list` constructor changes the meaning of brace initialization. * Additionally, I doesn't mention how nested `initializer_list` how are treated. Most importantly it should show or link to some examples on how the `initializer_list` are converted to objects, arrays, and non structured values because some developers (if not most) might not immediately realize from just looking at the reference documentation how nested `initializer_list` are parsed. Maybe also reference to the `initializer_list` constructors of `array` and `object`. * `object::insert` is not well-documented. I did not understand what it does when I insert an already existing key, in particular, whether assignment takes place or not. The same is true for `object::emplace`. * The `object` initializer constructor is listed under "Copy constructor". * The `object::erase` return value documentation is wrong. * `object::reserve` says "This inserts an element into the container." * `object::swap(object)` and the friend version `swap(object,object)` have rather inconsistent briefs. * The initializer_list constructor for array is listed under move constructors. * The pilfer constructors are not quite explained. There's a reference to the paper, but I think the library should have a page that concisely explains what it does and what a user of the library can expect from pilfered objects. In particular, how the thing differs from move and why a user would ever want to use it instead of move. Also the reference pages for pilfer and so on don't explain what it does. * `array::reserve` shows `BOOST_FORCEINLINE` in the docs. * `value_to` is unclear about the precedence of the three listed options (constructor, library provided conversion or `tag_invoke`) if multiple are present provided this is actually possible. The same is true for `value_from`. * `storage_ptr` thread safety is not documented. That's a real bummer for a `shared_ptr`-like construct. * I found it rather difficult to figure out the difference in the behavior of `parser::write` and `parser::write_some` just from reading the documentation. Either reference documentation should clearly state what the difference is. I had to open both pages side by side to actually spot the wording difference. For example, I think that the documentation of `write_some` should clearly state that it's intended use case is for parsing multiple JSON values from a byte stream presented through one or more buffers whereas `write` is intended for parsing one JSON value from one or more buffers. At least that's what I concluded from the documentation after some time. * The `parser` documentation should either link to or inline provide one or more examples how the parser is supposed to be used. In particular `write_some` would benefit hugely from an example that shows how to properly extract a stream of JSON values from a sequence of buffers. * If I read the documentation correctly, `basic_parser::write` behave more like `parser::write_some` than `parser::write`. I would prefer the call it `basic_parser::write_some`. * The `basic_parser` reference documentation would benefit from either a link to an example or an inline example. > - What is your evaluation of the potential usefulness of the library? I think the library supports a very narrow yet widespread use-case. It offers a modern interface and high performance. So I will definitely use and I believe it will also be useful to a wide audience. > - Did you try to use the library? With which compiler(s)? Did you have > any problems? I played with it in its STANDALONE mode with gcc 9.3. > - How much effort did you put into your evaluation? A glance? A quick > reading? In-depth study? Roughly 5 hours playing with small examples, but mostly looking into the documentation and into the code of `json::value` and related parts of the DOM implementation. > - Are you knowledgeable about the problem domain? I use JSON on a regular basis across different languages including c++ (mostly nlohmann's up to date). I've not implemented a JSON library myself. Kind regards, Max

3 4