Re: [FPR] Review of FPR starts Today: Sept 28 - Oct 7

From: Maximilian Riemensberger (riemaxi_at_[hidden])
Date: 2020-10-07 17:04:02

This is a short review from a user perspective.  I think the precise reflection part of the library has the potential to be very useful albeit for a limited set of problems.  However, for those problems it provides a significant value in my opinion. I used the library to streamline a custom protocol whose messages where already defined via aggregate types, in particular, serialization and deserialization.  It turned out to be really nice to use and it let me write much less and much cleaner code.

So I vote ACCEPT for the precise reflection part.  I cannot see an attractive use-case, definitely not in any code I worked with. So I don't really care to much about whether its' part of the library or not.

There are two things that I find really suboptimal:

- Providing global operators for all aggregate types regardless of whether I want it or not is dangerous.  For example, including the global_ops header in another header potentially changes the behavior of half the aggregates in a code base.  I would recommend against providing this header.

- The documentation could use some improvements.  There are examples around printing. The examples for the impatient are a bit too impatient for my taste. A brief description (just a sentence or so) of what each snippet tries to accomplish would help a great deal for any first time user.

- The Tutorial discusses flat reflection and the difference between flat and precise. If I mentally remove the flat stuff not much is left.  Well printing an aggregate.  I think a couple of motivating and interesting examples here that show most of the precise reflection abilities would help the potential users to grasp what the library can do how they could apply it to their concrete use cases.

- The reference is somewhat difficult to navigate and somewhat hard to read by putting the links just inside the header code.  I personally find the boost::asio/beast or the mp11 style references easier to work with.

- Also as other reviewers have mentioned already, it's not quite clear which range of types the library can work with.  A definition of a concept/requirement that specifies that (again see the asio named requirements) would certainly help.  Providing a type function that checks the requirements would be a great thing as well.

I spent around 5 hours playing with the library, reading the docs.  I did not look into the implementation.  I believe it is *magic* as the old library name suggests.  And I'm certainly not qualified to judge or implement such compile-time magic.

Thanks sharing the library and trying to get it into boost.  I will definitely use it.

Max

On 9/27/20 8:16 PM, Benedek Thaler via Boost wrote:
> The Boost formal review of the FPR (Flat Precise Reflection, ex Magic
> Get, ex PODs Flat Reflection) starts Today, taking place from September 28, 2020
> to October 7, 2020.
>
> 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
>
> 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 FPR documentation summarizes the out-of-the-box added
> functionality for aggregate initializable structures:
>
>   - comparison operators
>   - heterogeneous comparators
>   - hash
>   - stream operators
>   - access to members by index
>   - member reflections
>   - methods for cooperation with std::tuple
>   - methods to visit each field of the structure
>
> 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!
> }
>
> Please provide in your review information you think is valuable to
> understand your choice to ACCEPT or REJECT including FPR as a
> Boost library. Please be explicit about your decision (ACCEPT or REJECT).
>
> Some other questions you might want to consider answering:
>
>    - What is your evaluation of the design?
>    - What is your evaluation of the implementation?
>    - What is your evaluation of the documentation?
>    - What is your evaluation of the potential usefulness of the library?
>    - Did you try to use the library? With which compiler(s)? Did you
>      have any problems?
>    - How much effort did you put into your evaluation? A glance? A quick
>      reading? In-depth study?
>    - Are you knowledgeable about the problem domain?
>
> More information about the Boost Formal Review Process can be found
> at: http://www.boost.org/community/reviews.html
>
> Thank you for your effort in the Boost community.
>
> Benedek
> - review manager of the FPR library
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>