[json][review] Scott Determan review
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.
Scott Determan wrote:
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), [...]
It is indeed a best practice, for the reason you give. That's especially important for tag_invoke, because all overloads share the same name.
participants (2)
-
Peter Dimov
-
Scott Determan