Thank you, Peter, for your contribution. It is much appreciated. I would like to chime in to have the library ACCEPTED. I personally needed such functionality and had some components of it implemented and deployed for a long time. From my practical point of view the library does exactly what I often/always need, such as info for enums, to/from string conversion, serialization. Extending that functionality onto structs/classes in a uniform way is a bonus that I'll surely make use of. The potential of the idea included in the Standard or done on the compiler level is also encouraging. I am sure all wrinkles (real and imaginary) will be exposed and discussed and ironed out after the library is included and, therefore, gets a much wider exposure and deployment opportunities.

# Review of Describe ## Recommendation ACCEPT The library appears to be well designed, well-written and well-documented. I can see quite a few use cases in my work environment where it would be useful. Thank you, Peter, for sharing it and proposing it to boost. ## Design The design appears solid to me. Describing types seems easy and convenient. The `mod_*` flags are a bit unexpected at first. However, going through all of the possible combinations, I'm not sure if having lots of separate functions to retrieve each combination is any better. ## Implementation I only briefly skimmed over the implementation. It looks clean and organized. However, I have little experience with this kind preprocessor foo. ## Documentation The documentation is well-written and clear. I would have liked some explanation on the limitations of the library, not just a list of what is not supported. But Peter clarified this via email. Maybe it could be added to the documentation for everybody else. ## Usefulness I consider the library useful. I have went through our code base at work and there is quite a few things where it could come in handy. Obviously enums and printing. But also the JSON RPC example caught my eyes as we use a little JSON RPC as well. However, when going through our use cases, I found that describe often provides just a part of what is necessary to make it really useful. I consider none of the following points to be critical in hindering the acceptance of the library to boost as it is already pretty useful without those things. The scope of the library is ultimately up to those doing the hard work of designing, implementing , documenting and maintaining it. Also, I think most of it can be explored in the future independently if there is sufficient interest. ### Amount of reflection data and annotations The library aspires to be the one standard tool to *describe* your (user-defined) types and enums. Yet it "only" provides a mechanism to reflect on base and member name and the respective pointer. It does not provide reflection on the name of the type being described, nor does it provide annotations (comparable to language attributes), nor does it provide a mechanisms to describe free functions and function parameters (free and member). I understand that not every thing of this list is in scope or even doable given the current state of C++. But at least the first two points would be pretty nice. In particular, the first one (name of the type) is not clear to me why it is not provided since it already has to be named in the `BOOST_DESCRIBE_*` macro anyway. #### Annotations In a previous email I showed the `error_code` example. Others have showed serialization use cases. It comes up every-time additional data, a custom string (other than the name, which is restricted to be a valid identifier), or another type needs to be associated with the member or enumerator. Just to list some example: * error_code messages that are not identical to the error code enum identifier * mapping custom error codes onto error codes from other categories (like std, boost, http status codes, etc.) * conditional exclusion of fields from serialization * serializing to fields with names that are not valid c++ identifiers (Content-length comes to mind, although this one can be worked around with `_`) * annotating member functions with parameter names to support JSON RPC with objects, see also below I believe will grow as people start using describe. #### Reflection of the name of a described type As far as I understand, (boost/std) typeinfo cannot be used at compile-time (e.g. the name of the type). But if I describe my type, I would expect to have at least the name of the type available at compile-time as a string. So describe could provide at least this tiny bit of type info at compile-time (see also first item). This is probably out of scope, and I guess there is also problems with namespaces etc. But, I thought to bring it up as well. In particular, because if I use describe, I have to name the type and give it to the preprocessor already: struct A { std::string x = "x is real"; }; BOOST_DESCRIBE_STRUCT(A, (), (x)) struct B { A a{}; int i = 4; }; BOOST_DESCRIBE_STRUCT(B, (), (a, i)) So it would be nice to have also information about the type available, even if it's just the name. The universal print example prints {.i = 4, .a = {.x = x is real}} but I would like to print B{.i = 4, .a = A{.x = x is real}} and, if I describe `std::string` and `int` (ok, this might be insane), even BOOST_DESCRIBE_STRUCT(int, (), ()); BOOST_DESCRIBE_STRUCT(string, (), ()); prints B{.i = int{4}, .a = A{.x = string{x is real}}} To sum it up: If I describe a type `B`, I'd like to have all the information about the type available including it's name, it's members and bases, etc. #### Describing member functions and their arguments When I first saw the JSON RPC example, I thought that I could use it immediately. But taking a closer look revealed that it only supports parameter arrays and not parameter objects. And thinking about it there's really a fundamental problem. I can describe the member functions, but I cannot annotate or describe it's parameters to give them names. This is probably again out of scope. But considering describes use cases and examples, it looks like a next step towards library based reflection. ### Convenience tools for the common use cases The library provides a pretty minimal and convenient syntax to describe types. However, using those descriptions often seems to involve various degrees of heavy meta-programming. I think the library could provide a small set of convenience feature in this area for the common cases. I believe it would also help a great deal in adoption since I guess that the meta-programming wall even for very basic things might drive some users away. And whenever the default convenience tools is not good enough for a particular use-case it serves as a starting point to build your own version which is slightly different. I would consider for example `enum_to_string`, `string_to_enum`, `for_all_enumerators`, `get_member_by_name`, `get_member_by_pointer`, `for_all_members`, etc. such basic convenience tools. ### Interaction with PFR and data member offsets and data member index PFR is about **index**-based access to data member of aggregates. Describe is about **name**-based access to members of class-types. This view is certainly a bit over-simplified. But I think it has a certain truth to it. Now, it would be really awesome (and I have no idea if this is possible) if those two could interact. Consider the following example: struct A { std::string x = "x is real"; }; BOOST_DESCRIBE_STRUCT(A, (), (x)) struct B { A a{}; int i = 4; }; BOOST_DESCRIBE_STRUCT(B, (), (i, a)) // wrong order?! Now I can clear `boost::pfr::get<...>` those members. For example, `boost::pfr::get<0>(b)` returns `A&`. And I can `describe_members` them as well. Less convenient, but I can get a `A B::*` pointer. However, in an ideal world, I could match the pfr index to the member description ad vice versa. Also for all non-aggregates, boost describe could provide an alternative index based access if this is possible. As far as I know gcc on linux implements pointer to data members via offsets. Probably, all other compilers do something similar at least on the common platforms. So those offset could be sorted to check the order of data members in the describe macro and provide index based access. Also considering potential compiler-based describe, I would hope that the compiler provides information beyond just the name and the pointer, e.g., including index and offset for data members for example. At least for data types where those are/can be well defined. Layout order is/should be mandated anyway, so that the index and offset would make sense (ignoring `no_unique_address` and other corner cases). ### Who provides the description for boost and std libraries? For my types it's clear that I provide the description. Just after defining (or even inside) the enum, struct, class. But what about boost and std types. For example `boost::system::errc::errc_t` and `std::errc` seem obvious targets. There is probably quite a few more types where a description might be reasonable. Is it safe if everybody does it, maybe even slightly differently by including or excluding certain members/enumerators or ordering them in different ways? The only alternative to that mess that I can see is that `boost` or `boost::describe` provides at least the common ones optionally for convenience. ## Details about the review and reviewer - Did you try to use the library? With which compiler(s)? Did you have any problems? gcc-8 on debian. No Problems. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Approx 10 hours for documentation and playing with the library. I did only skim over the implementation. - Are you knowledgeable about the problem domain? From a users perspective. Regards, Max

On Fri, Mar 5, 2021 at 8:19 AM Richard Hodges via Boost < boost@lists.boost.org> wrote:

The Boost formal review of the Describe [...] The library is authored by Peter Dimov.

Thank you Peter for proposing this library.

Please provide in your review information you think is valuable to understand your choice to ACCEPT or REJECT including Describe as a Boost library. Please be explicit about your decision (ACCEPT or REJECT).

ACCEPT.

Some other questions you might want to consider answering: - What is your evaluation of the design?

Simple enough to describe the types. And macros can't be helped apparently. Some reviewers have complained about the DEFINE_ENUM macros. I'm fine with them, and would have liked to have them a few years back; would have saved me from creating my own inferior macro. They are welcome conveniences, to avoid repeating the enumerators. To close the ENUM chapter, I did run into two issues: https://github.com/pdimov/describe/issues/4 https://github.com/pdimov/describe/issues/6 I'm not sure what can be done about the first one. I copy/pasted/edited my enum definition, which uses trailing commas, and macros don't like trailing commas. Was clearly my mistake, but I certainly wasted time on this one. Which was compounded by the limit on 24 enumerators that blindsided me. I expect Peter will resolve the latter somehow, or at worse document the limit. I also opened https://github.com/pdimov/describe/issues/8, where I misused the STRUCT macro for an ENUM, and got no error. Easy to static_assert for that I suspect, although may not be worth it. Not for me to decide, misuse is misuse after all. My experimentation with Boost.Describe were based on generated code, using an existing code generator modified to create Peter's type descriptors, for a dozen enums, and about 200 structs for a custom network protocol, and I made a simple error during generation, thus this weird find. On the STRUCT side, I initially struggled because of namespaces. Again, I was not playing with Peter's examples directly, but trying to integrate directly Boost.Describe in my existing code, generated or not, with the hope to replace a part of it with Boost.Describe. And the generated structs happen to be in different namespaces, which can complicate matters compared to the simpler (in terms of namespaces) examples provided by Peter. I entered https://github.com/pdimov/describe/issues/7 and once again Peter helped me get past the build errors I got. I added universal printing (op<<) and generated op== for my 200+ structs, something I was missing before. The equality example Peter added during the review, which is not part of the doc, should probably be added, as it's generally useful IMHO (if one's not on C++20?) Once again, it was mostly user-error. The documentation clearly states the descriptors need to be in the described type's namespace (they are), but I was not putting the templates using the descriptors in that same namespace, and things got a bit "complicated" then. Maybe there should be a troubleshooting section in the doc, when encountering compiler errors. Easier said than done though, each case and/or compiler will be different I expect. Still, I feel like it's the biggest impediment to a successful use of Boost.Describe, as it's not easy to get past those build errors, and it might be off-putting to many "normal" developers like me.

- What is your evaluation of the implementation?

I did not look at the implementation. I do not feel qualified to review it, nor do I have the time, honestly.

- What is your evaluation of the documentation?

The documentation is very good. The language is clear. And does look good too. I've read it both on a large screen on desktop, and on a tablet. Several times :) Something I would have liked to see, was a section explaining in more details the template metaprogramming necessary when using the descriptors and MP11. For example, during the review it was mentioned SFINAE automatically removes from consideration non-described types when trying to instantate the templates using the descriptors, thus avoiding ambiguities, but the doc does not mention this. The print-an-enum examples, at compile-time versus runtime, give an inkling of the differences between the two worlds, but I feel a gentle more beginner-friendly explanation of what's going on, or links to outside doc, to better understand the concepts of a lambda with auto argument being "called" by a compile-time for-loop, for devs unfamiliar with these things, would be helpful. Not everyone is at that level of understanding, including me. There's also no section on compile-time performance in general, or the overhead of using BOOST_DEFINE_ENUM, versus a plain non-described enum. If that enum ends up included in many TUs, how much is that "costing" me? All my descriptor macros are in a separate file, i.e. explicit opt-in, to not force clients to #include Boost.Describe and its Boost.MP11 dependency. Am I overly cautious here? Without more details on compile-time performance, hard to say, except to try it, but on my cross-platform large "legacy" codebase, that's not convenient. I'd prefer the library doc giving my hard numbers, with 100, 1000 TUs with and w/o Boost.Describe used for example (The above is why I strongly disagree with forcing to use STRUCT macros inside the struct, like CLASS macros require, as I don't want all client-code to see those macros and necessary dependencies, unless they really need to)

- What is your evaluation of the potential usefulness of the library?

Very useful. I'm still not done fully leveraging it, but I'm certainly well on my way to use it in production. The ability to decouple the graph traversal of structs for operations done on them (e.g. serialization) is my end goal, and super useful, since in my case I need different types of serialization (and op== for unit testing). I had prototyped a Boost.Serialization-like solution, but Boost.Describe looks like a promising replacement. But again, I'm not done with that in time for this review to report success on the custom serialization aspects.

- Did you try to use the library? With which compiler(s)? Did you have any problems?

MSVC 2019 in C++17 mode. GCC 9.1 in C++17 mode. No particular problems, except my own failings and resulting build issues, as outlined above.

- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

Maybe 10-15 hours over several days, excluding the time to update my code generator. I've read the doc many times. And worked out build issues (with Peter's help) on my own "examples", based on Peter's examples.

- Are you knowledgeable about the problem domain?

On the enum side, somewhat, having developped (with help from this ML) a similar solution for enum-to-string, and string-to-enum several years ago. But the template to use descriptors are quite new to me, and I would have been lost without the examples. Sorting out build errors from template-voodoo like Boost.Describe requires is the challenge for devs like me, to be honest. When it builds, it works flawlessly and magically. When it doesn't, well, good luck! It's not this library's fault of course. I'm not sure anything can be done about it. I think that's it. My first Boost Review. Thanks, --DD