Re: [Boost] [Describe] Review Reminder

From: Dominique Devienne (ddevienne_at_[hidden])
Date: 2021-03-10 17:57:57

On Fri, Mar 5, 2021 at 8:19 AM Richard Hodges via Boost <
boost_at_[hidden]> 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