# 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